Enable Android x64 build

Enable Android x64 build

BUG=346626
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257683

[Torne, 2014-03-11 11:15:35]

You probably want to ask rmcilroy for reviews of general x64 build stuff (I've
added him here); benm and I are only really experts on android webview issues.
Just one quick comment..

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

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1301
build/common.gypi:1301: 'gcc_version%': '<!(python
<(DEPTH)/build/compiler_version.py)',
Is this correct? We're forcing gcc 4.6 at present..

[rmcilroy, 2014-03-11 11:42:19]

Thanks Zhenyu. A couple of comments.

https://codereview.chromium.org/194843002/diff/1/build/common.gypi
File build/common.gypi (left):

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#oldcode4008
build/common.gypi:4008: '-Wl,-dynamic-linker,/system/bin/linker',
This should probably be ok, but have you tested this being removed on older
32bit platforms (e.g. ICS) to make sure it works without the explicit linker
attribute?

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

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1301
build/common.gypi:1301: 'gcc_version%': '<!(python
<(DEPTH)/build/compiler_version.py)',
On 2014/03/11 11:15:35, Torne wrote:
> Is this correct? We're forcing gcc 4.6 at present..

I think x64 requires 4.8 - is that right Zhenyu?   We should keep other Android
arches at 4.6 for now though (fdegans@ is investigating whether we can move to
4.8 without size regressions right now).  Could you make this 4.6 or 4.8
depending on architecture here please (assuming x64 actually needs 4.8).

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1476
build/common.gypi:1476: 'gcc_version%': '<!(CXX=<(android_toolchain)/*-g++
python <(DEPTH)/build/compiler_version.py)',
Why is this required?  Isn't it done above?

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode3921
build/common.gypi:3921: # Place holder for x64 support, not tested.
Add a todo associated with a bug here please (so that the todo can be removed
once it's tested).

[Zhenyu Liang, 2014-03-12 10:27:23]

https://codereview.chromium.org/194843002/diff/1/build/common.gypi
File build/common.gypi (left):

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#oldcode4008
build/common.gypi:4008: '-Wl,-dynamic-linker,/system/bin/linker',
On 2014/03/11 11:42:19, rmcilroy wrote:
> This should probably be ok, but have you tested this being removed on older
32bit platforms (e.g. ICS) to make sure it works without the explicit linker
attribute?

Yes, I had tried with the original ndk in third_party/. gcc version 4.6 can
generate proper .interp section. I think we can remove this line safely.

Quick check:
% strings arm-linux-androideabi-gcc | grep /linker

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

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1301
build/common.gypi:1301: 'gcc_version%': '<!(python
<(DEPTH)/build/compiler_version.py)',
On 2014/03/11 11:42:19, rmcilroy wrote:
> I think x64 requires 4.8 - is that right Zhenyu? We should keep other Android
arches at 4.6 for now though (fdegans@ is investigating whether we can move to
4.8 without size regressions right now). Could you make this 4.6 or 4.8
depending on architecture here please (assuming x64 actually needs 4.8).

Yeah, we are using gcc 4.8 for x64 porting.
The change means it will determine the gcc version of android toolchain later
rather than assume it. Here, the gcc_version is the version of host compiler.

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1476
build/common.gypi:1476: 'gcc_version%': '<!(CXX=<(android_toolchain)/*-g++
python <(DEPTH)/build/compiler_version.py)',
On 2014/03/11 11:42:19, rmcilroy wrote:
> Why is this required? Isn't it done above?

It determines the gcc version of android toolchain here (by using the script
with CXX environment), and overwrite the gcc_version.

[rmcilroy, 2014-03-12 11:04:07]

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

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1301
build/common.gypi:1301: 'gcc_version%': '<!(python
<(DEPTH)/build/compiler_version.py)',
On 2014/03/12 10:27:23, Zhenyu Liang wrote:
> On 2014/03/11 11:42:19, rmcilroy wrote:
> > I think x64 requires 4.8 - is that right Zhenyu? We should keep other
Android
> arches at 4.6 for now though (fdegans@ is investigating whether we can move to
> 4.8 without size regressions right now). Could you make this 4.6 or 4.8
> depending on architecture here please (assuming x64 actually needs 4.8).
> 
> Yeah, we are using gcc 4.8 for x64 porting.
> The change means it will determine the gcc version of android toolchain later
> rather than assume it. Here, the gcc_version is the version of host compiler.

