Turn off SSE2 when building for ARM or MIPS.

Turn off SSE2 when building for ARM or MIPS.

Disable SSE2 when building the code for ARM or MIPS.

BUG= https://code.google.com/p/chromium/issues/detail?id=130022
TEST=make chrome

[petarj, 2012-09-18 20:22:33]

Small change to qcms.gyp to set qcms_use_sse to zero for MIPS.

[tony, 2012-09-18 20:35:42]

I think it would be better if in build/common.gypi, we added a condition that
forces disable_sse2=1 if target_arch=="mipsel" or target_arch=="arm".

[Lei Zhang, 2012-09-18 20:37:00]

On 2012/09/18 20:35:42, tony wrote:
> I think it would be better if in build/common.gypi, we added a condition that
> forces disable_sse2=1 if target_arch=="mipsel" or target_arch=="arm".

Indeed. There's no reason for these architectures to use SSE in any other part
of the code base.

[petarj, 2012-09-21 17:53:38]

On 2012/09/18 20:37:00, Lei Zhang wrote:
> On 2012/09/18 20:35:42, tony wrote:
> > I think it would be better if in build/common.gypi, we added a condition
that
> > forces disable_sse2=1 if target_arch=="mipsel" or target_arch=="arm".
> 
> Indeed. There's no reason for these architectures to use SSE in any other part
> of the code base.

The change modified to set disable_sse2 for ARM or MIPS in build/common.gypi.

[Lei Zhang, 2012-09-21 18:21:16]

lgtm

[tony, 2012-09-21 19:29:19]

LGTM

http://codereview.chromium.org/10947012/diff/3001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10947012/diff/3001/build/common.gypi#newcode1179
build/common.gypi:1179: 'disable_sse2%': 0,
Nit: I think you can delete the disable_sse2% variable on line 202 since this
will replace it.

[tony, 2012-09-21 19:30:05]

Also, let us know if you want someone to commit this change for you.

[petarj, 2012-09-21 21:19:42]

@tony
> Also, let us know if you want someone to commit this change for you.

Does CQ work for these files?
Whatever is easier/quicker, it's fine by me.

https://chromiumcodereview.appspot.com/10947012/diff/3001/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/10947012/diff/3001/build/common.gypi#n...
build/common.gypi:1179: 'disable_sse2%': 0,
On 2012/09/21 19:29:19, tony wrote:
> Nit: I think you can delete the disable_sse2% variable on line 202 since this
> will replace it.

If I do that, gyp complains with:
"gyp: Undefined variable disable_sse2 in build/all.gyp while trying to load
build/all.gyp".

[tony, 2012-09-21 21:27:55]

https://chromiumcodereview.appspot.com/10947012/diff/3001/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/10947012/diff/3001/build/common.gypi#n...
build/common.gypi:1179: 'disable_sse2%': 0,
On 2012/09/21 21:19:42, petarj wrote:
> On 2012/09/21 19:29:19, tony wrote:
> > Nit: I think you can delete the disable_sse2% variable on line 202 since
this
> > will replace it.
> 
> If I do that, gyp complains with:
> "gyp: Undefined variable disable_sse2 in build/all.gyp while trying to load
> build/all.gyp".

Ah, you're right.  I would delete the lines around 202 and move this condition
into the conditions block around like 388.  See the use_skia% variable as an
example.

[petarj, 2012-09-21 22:02:21]

https://chromiumcodereview.appspot.com/10947012/diff/3001/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/10947012/diff/3001/build/common.gypi#n...
build/common.gypi:1179: 'disable_sse2%': 0,
On 2012/09/21 21:27:55, tony wrote:
> On 2012/09/21 21:19:42, petarj wrote:
> > On 2012/09/21 19:29:19, tony wrote:
> > > Nit: I think you can delete the disable_sse2% variable on line 202 since
> this
> > > will replace it.
> > 
> > If I do that, gyp complains with:
> > "gyp: Undefined variable disable_sse2 in build/all.gyp while trying to load
> > build/all.gyp".
> 
> Ah, you're right.  I would delete the lines around 202 and move this condition
> into the conditions block around like 388.  See the use_skia% variable as an
> example.

I was actually experimenting with this one before, but that did not work since
"target_arch" was not defined there.
I have just double checked, and here is what I get:

NameError: name 'target_arch' is not defined while evaluating condition
'target_arch=="arm" or target_arch=="mipsel"' in build/all.gyp while trying to
load build/all.gyp

[tony, 2012-09-21 22:40:45]

https://chromiumcodereview.appspot.com/10947012/diff/3001/build/common.gypi
File build/common.gypi (right):

