Use gcc by default for gyp untrusted ARM builds

Use gcc by default for gyp untrusted ARM builds.

This is needed to get working .nexe test files out
of a chromium ARM build.

The old (broken) path can still be enabled using the
new force_arm_pnacl option.  This path is needed to
build the pnacl ARM shims.

This required a chrome-side change to land there
and not break the build:

https://codereview.chromium.org/13129002

BUG=196627
TEST=run nacl brower tests one ARM linux

Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=11120

[Sam Clegg, 2013-03-28 00:22:01]

Any tips on how to make this change not require an immediate corresponding
chrome change?

I looked at a few options but couldn't find one that made sense.

[Sam Clegg, 2013-03-28 00:23:23]

On 2013/03/28 00:22:01, Sam Clegg wrote:
> Any tips on how to make this change not require an immediate corresponding
> chrome change?
> 
> I looked at a few options but couldn't find one that made sense.

The -Wno-unused-local-typedefs is needed to continue to compile with -Wall and
gcc 4.8.

[Derek Schuff, 2013-03-28 17:33:41]

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi
File build/untrusted.gypi (right):

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi#newcode401
build/untrusted.gypi:401: # pnacl ARM build (only used if force_arm_pnacl is
set)
in the future we may be able to remove the pnacl ARM build since the shims are
just libraries. Maybe save for a future cleanup CL though. Probably we can make
the shims always built with pnacl (instead of building them with nacl-gcc on
x86-64 and pnacl on arm)

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi#newcode437
build/untrusted.gypi:437: '--link_flags=-arch arm --pnacl-allow-translate
--pnacl-allow-native -Wl,--pnacl-irt-link
-B<(SHARED_INTERMEDIATE_DIR)/tc_newlib/libarm ^(link_flags) >(_link_flags)',
pnacl-irt-link should no longer be necessary, especially since the shims are
just archives with native .o files (i suspect this is just a leftover from
previous testing)

https://codereview.chromium.org/12499011/diff/1/src/untrusted/irt/irt.gyp
File src/untrusted/irt/irt.gyp (right):

https://codereview.chromium.org/12499011/diff/1/src/untrusted/irt/irt.gyp#new...
src/untrusted/irt/irt.gyp:124: '-mtp=soft'
seems odd that this is here for arm but the corresponding flags are not here for
x86. does this duplicate something in untrusted.gypi?

[Roland McGrath, 2013-03-28 17:37:54]

https://codereview.chromium.org/12499011/diff/1/src/untrusted/irt/irt.gyp
File src/untrusted/irt/irt.gyp (right):

https://codereview.chromium.org/12499011/diff/1/src/untrusted/irt/irt.gyp#new...
src/untrusted/irt/irt.gyp:124: '-mtp=soft'
On 2013/03/28 17:33:41, Derek Schuff wrote:
> seems odd that this is here for arm but the corresponding flags are not here
for
> x86. does this duplicate something in untrusted.gypi?

See newlib_tls_flags in untrusted.gypi; that uses it for all builds using
newlib, when it's only really needed or desireable for building components of
the IRT.

[Derek Schuff, 2013-03-28 17:49:31]

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi
File build/untrusted.gypi (right):

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi#newcode351
build/untrusted.gypi:351: '--compile_flags=-Wno-unused-local-typedefs -Wno-psabi
-mtp=soft ^(gcc_compile_flags) >(_gcc_compile_flags) ^(compile_flags)
>(_compile_flags)',
putting this here is the equivalent of x86's use of newlib_tls_flags in every
newlib build, which as Roland noted is undesirable. The reason newlib_tls_flags
is on every newlib build is because it was easier than putting it on every
library linked into the IRT.

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi#newcode766
build/untrusted.gypi:766: # equivalent arm option is apparently -mtp=soft but we
don't need use
Maybe change this comment to
"This option is currently only used by x86/x64 builds. The equivalent ARM option
is -mtp=soft, but we only use it on the IRT and its libraries instead of
everywhere."

https://codereview.chromium.org/12499011/diff/1/src/untrusted/irt/irt.gyp
File src/untrusted/irt/irt.gyp (right):

https://codereview.chromium.org/12499011/diff/1/src/untrusted/irt/irt.gyp#new...
src/untrusted/irt/irt.gyp:124: '-mtp=soft'
Got it. so this should only be here, and in whatever nlibs get linked into the
IRT (e.g. gio_lib, platform_lib, srpc_lib etc as listed in the dependencies
above)

[Derek Schuff, 2013-03-28 17:51:16]

btw the reason the TLS flag wasn't needed everywhere for pnacl was that all the
object files were bitcode, and the flag only applies at translation time, so it
was just passed to the translator in the IRT link line.

[Roland McGrath, 2013-03-28 18:00:06]

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi
File build/untrusted.gypi (right):

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi#newcode766
build/untrusted.gypi:766: # equivalent arm option is apparently -mtp=soft but we
don't need use
On 2013/03/28 17:49:31, Derek Schuff wrote:
> Maybe change this comment to
> "This option is currently only used by x86/x64 builds. The equivalent ARM
option
> is -mtp=soft, but we only use it on the IRT and its libraries instead of
> everywhere."

