Repair the build for Android on IA in a x64 host platform

Repair the build for Android on IA in a x64 host platform

There are two issues here:
1) Define the macro 'V8_HOST_ARCH_IA32'
2) Add the option '-m32' for cflags and ldflags like arm

BUG=126614
TEST=

[yongsheng, 2012-05-10 01:32:25]

Jakob and Michael, could you please help review this patch? Thanks

[yongsheng, 2012-05-10 01:32:25]

Jakob and Michael, could you please help review this patch? Thanks

[yongsheng, 2012-05-11 01:29:04]

On 2012/05/10 01:32:25, yongsheng.zhu wrote:
> Jakob and Michael, could you please help review this patch? Thanks

Hi, all
could you review the patch or do you know who can help review it?
Thanks

[Jakob Kummerow, 2012-05-11 09:22:56]

You did pick the right reviewers, I just didn't get around to commenting
earlier.

I don't think we should set V8_HOST_ARCH_IA32 from the gyp file. The -m32 flag
should be enough to let the preprocessor magic in globals.h figure that out
automatically.

As for passing -m32, I'd like to fix that in a more generic way. I'll prepare a
CL for that shortly. I think the following logic should work:

When v8_target_arch is any of {'ia32', 'arm', 'mips'}, then for host and target
compiler individually figure out if they support -m32, and if yes, set it; where
the host compiler is determined by the lookup chain $CXX.host -> $CXX -> $(which
g++), and the target compiler by $CXX.target -> $CXX -> $(which g++).

[yongsheng, 2012-05-11 12:20:16]

On 2012/05/11 09:22:56, Jakob wrote:
> You did pick the right reviewers, I just didn't get around to commenting
> earlier.
> 
> I don't think we should set V8_HOST_ARCH_IA32 from the gyp file. The -m32 flag
> should be enough to let the preprocessor magic in globals.h figure that out
> automatically.
> 
> As for passing -m32, I'd like to fix that in a more generic way. I'll prepare
a
> CL for that shortly. I think the following logic should work:
> 
> When v8_target_arch is any of {'ia32', 'arm', 'mips'}, then for host and
target
> compiler individually figure out if they support -m32, and if yes, set it;
where
> the host compiler is determined by the lookup chain $CXX.host -> $CXX ->
$(which
> g++), and the target compiler by $CXX.target -> $CXX -> $(which g++).
okay, so you'll provide a patch for this issue?

[Jakob Kummerow, 2012-05-11 13:50:46]

On 2012/05/11 12:20:16, yongsheng.zhu wrote:
> okay, so you'll provide a patch for this issue?

Yes, I'm planning to, but it'll take a week or two.
If you want this fixed sooner, go ahead and do it yourself :-)

Also, in the future, please create CLs against our bleeding_edge branch rather
than trunk.

[yongsheng, 2012-05-12 00:26:22]

On 2012/05/11 13:50:46, Jakob wrote:
> On 2012/05/11 12:20:16, yongsheng.zhu wrote:
> > okay, so you'll provide a patch for this issue?
> 
> Yes, I'm planning to, but it'll take a week or two.
> If you want this fixed sooner, go ahead and do it yourself :-)
> 
> Also, in the future, please create CLs against our bleeding_edge branch rather
> than trunk.
okay, I'll do it and create a new CL against 'bleeding_edge'branch.
Thanks for your ideas.

[yongsheng, 2012-05-14 07:07:43]

On 2012/05/11 09:22:56, Jakob wrote:
> You did pick the right reviewers, I just didn't get around to commenting
> earlier.
> 
> I don't think we should set V8_HOST_ARCH_IA32 from the gyp file. The -m32 flag
> should be enough to let the preprocessor magic in globals.h figure that out
> automatically.
> 
> As for passing -m32, I'd like to fix that in a more generic way. I'll prepare
a
> CL for that shortly. I think the following logic should work:
> 
> When v8_target_arch is any of {'ia32', 'arm', 'mips'}, then for host and
target
> compiler individually figure out if they support -m32, and if yes, set it;
where
> the host compiler is determined by the lookup chain $CXX.host -> $CXX ->
$(which
> g++), and the target compiler by $CXX.target -> $CXX -> $(which g++).