https://chromiumcodereview.appspot.com/10947012/diff/3001/build/common.gypi#n...
build/common.gypi:1179: 'disable_sse2%': 0,
On 2012/09/21 22:02:21, petarj wrote:
> I was actually experimenting with this one before, but that did not work since
> "target_arch" was not defined there.
> I have just double checked, and here is what I get:
> 
> NameError: name 'target_arch' is not defined while evaluating condition
> 'target_arch=="arm" or target_arch=="mipsel"' in build/all.gyp while trying to
> load build/all.gyp

I see, target_arch is defined at the same level as disable_sse2.  I guess this
is as good as it will get.

Sorry for the runaround. Thanks for trying all my requests.

LGTM

[commit-bot: I haz the power, 2012-09-21 22:41:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/petarj@mips.com/10947012/3001

[commit-bot: I haz the power, 2012-09-21 22:54:27]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[tony, 2012-09-21 23:11:13]

On 2012/09/21 22:54:27, I haz the power (commit-bot) wrote:
> Sorry for I got bad news for ya.
> Compile failed with a clobber build.
> Your code is likely broken or HEAD is junk. Please ensure your
> code is not broken then alert the build sheriffs.
> Look at the try server FAQ for more details.

Hmm, this failed on the android build.  Maybe disable_sse2 isn't getting set
properly?

[petarj, 2012-09-21 23:42:29]

On 2012/09/21 23:11:13, tony wrote:
> On 2012/09/21 22:54:27, I haz the power (commit-bot) wrote:
> > Sorry for I got bad news for ya.
> > Compile failed with a clobber build.
> > Your code is likely broken or HEAD is junk. Please ensure your
> > code is not broken then alert the build sheriffs.
> > Look at the try server FAQ for more details.
> 
> Hmm, this failed on the android build.  Maybe disable_sse2 isn't getting set
> properly?

It looks to me like disable_sse2 is not defined at all when built for Android.
Does Chromium get built for Android/ARM differently (i.e. does it make use of
build/common.gypi)?

[petarj, 2012-09-22 01:55:41]

On 2012/09/21 23:11:13, tony wrote:
> On 2012/09/21 22:54:27, I haz the power (commit-bot) wrote:
> > Sorry for I got bad news for ya.
> > Compile failed with a clobber build.
> > Your code is likely broken or HEAD is junk. Please ensure your
> > code is not broken then alert the build sheriffs.
> > Look at the try server FAQ for more details.
> 
> Hmm, this failed on the android build.  Maybe disable_sse2 isn't getting set
> properly?

Setting disable_sss2 to 1 on line 202 would resolve the Android issue.
Would that be fine or that could impact something else?

[Lei Zhang, 2012-09-22 01:59:09]

On 2012/09/22 01:55:41, petarj wrote:
> On 2012/09/21 23:11:13, tony wrote:
> > On 2012/09/21 22:54:27, I haz the power (commit-bot) wrote:
> > > Sorry for I got bad news for ya.
> > > Compile failed with a clobber build.
> > > Your code is likely broken or HEAD is junk. Please ensure your
> > > code is not broken then alert the build sheriffs.
> > > Look at the try server FAQ for more details.
> > 
> > Hmm, this failed on the android build.  Maybe disable_sse2 isn't getting set
> > properly?
> 
> Setting disable_sss2 to 1 on line 202 would resolve the Android issue.
> Would that be fine or that could impact something else?

The way we use that is to enable it by default, and disable it where needed. If
you disable sse2 by default, I think layout tests are going to fail on 32-bit
Linux due to x87 math.

[petarj, 2012-09-25 01:40:58]

On 2012/09/22 01:59:09, Lei Zhang wrote:
> On 2012/09/22 01:55:41, petarj wrote:
> > On 2012/09/21 23:11:13, tony wrote:
> > > On 2012/09/21 22:54:27, I haz the power (commit-bot) wrote:
> > > > Sorry for I got bad news for ya.
> > > > Compile failed with a clobber build.
> > > > Your code is likely broken or HEAD is junk. Please ensure your
> > > > code is not broken then alert the build sheriffs.
> > > > Look at the try server FAQ for more details.
> > > 
> > > Hmm, this failed on the android build.  Maybe disable_sse2 isn't getting
set
> > > properly?
> > 
> > Setting disable_sss2 to 1 on line 202 would resolve the Android issue.
> > Would that be fine or that could impact something else?
> 
> The way we use that is to enable it by default, and disable it where needed.
If
> you disable sse2 by default, I think layout tests are going to fail on 32-bit
> Linux due to x87 math.

Tony, Lei, thanks for the help and for the fix at:
https://chromiumcodereview.appspot.com/10972012/

Petar