Use thumb versions of external libraries with ARMv7.

Use thumb versions of external libraries with ARMv7.

Previously, we were using the non-thumb versions of these libraries.
This should save around 60 kiB in binary size.
This also includes some minor style fix in build/common.gypi

Committed: https://crrev.com/3572927ce7c9ce6d528820f9575a93985f9309bf
Cr-Commit-Position: refs/heads/master@{#303015}

[Fabrice (no longer in Chrome), 2014-11-03 16:20:23]

fdegans@chromium.org changed reviewers:
+ brettw@chromium.org, pasko@chromium.org, thakis@chromium.org

[Fabrice (no longer in Chrome), 2014-11-03 16:23:02]

PTAL
I think we should get a small improvement in general performance due to the lack
of ARM/Thumb switching.
I had to move the ABI section earlier in the gn version because the
arm_use_thumb definition is in build/config/arm.gni.

[pasko, 2014-11-03 16:47:15]

Cool finding!

This small change is certainly worth 60K binary size savings even if performance
benefits are negligible. It can't possibly get slower :)

l-g-t-m modulo gyp code reuse question below.

https://codereview.chromium.org/695333003/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/695333003/diff/1/build/common.gypi#newcode4640
build/common.gypi:4640: '<!(<(android_toolchain)/*-gcc -mthumb
-print-libgcc-file-name)',
I suspect we can expand a variable here to be either '-mthumb' or ''. A variable
can be named something like 'thumb_mode_gcc'.

I'd prefer that to avoid copying the -l{c,dl,m} etc all over.

[Fabrice (no longer in Chrome), 2014-11-03 18:00:41]

Thanks for the quick review!
I added yet another gyp variable, which allows us to remove a few conditionals.

https://codereview.chromium.org/695333003/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/695333003/diff/1/build/common.gypi#newcode4640
build/common.gypi:4640: '<!(<(android_toolchain)/*-gcc -mthumb
-print-libgcc-file-name)',
On 2014/11/03 16:47:15, pasko wrote:
> I suspect we can expand a variable here to be either '-mthumb' or ''. A
variable
> can be named something like 'thumb_mode_gcc'.
> 
> I'd prefer that to avoid copying the -l{c,dl,m} etc all over.

Done.

[Nico, 2014-11-03 18:03:54]

common.gypi looks somewhat wrong to me, but I'm not sure.

https://codereview.chromium.org/695333003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/695333003/diff/20001/build/common.gypi#newcod...
build/common.gypi:1826: 'arm_thumb%': 1,
Does this still work?

https://codereview.chromium.org/695333003/diff/20001/build/common.gypi#newcod...
build/common.gypi:2268: 'arm_thumb%': 0,
Isn't all the arm_thumb stuff here now one scope to far out?

[Fabrice (no longer in Chrome), 2014-11-03 18:31:22]

Thanks, I don't know know what I was thinking. Patch set 3 should work now.
I did some very minor style fix in common.gypi too, with a missing blank line
and a comment not aligned properly.

https://codereview.chromium.org/695333003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/695333003/diff/20001/build/common.gypi#newcod...
build/common.gypi:2268: 'arm_thumb%': 0,
On 2014/11/03 18:03:54, Nico wrote:
> Isn't all the arm_thumb stuff here now one scope to far out?

Blasted! You are right, gyp is confusing :/
The alternative is to nest everything further than it already is.
Adding the new variable near the libraries to avoid duplication.

[Nico, 2014-11-03 18:37:58]

gyp lgtm

[pasko, 2014-11-03 18:46:23]

lgtm, thanks!

[Fabrice (no longer in Chrome), 2014-11-05 11:55:30]

Brett, friendly ping.

[brettw, 2014-11-05 18:58:41]

gn lgtm

[Fabrice (no longer in Chrome), 2014-11-05 19:04:10]

The CQ bit was checked by fdegans@chromium.org

[commit-bot: I haz the power, 2014-11-05 19:05:08]

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

[commit-bot: I haz the power, 2014-11-05 21:01:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-05 21:01:11]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[Fabrice (no longer in Chrome), 2014-11-06 12:20:58]

The CQ bit was checked by fdegans@chromium.org

[commit-bot: I haz the power, 2014-11-06 12:21:23]

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

[commit-bot: I haz the power, 2014-11-06 12:28:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-06 12:28:52]

Patchset 3 (id:??) landed as
https://crrev.com/3572927ce7c9ce6d528820f9575a93985f9309bf
Cr-Commit-Position: refs/heads/master@{#303015}