Add gyp define for optimization level

Add gyp define for optimization level

Committed: http://code.google.com/p/skia/source/detail?r=9710

[borenet, 2013-06-20 14:36:09]

This is a first step toward running Valgrind on the bots
(https://code.google.com/p/skia/issues/detail?id=529).

https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi
File gyp/common_conditions.gypi (right):

https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi#ne...
gyp/common_conditions.gypi:338: 'cflags': ['-O2'],
I didn't mess with this.

https://codereview.chromium.org/17370005/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (left):

https://codereview.chromium.org/17370005/diff/1/gyp/common_variables.gypi#old...
gyp/common_variables.gypi:131: 'skia_cros_target%': '',
Cleanup: This isn't needed any more.

[epoger, 2013-06-20 18:25:55]

https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi
File gyp/common_conditions.gypi (right):

https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi#ne...
gyp/common_conditions.gypi:79: 'Optimization':
'<(skia_release_optimization_level)', # full (/Ox)
please delete the comment, which pertains to the value '3' specifically

More importantly, do we have any guarantee that the MSVS and GCC optimization
levels will move in concert?  (I think it may just be a happy accident that they
are both set to '3' right now...)

[epoger, 2013-06-20 18:26:38]

P.S. You might consider making bungeman the lead reviewer on this; IIRC he is
fairly knowledgable about both gyp and MSVS flags.

[borenet, 2013-06-20 18:29:28]

Adding Ben.

[bungeman-skia, 2013-06-20 19:19:35]

Is the idea to have an option for a release build with symbols and no
optimization?

https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi
File gyp/common_conditions.gypi (left):

https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi#ol...
gyp/common_conditions.gypi:140: 'cflags': ['-O3 -g'],
Huh, I thought the last time we mucked with this we set this to 2. In modern gcc
-O2 is often faster that -O3 just because the code size is smaller. I have no
idea if we've actually tested on Skia though.

https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi
File gyp/common_conditions.gypi (right):

https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi#ne...
gyp/common_conditions.gypi:79: 'Optimization':
'<(skia_release_optimization_level)', # full (/Ox)
On 2013/06/20 18:25:55, epoger wrote:
> please delete the comment, which pertains to the value '3' specifically
> 
> More importantly, do we have any guarantee that the MSVS and GCC optimization
> levels will move in concert?  (I think it may just be a happy accident that
they
> are both set to '3' right now...)

gcc uses 0 (none), 1 (basic, local), 2 (everything that's spec compliant and
doesn't make code bigger), 3 (stuff that might make the code bigger / slower)

clang is like gcc, but adds 4 (use ltcg)

the values for msvs are 0 (none), 1 (size), 2 (speed), 3 (full).

Is the idea to have a flag which compiles without optimization? That would be an
easier conditional.

[borenet, 2013-06-20 19:24:12]

On 2013/06/20 19:19:35, bungeman1 wrote:
> Is the idea to have an option for a release build with symbols and no
> optimization?
> 

I want to build release with -O1 so that I can run valgrind in a reasonable
amount of time.

> https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi
> File gyp/common_conditions.gypi (left):
> 
>
https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi#ol...
> gyp/common_conditions.gypi:140: 'cflags': ['-O3 -g'],
> Huh, I thought the last time we mucked with this we set this to 2. In modern
gcc
> -O2 is often faster that -O3 just because the code size is smaller. I have no
> idea if we've actually tested on Skia though.
> 
> https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi
> File gyp/common_conditions.gypi (right):
> 
>
https://codereview.chromium.org/17370005/diff/1/gyp/common_conditions.gypi#ne...
> gyp/common_conditions.gypi:79: 'Optimization':
> '<(skia_release_optimization_level)', # full (/Ox)
> On 2013/06/20 18:25:55, epoger wrote:
> > please delete the comment, which pertains to the value '3' specifically
> > 
> > More importantly, do we have any guarantee that the MSVS and GCC
optimization
> > levels will move in concert?  (I think it may just be a happy accident that
> they
> > are both set to '3' right now...)
> 
> gcc uses 0 (none), 1 (basic, local), 2 (everything that's spec compliant and
> doesn't make code bigger), 3 (stuff that might make the code bigger / slower)
> 
> clang is like gcc, but adds 4 (use ltcg)
> 
> the values for msvs are 0 (none), 1 (size), 2 (speed), 3 (full).
> 
> Is the idea to have a flag which compiles without optimization? That would be
an
> easier conditional.

Maybe it would be better to have a "skia_valgrind" variable and switch things
based on that?

[borenet, 2013-06-20 19:51:31]

Uploaded patch set 2, which gives different defaults for Visual Studio and GCC
(and similar compilers).

[bungeman-skia, 2013-06-20 20:08:51]

Aside from adding a few small comments, lgtm.

https://codereview.chromium.org/17370005/diff/9001/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/17370005/diff/9001/gyp/common_variables.gypi#...
gyp/common_variables.gypi:97: 'skia_default_vs_optimization_level': 3,
nit: can we put the comment '# full (/Ox)' back here (it's nice to have some
confirmation of what '3' means short of digging through msdn docs.

https://codereview.chromium.org/17370005/diff/9001/gyp/common_variables.gypi#...
gyp/common_variables.gypi:98: 'skia_default_gcc_optimization_level': 3,
nit: comment '# -O3

[borenet, 2013-06-20 20:27:12]

https://codereview.chromium.org/17370005/diff/9001/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/17370005/diff/9001/gyp/common_variables.gypi#...
gyp/common_variables.gypi:97: 'skia_default_vs_optimization_level': 3,
On 2013/06/20 20:08:51, bungeman1 wrote:
> nit: can we put the comment '# full (/Ox)' back here (it's nice to have some
> confirmation of what '3' means short of digging through msdn docs.

Done.

https://codereview.chromium.org/17370005/diff/9001/gyp/common_variables.gypi#...
gyp/common_variables.gypi:98: 'skia_default_gcc_optimization_level': 3,
On 2013/06/20 20:08:51, bungeman1 wrote:
> nit: comment '# -O3

Done.

[commit-bot: I haz the power, 2013-06-20 20:27:37]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/borenet@google.com/17370005/14001

[commit-bot: I haz the power, 2013-06-20 20:28:59]

Change committed as 9710