One question about this, even the target arch is 64 bit, if it supports -m32,
always set this option?

[Jakob Kummerow, 2012-05-14 08:37:47]

On 2012/05/14 07:07:43, yongsheng.zhu wrote:
> One question about this, even the target arch is 64 bit, if it supports -m32,
> always set this option?

When the v8_target_arch (i.e. the code generation target architecture) is any of
ia32, arm, or mips, then the target_arch (i.e. the architecture where the
compiled binaries run) must be either the same (cross-compile or native builds)
or ia32 (simulator builds). So yes, when you're compiling the simulator to run
on a 64 bit system, you must pass -m32 to the compiler. Further, for snapshot
creation, you also must pass -m32 to the host toolset if the build host is a 64
bit system.

Supported combinations of host_arch | target_arch | v8_target_arch are:

(native)
arm | arm | arm
ia32 | ia32 | ia32
mips | mips | mips (not sure how well this is tested)
x64 | x64 | x64

(cross-compiling)
ia32 | arm | arm
ia32 | mips | mips

(simulator)
ia32 | ia32 | arm
ia32 | ia32 | mips

Since x86_64 operating systems can execute ia32 binaries, you can use 64 bit
systems as "host" and "target" in any place where ia32 is listed above, but in
*all of these cases* you must pass -m32 to the toolchain to force creation of
ia32 binaries.
Only when v8_target_arch==x64 (which is only supported when
host_arch==target_arch==x64 too), setting -m32 would be wrong.

[yongsheng, 2012-05-14 09:19:49]

got it, thanks.
Another issue needs you confirmation before I upload the patch:
I'm using $CXX_target and $CXX_host instead of $CXX.target and $CXX.host(seems
these 2 are invalid names) in the lookup chain. I find 'CXX_target' is defined
in build/android/envsetup.sh but I'm not sure whether they are suitable.
Below is the code to check the option for '-m32':
'm32flag': '<!((echo | $(echo ${CXX_host:-${CXX:-$(which g++)}}) -m32 -E - >
/dev/null 2>&1) && echo -n "-m32" || true)'
'm32flag': '<!((echo | $(echo ${CXX_target:-${CXX:-$(which g++)}}) -m32 -E - >
/dev/null 2>&1) && echo -n "-m32" || true)'

[Jakob Kummerow, 2012-05-14 17:25:38]

On 2012/05/14 09:19:49, yongsheng.zhu wrote:
> Another issue needs you confirmation before I upload the patch:
> I'm using $CXX_target and $CXX_host instead of $CXX.target and $CXX.host(seems
> these 2 are invalid names) in the lookup chain. I find 'CXX_target' is defined
> in build/android/envsetup.sh but I'm not sure whether they are suitable.

Uhm... interesting. The Makefiles generated by GYP use "CXX.target", but you're
right, bash says "export CXX.target=foo" is invalid. Oh well. Let's just use
CXX_target then. (Probably a bug in GYP's make generator, but can easily be
worked around by setting CXX instead.)

> Below is the code to check the option for '-m32':
> 'm32flag': '<!((echo | $(echo ${CXX_host:-${CXX:-$(which g++)}}) -m32 -E - >
> /dev/null 2>&1) && echo -n "-m32" || true)'
> 'm32flag': '<!((echo | $(echo ${CXX_target:-${CXX:-$(which g++)}}) -m32 -E - >
> /dev/null 2>&1) && echo -n "-m32" || true)'

Looks good. Looking forward to your patch :-)

[yongsheng, 2012-05-15 07:32:34]

> Looks good. Looking forward to your patch :-)
upload a new patch for 'bleeding_edge' branch. See
http://codereview.chromium.org/10335014/'
Will close this one.