IMHO we should be consistent.  -mtp=soft has exactly the same meaning and
issues as -mtls-use-call.  Either it's too much hassle to selectively use
it in only the places it's needed, or it's worth it to avoid the
de-optimization of other things (SDK libraries and PPAPI tests, I guess).
The question doesn't differ between ARM and x86.

Note that the test that verifies that no place that mattered was missing
this option (by examining the IRT binary) got disabled in Chrome because it
was being too slow on Windows and annoying Chrome developers.  So
conservatism says we should not do a more-specific enabling of the option,
which is subject to possibly missing a spot, before we have that test
re-enabled in some fashion.

[Sam Clegg, 2013-03-28 23:02:59]

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi
File build/untrusted.gypi (right):

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi#newcode401
build/untrusted.gypi:401: # pnacl ARM build (only used if force_arm_pnacl is
set)
On 2013/03/28 17:33:41, Derek Schuff wrote:
> in the future we may be able to remove the pnacl ARM build since the shims are
> just libraries. Maybe save for a future cleanup CL though. Probably we can
make
> the shims always built with pnacl (instead of building them with nacl-gcc on
> x86-64 and pnacl on arm)

Yup.  We should be able to completely remove this block, but I'll leave that for
a separate CL if thats OK.

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi#newcode437
build/untrusted.gypi:437: '--link_flags=-arch arm --pnacl-allow-translate
--pnacl-allow-native -Wl,--pnacl-irt-link
-B<(SHARED_INTERMEDIATE_DIR)/tc_newlib/libarm ^(link_flags) >(_link_flags)',
On 2013/03/28 17:33:41, Derek Schuff wrote:
> pnacl-irt-link should no longer be necessary, especially since the shims are
> just archives with native .o files (i suspect this is just a leftover from
> previous testing)

I'm not sure its worth messing with this block right now
given that its going to be removed soon anyway.

https://codereview.chromium.org/12499011/diff/1/build/untrusted.gypi#newcode766
build/untrusted.gypi:766: # equivalent arm option is apparently -mtp=soft but we
don't need use
On 2013/03/28 18:00:06, Roland McGrath wrote:
> On 2013/03/28 17:49:31, Derek Schuff wrote:
> > Maybe change this comment to
> > "This option is currently only used by x86/x64 builds. The equivalent ARM
> option
> > is -mtp=soft, but we only use it on the IRT and its libraries instead of
> > everywhere."
> 
> IMHO we should be consistent.  -mtp=soft has exactly the same meaning and
> issues as -mtls-use-call.  Either it's too much hassle to selectively use
> it in only the places it's needed, or it's worth it to avoid the
> de-optimization of other things (SDK libraries and PPAPI tests, I guess).
> The question doesn't differ between ARM and x86.
> 
> Note that the test that verifies that no place that mattered was missing
> this option (by examining the IRT binary) got disabled in Chrome because it
> was being too slow on Windows and annoying Chrome developers.  So
> conservatism says we should not do a more-specific enabling of the option,
> which is subject to possibly missing a spot, before we have that test
> re-enabled in some fashion.

Done. Made it consistent.

https://codereview.chromium.org/12499011/diff/1/src/untrusted/irt/irt.gyp
File src/untrusted/irt/irt.gyp (right):

https://codereview.chromium.org/12499011/diff/1/src/untrusted/irt/irt.gyp#new...
src/untrusted/irt/irt.gyp:124: '-mtp=soft'
On 2013/03/28 17:49:31, Derek Schuff wrote:
> Got it. so this should only be here, and in whatever nlibs get linked into the
> IRT (e.g. gio_lib, platform_lib, srpc_lib etc as listed in the dependencies
> above)

Done.  Went for consistency with x86 which is to say that all newlib code built
by gyp has this flag now.

[Sam Clegg, 2013-04-01 17:41:21]

ping.

[Derek Schuff, 2013-04-01 17:48:55]

On 2013/04/01 17:41:21, Sam Clegg wrote:
> ping.

sorry, I was out on Friday. LGTM now.

[Sam Clegg, 2013-04-02 17:36:24]

On 2013/04/01 17:48:55, Derek Schuff wrote:
> On 2013/04/01 17:41:21, Sam Clegg wrote:
> > ping.
> 
> sorry, I was out on Friday. LGTM now.

Please take another look.  I updated irt core build which was not caught by the
chromium try jobs.

[Derek Schuff, 2013-04-02 17:39:29]

still looks good.

[commit-bot: I haz the power, 2013-04-02 17:45:15]

CQ is trying da patch. Follow status at
https://nativeclient-status.appspot.com/cq/sbc@chromium.org/12499011/54001

[commit-bot: I haz the power, 2013-04-02 19:39:27]

Change committed as 11120

[Mark Seaborn, 2013-04-04 15:07:39]

Since the Chromium-side change caused a build failure and was reverted, do we
need to revert this NaCl-side change too, so that it's not blocking Chromium
DEPS updates?

[Sam Clegg, 2013-04-04 16:05:57]

On 2013/04/04 15:07:39, Mark Seaborn wrote:
> Since the Chromium-side change caused a build failure and was reverted, do we
> need to revert this NaCl-side change too, so that it's not blocking Chromium
> DEPS updates?

Looks like the chrome side breakage was the missing arm toolchain on chromeos
builds.  I can probably find a simple fix for that today.  If it can't wait then
obviously we can revert this in the mean time.