Don't use m32 cflag for x86_64.

Don't use m32 cflag for x86_64.

When checking the skia_arch_type for "x86", instead of doing an
== compare, check if "x86" in skia_arch_type, so it will cover
both x86 and x86_64.

Except when we specifically want x86.

Set skia_arch_width based on "64" in skia_arch_type. No need to specify
in scripts.

In gyp_to_android.py, create a separate var_dict for x86_64.

BUG=skia:3419
Committed: https://skia.googlesource.com/skia/+/df1c3373fc198f465cb0077516993dd41f268416

[scroggo, 2015-02-11 18:01:51]

scroggo@google.com changed reviewers:
+ djsollen@google.com, mtklein@google.com

[scroggo, 2015-02-11 18:01:52]

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

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:232: '-m64',
In a follow-up CL, I'll condition this on !skia_android_framework.

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:631: [ '"x86" in skia_arch_type', {
Same here

https://codereview.chromium.org/916113002/diff/1/platform_tools/android/bin/g...
File platform_tools/android/bin/gyp_to_android.py (right):

https://codereview.chromium.org/916113002/diff/1/platform_tools/android/bin/g...
platform_tools/android/bin/gyp_to_android.py:110: x86_64_var_dict =
generate_var_dict(tmp_folder, main_gyp_file, 'x86_64',
This whole section needs to be cleaned up, which I'll do in a separate CL.

[djsollen, 2015-02-11 18:11:23]

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

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:230: [ 'skia_arch_width == 64 and "x86" in
skia_arch_type', {
skia_arch_type == "x86_64" seems more concise.

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:631: [ '"x86" in skia_arch_type', {
On 2015/02/11 18:01:52, scroggo wrote:
> Same here

Yes please!

[mtklein, 2015-02-11 18:15:05]

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

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:31: [ '"x86" in skia_arch_type', {
You can probably leave this as-is.  These are both the default for x86_64
builds.

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:230: [ 'skia_arch_width == 64 and "x86" in
skia_arch_type', {
Can't this just be
  if skia_arch_type == "x86_64"
or am I missing the point?

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:238: [ 'skia_arch_width == 32 and "x86" in
skia_arch_type', {
And this 
   if skia_arch_type == "x86"
?

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:631: [ '"x86" in skia_arch_type', {
This one seems right to me.  That's something we need to turn on explicitly for
both x86 and x86_64.

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi#ne...
gyp/common_variables.gypi:85: [ 'skia_os in ["linux", "freebsd", "openbsd",
"solaris", "mac"] or "64" in skia_arch_type', {
So are we going to get "x86" and "x86_64"  as skia_arch_type consistently
through all the builds, or only for Android builds?  Am I thinking right that on
desktops we'll still see "x86" and skia_arch_width=64?  That seems pretty
confusing.

https://codereview.chromium.org/916113002/diff/1/gyp/opts.gyp
File gyp/opts.gyp (right):

https://codereview.chromium.org/916113002/diff/1/gyp/opts.gyp#newcode48
gyp/opts.gyp:48: or (skia_os == "android" and skia_arch_type not in ["arm",
"mips", "arm64"] and "x86" not in skia_arch_type)', {
seems more straightforward to just write

["arm", "mips", "arm64", "x86", "x86_64"]

?

[scroggo, 2015-02-11 20:12:10]

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

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:31: [ '"x86" in skia_arch_type', {
On 2015/02/11 18:15:05, mtklein wrote:
> You can probably leave this as-is.  These are both the default for x86_64
> builds.

Done.

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:230: [ 'skia_arch_width == 64 and "x86" in
skia_arch_type', {
On 2015/02/11 18:15:05, mtklein wrote:
> Can't this just be
>   if skia_arch_type == "x86_64"
> or am I missing the point?

No, you're correct. I caught this in my follow-up. Adding here.

https://codereview.chromium.org/916113002/diff/1/gyp/common_conditions.gypi#n...
gyp/common_conditions.gypi:238: [ 'skia_arch_width == 32 and "x86" in
skia_arch_type', {
On 2015/02/11 18:15:05, mtklein wrote:
> And this 
>    if skia_arch_type == "x86"
> ?

Done.

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi#ne...
gyp/common_variables.gypi:85: [ 'skia_os in ["linux", "freebsd", "openbsd",
"solaris", "mac"] or "64" in skia_arch_type', {
On 2015/02/11 18:15:05, mtklein wrote:
> So are we going to get "x86" and "x86_64"  as skia_arch_type consistently
> through all the builds, or only for Android builds? 

I would like it to be the case for all builds (although I have no control over
people who manually set their gyp defines... Do we ever do that on the bots?).

> Am I thinking right that on
> desktops we'll still see "x86" and skia_arch_width=64?  That seems pretty
> confusing.

I think you are correct.

If we were consistent, this line could just say

['"64" in skia_arch_type',

If I understand this nesting process, that would mean we'd need to set the
default to x86_64 in the variables 3 dict above for these OSs.

Which means I'd need to add one deeper level which defines skia_os, and in level
3 do this check to switch to x86_64. Does that sound correct?

https://codereview.chromium.org/916113002/diff/1/gyp/opts.gyp
File gyp/opts.gyp (right):

https://codereview.chromium.org/916113002/diff/1/gyp/opts.gyp#newcode48
gyp/opts.gyp:48: or (skia_os == "android" and skia_arch_type not in ["arm",
"mips", "arm64"] and "x86" not in skia_arch_type)', {
On 2015/02/11 18:15:05, mtklein wrote:
> seems more straightforward to just write
> 
> ["arm", "mips", "arm64", "x86", "x86_64"]
> 
> ?

That seems fine to me. Done.

[scroggo, 2015-02-11 20:15:26]

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi#ne...
gyp/common_variables.gypi:85: [ 'skia_os in ["linux", "freebsd", "openbsd",
"solaris", "mac"] or "64" in skia_arch_type', {
On 2015/02/11 20:12:10, scroggo wrote:
> On 2015/02/11 18:15:05, mtklein wrote:
> > So are we going to get "x86" and "x86_64"  as skia_arch_type consistently
> > through all the builds, or only for Android builds? 
> 
> I would like it to be the case for all builds (although I have no control over
> people who manually set their gyp defines... Do we ever do that on the bots?).
> 
> > Am I thinking right that on
> > desktops we'll still see "x86" and skia_arch_width=64?  That seems pretty
> > confusing.
> 
> I think you are correct.
> 
> If we were consistent, this line could just say
> 
> ['"64" in skia_arch_type',
> 
> If I understand this nesting process, that would mean we'd need to set the
> default to x86_64 in the variables 3 dict above for these OSs.
> 
> Which means I'd need to add one deeper level which defines skia_os, and in
level
> 3 do this check to switch to x86_64. Does that sound correct?

And now of course I'm not sure of that. It appears that we already both set and
condtion on skia_os in level 2...

[scroggo, 2015-02-12 15:35:46]

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi#ne...
gyp/common_variables.gypi:85: [ 'skia_os in ["linux", "freebsd", "openbsd",
"solaris", "mac"] or "64" in skia_arch_type', {
On 2015/02/11 20:15:26, scroggo wrote:
> On 2015/02/11 20:12:10, scroggo wrote:
> > On 2015/02/11 18:15:05, mtklein wrote:
> > > So are we going to get "x86" and "x86_64"  as skia_arch_type consistently
> > > through all the builds, or only for Android builds? 
> > 
> > I would like it to be the case for all builds (although I have no control
over
> > people who manually set their gyp defines... Do we ever do that on the
bots?).
> > 
> > > Am I thinking right that on
> > > desktops we'll still see "x86" and skia_arch_width=64?  That seems pretty
> > > confusing.
> > 
> > I think you are correct.
> > 
> > If we were consistent, this line could just say
> > 
> > ['"64" in skia_arch_type',
> > 
> > If I understand this nesting process, that would mean we'd need to set the
> > default to x86_64 in the variables 3 dict above for these OSs.
> > 
> > Which means I'd need to add one deeper level which defines skia_os, and in
> level
> > 3 do this check to switch to x86_64. Does that sound correct?
> 
> And now of course I'm not sure of that. It appears that we already both set
and
> condtion on skia_os in level 2...

I made that change in https://codereview.chromium.org/919003002, which builds on
this one. PTAL.

[scroggo, 2015-02-12 15:55:47]

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/916113002/diff/1/gyp/common_variables.gypi#ne...
gyp/common_variables.gypi:85: [ 'skia_os in ["linux", "freebsd", "openbsd",
"solaris", "mac"] or "64" in skia_arch_type', {
On 2015/02/12 15:35:45, scroggo wrote:
> On 2015/02/11 20:15:26, scroggo wrote:
> > On 2015/02/11 20:12:10, scroggo wrote:
> > > On 2015/02/11 18:15:05, mtklein wrote:
> > > > So are we going to get "x86" and "x86_64"  as skia_arch_type
consistently
> > > > through all the builds, or only for Android builds? 
> > > 
> > > I would like it to be the case for all builds (although I have no control
> over
> > > people who manually set their gyp defines... Do we ever do that on the
> bots?).
> > > 
> > > > Am I thinking right that on
> > > > desktops we'll still see "x86" and skia_arch_width=64?  That seems
pretty
> > > > confusing.
> > > 
> > > I think you are correct.
> > > 
> > > If we were consistent, this line could just say
> > > 
> > > ['"64" in skia_arch_type',
> > > 
> > > If I understand this nesting process, that would mean we'd need to set the
> > > default to x86_64 in the variables 3 dict above for these OSs.
> > > 
> > > Which means I'd need to add one deeper level which defines skia_os, and in
> > level
> > > 3 do this check to switch to x86_64. Does that sound correct?
> > 
> > And now of course I'm not sure of that. It appears that we already both set
> and
> > condtion on skia_os in level 2...
> 
> I made that change in https://codereview.chromium.org/919003002, which builds
on
> this one. PTAL.

Pulled in that change here. PTAL.

[djsollen, 2015-02-12 16:12:31]

lgtm

[scroggo, 2015-02-12 17:44:53]

The CQ bit was checked by scroggo@google.com

[commit-bot: I haz the power, 2015-02-12 17:45:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/916113002/80001

[mtklein, 2015-02-12 17:46:34]

The CQ bit was unchecked by mtklein@google.com

[mtklein, 2015-02-12 17:47:29]

This LGTM, but I think we need to land some bot changes before we can land it.

Just trying this out on my local linux machine, this will make the 32-bit bots
64-bit.  I think we need to switch them over to ./gyp_skia -Dskia_arch_width=32
-Dskia_arch_type=x86 first, land this, then remove the -Dskia_arch_width=32.

[mtklein, 2015-02-12 18:47:38]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-02-12 18:47:44]

Here we go!

[commit-bot: I haz the power, 2015-02-12 18:48:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/916113002/80001

[commit-bot: I haz the power, 2015-02-12 18:48:31]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/df1c3373fc198f465cb0077516993dd41f268416