Is there a reason you cannot do:
    ['OS=="android"', {
        ['target_arch=="x64"', {
           'gcc_version%': 48,	
        }, {
           'gcc_version%': 46,	
        }],
    }, {
    ....

As I suggested above?  I think this would be more obvious what it was doing.

[Zhenyu Liang, 2014-03-12 11:30:53]

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

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1301
build/common.gypi:1301: 'gcc_version%': '<!(python
<(DEPTH)/build/compiler_version.py)',
On 2014/03/12 11:04:08, rmcilroy wrote:
> 
> Is there a reason you cannot do:
>     ['OS=="android"', {
>         ['target_arch=="x64"', {
>            'gcc_version%': 48,	
>         }, {
>            'gcc_version%': 46,	
>         }],
>     }, {
>     ....
> 
> As I suggested above?  I think this would be more obvious what it was doing.

I had tried this way initially. It doesn't work. GYP complains "target_arch is
not defined". It seems that target_arch is not defined in the stage parsing
variables.

[rmcilroy, 2014-03-12 15:09:53]

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

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1301
build/common.gypi:1301: 'gcc_version%': '<!(python
<(DEPTH)/build/compiler_version.py)',
On 2014/03/12 11:30:53, Zhenyu Liang wrote:
> On 2014/03/12 11:04:08, rmcilroy wrote:
> > 
> > Is there a reason you cannot do:
> >     ['OS=="android"', {
> >         ['target_arch=="x64"', {
> >            'gcc_version%': 48,	
> >         }, {
> >            'gcc_version%': 46,	
> >         }],
> >     }, {
> >     ....
> > 
> > As I suggested above?  I think this would be more obvious what it was doing.
> 
> I had tried this way initially. It doesn't work. GYP complains "target_arch is
> not defined". It seems that target_arch is not defined in the stage parsing
> variables.

Are you sure?  When I try this myself it seems to works with no mention of
target_arch not being defined.  What is the setup you are using to build chrome?
 Are you using envsetup.sh and android_gyp?

[Zhenyu Liang, 2014-03-12 18:46:31]

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

https://codereview.chromium.org/194843002/diff/1/build/common.gypi#newcode1301
build/common.gypi:1301: 'gcc_version%': '<!(python
<(DEPTH)/build/compiler_version.py)',
On 2014/03/12 15:09:53, rmcilroy wrote:
> 
> Are you sure?  When I try this myself it seems to works with no mention of
> target_arch not being defined.  What is the setup you are using to build
chrome?
>  Are you using envsetup.sh and android_gyp?

Sorry, I misremembered that _toolset is what I tried.
target_arch is OK. Updated.

[rmcilroy, 2014-03-12 19:27:30]

lgtm with one final comment.  Thanks.

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

https://codereview.chromium.org/194843002/diff/20001/build/common.gypi#newcod...
build/common.gypi:1310: ],
replace tabs with spaces

[Zhenyu Liang, 2014-03-13 03:15:46]

The CQ bit was checked by zhenyu.liang@intel.com

[commit-bot: I haz the power, 2014-03-13 03:17:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zhenyu.liang@intel.com/194843002/40001

[commit-bot: I haz the power, 2014-03-13 03:42:01]

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

[commit-bot: I haz the power, 2014-03-13 03:42:02]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Zhenyu Liang, 2014-03-15 20:06:46]

The CQ bit was checked by zhenyu.liang@intel.com

[commit-bot: I haz the power, 2014-03-15 20:06:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zhenyu.liang@intel.com/194843002/40001

[commit-bot: I haz the power, 2014-03-15 20:37:48]

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

[commit-bot: I haz the power, 2014-03-15 20:37:49]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Zhenyu Liang, 2014-03-17 06:27:59]

The CQ bit was checked by zhenyu.liang@intel.com

[commit-bot: I haz the power, 2014-03-17 06:28:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zhenyu.liang@intel.com/194843002/40001

[commit-bot: I haz the power, 2014-03-17 06:58:51]

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

[commit-bot: I haz the power, 2014-03-17 06:58:52]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Torne, 2014-03-17 10:51:13]

Zhenyu, the presubmit is failing because you aren't listed in the AUTHORS file.
I checked and you are covered by one of the Intel CCLA entries so it's fine to
just add your own name/email to the list.

[Zhenyu Liang, 2014-03-17 11:01:19]

On 2014/03/17 10:51:13, Torne wrote:
> Zhenyu, the presubmit is failing because you aren't listed in the AUTHORS
file.
> I checked and you are covered by one of the Intel CCLA entries so it's fine to
> just add your own name/email to the list.

Thank you, Torne. I'm just wondering about the failures.
Updated.

[Zhenyu Liang, 2014-03-17 11:01:32]

The CQ bit was checked by zhenyu.liang@intel.com

[commit-bot: I haz the power, 2014-03-17 11:01:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zhenyu.liang@intel.com/194843002/60001

[commit-bot: I haz the power, 2014-03-17 14:33:53]

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

[commit-bot: I haz the power, 2014-03-17 14:33:54]

Failed to apply patch for build/common.gypi:
While running patch -p0 --forward --force --no-backup-if-mismatch;
  patching file build/common.gypi
  Hunk #1 succeeded at 789 (offset -3 lines).
  Hunk #2 succeeded at 818 (offset -3 lines).
  Hunk #3 FAILED at 1301.
  Hunk #4 succeeded at 1429 (offset 2 lines).
  Hunk #5 succeeded at 1737 (offset -5 lines).
  Hunk #6 succeeded at 3250 (offset -15 lines).
  Hunk #7 succeeded at 3925 (offset 3 lines).
  Hunk #8 succeeded at 4048 (offset 3 lines).
  1 out of 8 hunks FAILED -- saving rejects to file build/common.gypi.rej

Patch:       build/common.gypi
Index: build/common.gypi
===================================================================
--- build/common.gypi	(revision 256167)
+++ build/common.gypi	(working copy)
@@ -792,10 +792,10 @@
           'test_isolation_mode%': 'noop',
         }],
         # Whether Android ARM or x86 build uses OpenMAX DL FFT.
-        ['OS=="android" and ((target_arch=="arm" and arm_version >= 7) or
target_arch=="ia32") and android_webview_build==0', {
-          # Currently only supported on Android ARMv7+, or ia32
+        ['OS=="android" and ((target_arch=="arm" and arm_version >= 7) or
target_arch=="ia32" or target_arch=="x64") and android_webview_build==0', {
+          # Currently only supported on Android ARMv7+, ia32 or x64
           # without webview.  When enabled, this will also enable
-          # WebAudio support on Android ARM and ia32.  Default is
+          # WebAudio support on Android ARM, ia32 and x64.  Default is
           # enabled.  Whether WebAudio is actually available depends
           # on runtime settings and flags.
           'use_openmax_dl_fft%': 1,
@@ -821,9 +821,9 @@
           'enable_printing%': 0,
         }],
 
-	# By default, use ICU data file (icudtl.dat) on all platforms
-	# except when building Android WebView.
-	# TODO(jshin): Handle 'use_system_icu' on Linux (Chromium).
+        # By default, use ICU data file (icudtl.dat) on all platforms
+        # except when building Android WebView.
+        # TODO(jshin): Handle 'use_system_icu' on Linux (Chromium).
         ['android_webview_build==0', {
           'icu_use_data_file_flag%' : 1,
         }, {
@@ -1301,7 +1301,13 @@
         'conditions': [
           ['OS=="android"', {
             # We directly set the gcc_version since we know what we use.
-            'gcc_version%': 46,
+            'conditions': [
+              ['target_arch=="x64"', {
+                'gcc_version%': 48,
+              }, {
+                'gcc_version%': 46,
+              }],
+            ],
           }, {
             'gcc_version%': '<!(python <(DEPTH)/build/compiler_version.py)',
           }],
@@ -1427,6 +1433,12 @@
               'android_ndk_sysroot%':
'<(android_ndk_root)/platforms/android-14/arch-x86',
               'android_toolchain%':
'<(android_ndk_root)/toolchains/x86-4.6/prebuilt/<(host_os)-<(android_host_arch)/bin',
             }],
+            ['target_arch == "x64"', {
+              'android_app_abi%': 'x86_64',
+              'android_gdbserver%':
'<(android_ndk_root)/prebuilt/android-x86_64/gdbserver/gdbserver',
+              'android_ndk_sysroot%':
'<(android_ndk_root)/platforms/android-19/arch-x86_64',
+              'android_toolchain%':
'<(android_ndk_root)/toolchains/x86_64-4.8/prebuilt/<(host_os)-<(android_host_arch)/bin',
+            }],
             ['target_arch=="arm"', {
               'conditions': [
                 ['arm_version<7', {
@@ -1736,7 +1748,7 @@
       ['use_titlecase_in_grd_files==1', {
         'grit_defines': ['-D', 'use_titlecase'],
       }],
-      ['OS=="android" and target_arch=="ia32"', {
+      ['OS=="android" and (target_arch=="ia32" or target_arch=="x64")', {
         # WebAudio on Android/x86 is disabled by default, unlike
         # everywhere else, so use appropriate message.
         'grit_defines': ['-D', 'use_webaudio_enable_message'],
@@ -3259,6 +3271,30 @@
               }],
             ],
           }],
+          ['target_arch=="x64"', {
+            'target_conditions': [
+              ['_toolset=="target"', {
+                'conditions': [
+                  # Use gold linker for Android x64 target.
+                  ['OS=="android"', {
+                    'cflags': [
+                      '-fuse-ld=gold',
+                    ],
+                    'ldflags': [
+                      '-fuse-ld=gold',
+                    ],
+                  }],
+                ],
+                'cflags': [
+                  '-m64',
+                  '-march=x86-64',
+                ],
+                'ldflags': [
+                  '-m64',
+                ],
+              }],
+            ],
+          }],
           ['target_arch=="arm"', {
             'target_conditions': [
               ['_toolset=="target"', {
@@ -3892,6 +3928,16 @@
                       '-target x86-linux-androideabi',
                     ],
                   }],
+                  # Place holder for x64 support, not tested.
+                  # TODO: Enable clang support for Android x64.
http://crbug.com/346626
+                  ['target_arch=="x64"', {
+                    'cflags': [
+                      '-target x86_64-linux-androideabi',
+                    ],
+                    'ldflags': [
+                      '-target x86_64-linux-androideabi',
+                    ],
+                  }],
                 ],
               }],
               ['asan==1', {
@@ -4005,7 +4051,6 @@
               ['_type=="executable"', {
                 'ldflags': [
                   '-Bdynamic',
-                  '-Wl,-dynamic-linker,/system/bin/linker',
                   '-Wl,--gc-sections',
                   '-Wl,-z,nocopyreloc',
                   # crtbegin_dynamic.o should be the last item in ldflags.

[Zhenyu Liang, 2014-03-17 16:18:46]

The CQ bit was unchecked by zhenyu.liang@intel.com

[Zhenyu Liang, 2014-03-17 16:18:49]

The CQ bit was checked by zhenyu.liang@intel.com

[commit-bot: I haz the power, 2014-03-17 16:19:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zhenyu.liang@intel.com/194843002/80001

[commit-bot: I haz the power, 2014-03-17 16:20:33]

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

[commit-bot: I haz the power, 2014-03-17 16:20:36]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[Zhenyu Liang, 2014-03-17 16:58:42]

The CQ bit was checked by zhenyu.liang@intel.com

[commit-bot: I haz the power, 2014-03-17 16:59:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zhenyu.liang@intel.com/194843002/80001

[commit-bot: I haz the power, 2014-03-17 20:28:24]

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

[commit-bot: I haz the power, 2014-03-17 20:28:26]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[Zhenyu Liang, 2014-03-18 00:57:05]

The CQ bit was checked by zhenyu.liang@intel.com

[commit-bot: I haz the power, 2014-03-18 00:59:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zhenyu.liang@intel.com/194843002/80001

[commit-bot: I haz the power, 2014-03-18 05:56:18]

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

[commit-bot: I haz the power, 2014-03-18 05:56:20]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[Zhenyu Liang, 2014-03-18 07:39:19]

The CQ bit was checked by zhenyu.liang@intel.com

[commit-bot: I haz the power, 2014-03-18 07:39:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zhenyu.liang@intel.com/194843002/80001

[commit-bot: I haz the power, 2014-03-18 17:43:56]

Change committed as 257683