Issue 1888763002: Build 64bit browser for Android with clang for ARMv8

khasim.mohammed

4 years, 8 months ago (2016-04-14 11:24:38 UTC) #1

urvang

urvang@chromium.org changed reviewers: + urvang@chromium.org

4 years, 8 months ago (2016-04-14 15:33:48 UTC) #2

Nico

Looks pretty good, thanks! https://codereview.chromium.org/1888763002/diff/1/build/config/android/config.gni File build/config/android/config.gni (right): https://codereview.chromium.org/1888763002/diff/1/build/config/android/config.gni#newcode249 build/config/android/config.gni:249: import("//build/config/arm.gni") why is this needed? ...

4 years, 8 months ago (2016-04-14 15:43:56 UTC) #4

Raymond Toy

rtoy@chromium.org changed reviewers: + rtoy@chromium.org

4 years, 8 months ago (2016-04-14 15:49:38 UTC) #5

Raymond Toy

https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/platform/audio/DenormalDisabler.h File third_party/WebKit/Source/platform/audio/DenormalDisabler.h (right): https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/platform/audio/DenormalDisabler.h#newcode134 third_party/WebKit/Source/platform/audio/DenormalDisabler.h:134: asm volatile("mrs %x[result], FPCR" : [result] "=r" (result)); So ...

4 years, 8 months ago (2016-04-14 15:49:38 UTC) #6

khasim.mohammed

https://codereview.chromium.org/1888763002/diff/1/build/config/android/config.gni File build/config/android/config.gni (right): https://codereview.chromium.org/1888763002/diff/1/build/config/android/config.gni#newcode249 build/config/android/config.gni:249: import("//build/config/arm.gni") On 2016/04/14 15:43:55, Nico wrote: > why is ...

4 years, 8 months ago (2016-04-14 16:13:41 UTC) #7

khasim.mohammed

https://codereview.chromium.org/1888763002/diff/1/build/config/compiler/BUILD.gn File build/config/compiler/BUILD.gn (right): https://codereview.chromium.org/1888763002/diff/1/build/config/compiler/BUILD.gn#newcode637 build/config/compiler/BUILD.gn:637: cflags = [ "-mcpu=$arm_fpu" ] On 2016/04/14 15:43:55, Nico ...

4 years, 8 months ago (2016-04-14 16:38:46 UTC) #8

khasim.mohammed

On 2016/04/14 16:38:46, khasim.mohammed wrote: > https://codereview.chromium.org/1888763002/diff/1/build/config/compiler/BUILD.gn > File build/config/compiler/BUILD.gn (right): > > https://codereview.chromium.org/1888763002/diff/1/build/config/compiler/BUILD.gn#newcode637 > ...

4 years, 7 months ago (2016-05-06 06:48:24 UTC) #9

Raymond Toy

On 2016/04/14 15:49:38, Raymond Toy wrote: > https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/platform/audio/DenormalDisabler.h > File third_party/WebKit/Source/platform/audio/DenormalDisabler.h (right): > > https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/platform/audio/DenormalDisabler.h#newcode134 ...

4 years, 7 months ago (2016-05-06 16:35:36 UTC) #10

khasim.mohammed

On 2016/05/06 16:35:36, Raymond Toy wrote: > On 2016/04/14 15:49:38, Raymond Toy wrote: > > ...

4 years, 7 months ago (2016-05-07 03:00:31 UTC) #11

khasim.mohammed

On 2016/05/07 03:00:31, khasim.mohammed wrote: > On 2016/05/06 16:35:36, Raymond Toy wrote: > > On ...

4 years, 7 months ago (2016-05-07 03:09:07 UTC) #12

Raymond Toy

On 2016/05/07 03:00:31, khasim.mohammed wrote: > On 2016/05/06 16:35:36, Raymond Toy wrote: > > On ...

4 years, 7 months ago (2016-05-09 20:47:06 UTC) #13

On 2016/05/07 03:00:31, khasim.mohammed wrote:
> On 2016/05/06 16:35:36, Raymond Toy wrote:
> > On 2016/04/14 15:49:38, Raymond Toy wrote:
> > >
> >
>
https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/p...
> > > File third_party/WebKit/Source/platform/audio/DenormalDisabler.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/p...
> > > third_party/WebKit/Source/platform/audio/DenormalDisabler.h:134: asm
> > > volatile("mrs %x[result], FPCR" : [result] "=r" (result));
> > > So "result" is basically an integer so %x[result] is a 64-bit register? 
> Would
> > > it be better to declare result to be int64_t instead of just plain int
(line
> > > 132)?
> > > 
> > > Also, isn't the FPCR register just 32 bits long?  Then it doesn't matter
if
> > it's
> > > stored in a 32-bit or 64-bit register?
> > 
> > AFAICT, you haven't actually answered these questions. There's also a
> disconnect
> > if result is declared to be an int, but you use a 64-bit register (even if
> they
> > are the same physical register.
> 
> Ah, sorry -  I missed replying to this one. Actually this is not related to
the
> variable declaration, it is the syntax of ARMv8 MSR and MRS registers.
> 
> The Syntax is 
> mrs  Xt, systemreg
> 
> Xt : Is the 64-bit name of the general-purpose destination register, in the
> range 0 to 31.

Perhaps not relevent, but if Xt is required, why doesn't the compiler complain
if Xt isn't given?

And if Xt is really required, perhaps it makes more sense to also declare result
to be int64_t instead of int.  (And updating the callers/users appropriately.) 
Of course we do know that FPCR is always in the low 32-bits so it doesn't matter
now.
> 
> More details here :
>
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0502d/Chdfifdc...
> 
> So what we are doing with "X" is correct for MRS and MSR instructions. 
> 
> Let me know if you need more details.

khasim.mohammed

On 2016/05/09 20:47:06, Raymond Toy wrote: > On 2016/05/07 03:00:31, khasim.mohammed wrote: > > On ...

4 years, 7 months ago (2016-05-10 02:43:35 UTC) #14

On 2016/05/09 20:47:06, Raymond Toy wrote:
> On 2016/05/07 03:00:31, khasim.mohammed wrote:
> > On 2016/05/06 16:35:36, Raymond Toy wrote:
> > > On 2016/04/14 15:49:38, Raymond Toy wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/p...
> > > > File third_party/WebKit/Source/platform/audio/DenormalDisabler.h
(right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/p...
> > > > third_party/WebKit/Source/platform/audio/DenormalDisabler.h:134: asm
> > > > volatile("mrs %x[result], FPCR" : [result] "=r" (result));
> > > > So "result" is basically an integer so %x[result] is a 64-bit register? 
> > Would
> > > > it be better to declare result to be int64_t instead of just plain int
> (line
> > > > 132)?
> > > > 
> > > > Also, isn't the FPCR register just 32 bits long?  Then it doesn't matter
> if
> > > it's
> > > > stored in a 32-bit or 64-bit register?
> > > 
> > > AFAICT, you haven't actually answered these questions. There's also a
> > disconnect
> > > if result is declared to be an int, but you use a 64-bit register (even if
> > they
> > > are the same physical register.
> > 
> > Ah, sorry -  I missed replying to this one. Actually this is not related to
> the
> > variable declaration, it is the syntax of ARMv8 MSR and MRS registers.
> > 
> > The Syntax is 
> > mrs  Xt, systemreg
> > 
> > Xt : Is the 64-bit name of the general-purpose destination register, in the
> > range 0 to 31.
> 
> Perhaps not relevent, but if Xt is required, why doesn't the compiler complain
> if Xt isn't given?

Compiler does complain, 
../../third_party/WebKit/Source/platform/audio/DenormalDisabler.h:134:61: error:
value size does not match register size specified by the constraint and modifier
[-Werror,-Wasm-operand-widths]
        asm volatile("mrs %[result], FPCR" : [result] "=r" (result));
                                                            ^
../../third_party/WebKit/Source/platform/audio/DenormalDisabler.h:134:27: note:
use constraint modifier "w"
        asm volatile("mrs %[result], FPCR" : [result] "=r" (result));
                          ^~~~~~~~~
                          %w[result]

I don't know why it shows w when there is no operand w for mrs or msr. But we do
get error on this from compiler.

> 
> And if Xt is really required, perhaps it makes more sense to also declare
result
> to be int64_t instead of int.  (And updating the callers/users appropriately.)

> Of course we do know that FPCR is always in the low 32-bits so it doesn't
matter
> now.

Yes, it doesn't matter as FPCR is always in lower 32 bits. 

> > 
> > More details here :
> >
>
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0502d/Chdfifdc...
> > 
> > So what we are doing with "X" is correct for MRS and MSR instructions. 
> > 
> > Let me know if you need more details.

Thanks, can you please +2 this patch ?

Raymond Toy

On 2016/05/10 at 02:43:35, khasim.mohammed wrote: > On 2016/05/09 20:47:06, Raymond Toy wrote: > > ...

4 years, 7 months ago (2016-05-10 02:46:13 UTC) #15

On 2016/05/10 at 02:43:35, khasim.mohammed wrote:
> On 2016/05/09 20:47:06, Raymond Toy wrote:
> > On 2016/05/07 03:00:31, khasim.mohammed wrote:
> > > On 2016/05/06 16:35:36, Raymond Toy wrote:
> > > > On 2016/04/14 15:49:38, Raymond Toy wrote:
> > > > >
> > > >
> > >
> >
https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/p...
> > > > > File third_party/WebKit/Source/platform/audio/DenormalDisabler.h
(right):
> > > > > 
> > > > >
> > > >
> > >
> >
https://codereview.chromium.org/1888763002/diff/1/third_party/WebKit/Source/p...
> > > > > third_party/WebKit/Source/platform/audio/DenormalDisabler.h:134: asm
> > > > > volatile("mrs %x[result], FPCR" : [result] "=r" (result));
> > > > > So "result" is basically an integer so %x[result] is a 64-bit
register? 
> > > Would
> > > > > it be better to declare result to be int64_t instead of just plain int
> > (line
> > > > > 132)?
> > > > > 
> > > > > Also, isn't the FPCR register just 32 bits long?  Then it doesn't
matter
> > if
> > > > it's
> > > > > stored in a 32-bit or 64-bit register?
> > > > 
> > > > AFAICT, you haven't actually answered these questions. There's also a
> > > disconnect
> > > > if result is declared to be an int, but you use a 64-bit register (even
if
> > > they
> > > > are the same physical register.
> > > 
> > > Ah, sorry -  I missed replying to this one. Actually this is not related
to
> > the
> > > variable declaration, it is the syntax of ARMv8 MSR and MRS registers.
> > > 
> > > The Syntax is 
> > > mrs  Xt, systemreg
> > > 
> > > Xt : Is the 64-bit name of the general-purpose destination register, in
the
> > > range 0 to 31.
> > 
> > Perhaps not relevent, but if Xt is required, why doesn't the compiler
complain
> > if Xt isn't given?
> 
> Compiler does complain, 
> ../../third_party/WebKit/Source/platform/audio/DenormalDisabler.h:134:61:
error: value size does not match register size specified by the constraint and
modifier [-Werror,-Wasm-operand-widths]
>         asm volatile("mrs %[result], FPCR" : [result] "=r" (result));
>                                                             ^
> ../../third_party/WebKit/Source/platform/audio/DenormalDisabler.h:134:27:
note: use constraint modifier "w"
>         asm volatile("mrs %[result], FPCR" : [result] "=r" (result));
>                           ^~~~~~~~~
>                           %w[result]
> 
> I don't know why it shows w when there is no operand w for mrs or msr. But we
do get error on this from compiler.

Thanks for the snippet. I guess the compiler has changed (or you're using a
different version) than when I created these.

And thanks for the explanations!
> 
> > 
> > And if Xt is really required, perhaps it makes more sense to also declare
result
> > to be int64_t instead of int.  (And updating the callers/users
appropriately.) 
> > Of course we do know that FPCR is always in the low 32-bits so it doesn't
matter
> > now.
> 
> Yes, it doesn't matter as FPCR is always in lower 32 bits. 
> 
> > > 
> > > More details here :
> > >
> >
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0502d/Chdfifdc...
> > > 
> > > So what we are doing with "X" is correct for MRS and MSR instructions. 
> > > 
> > > Let me know if you need more details.
> 
> Thanks, can you please +2 this patch ?

jochen (gone - plz use gerrit)

jochen@chromium.org changed reviewers: + jochen@chromium.org

4 years, 7 months ago (2016-05-10 05:18:34 UTC) #17

jochen (gone - plz use gerrit)

https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni File build/config/arm.gni (right): https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni#newcode75 build/config/arm.gni:75: arm_fpu = "cortex-a57+simd+crypto+fp+crc" Can you check that this results ...

4 years, 7 months ago (2016-05-10 05:18:35 UTC) #18

khasim.mohammed

On 10 May 2016 at 10:48, <jochen@chromium.org> wrote: > > https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni > File build/config/arm.gni (right): ...

4 years, 7 months ago (2016-05-10 05:45:01 UTC) #19

khasim.mohammed

On 10 May 2016 at 10:48, <jochen@chromium.org> wrote: > > https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni > File build/config/arm.gni (right): ...

4 years, 7 months ago (2016-05-10 05:45:01 UTC) #20

jochen (gone - plz use gerrit)

On 2016/05/10 at 05:45:01, khasim.mohammed wrote: > On 10 May 2016 at 10:48, <jochen@chromium.org> wrote: ...

4 years, 7 months ago (2016-05-10 05:57:51 UTC) #21

khasim.mohammed

On 2016/05/10 05:57:51, jochen (soon OOO) wrote: > On 2016/05/10 at 05:45:01, khasim.mohammed wrote: > ...

4 years, 7 months ago (2016-05-12 07:08:51 UTC) #22

On 2016/05/10 05:57:51, jochen (soon OOO) wrote:
> On 2016/05/10 at 05:45:01, khasim.mohammed wrote:
> > On 10 May 2016 at 10:48, <mailto:jochen@chromium.org> wrote:
> > 
> > >
> > > https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni
> > > File build/config/arm.gni (right):
> > >
> > >
> > >
>
https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni#ne...
> > > build/config/arm.gni:75: arm_fpu = "cortex-a57+simd+crypto+fp+crc"
> > > Can you check that this results in the same flags for v8 as the gyp
> > > build would?
> > >
> > > I used gn for building these, do you want me to try gyp ? I thought it was
> > outdated so was avoiding gyp.
> 
> V8 still uses gyp for the standalone build and has full arm64 support, and
it's
> key that the gn configuration results in the exact same set of CPU specific
> flags and defines
> 
> Building is not enough. You have to manually compare the flags.
> 
> If you check out a version of chromium that still has gyp support, you can use
> the script in tools/gn/bin to compare the flags
> 
> Otherwise, I'd recommend looking at the ninja files and compare the flags
> manually
> 

Didn't I reply to this ? Not sure why I don't see my message :

My observations are captured below :

with gn
=======

cflags = -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector
-funwind-tables -fPIC -pipe -fcolor-diagnostics
-fdebug-prefix-map=/home/chromium/src=. -ffunction-sections -fno-short-enums
-target aarch64-linux-android -mcpu=cortex-a57+simd+crypto+fp+crc -Os
-fdata-sections -ffunction-sections -g1
--sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
-fvisibility=hidden -Xclang -load -Xclang
../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang
-add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs
-Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang
follow-macro-expansion -Wheader-hygiene -Wstring-conversion -Werror -Wall
-Wno-unused-variable -Wno-asm-operand-widths -Wno-missing-field-initializers
-Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
-Wno-deprecated-register -Wno-unneeded-internal-declaration
-Wno-inconsistent-missing-override -Wno-shift-negative-value
cflags_cc = -fno-threadsafe-statics -fvisibility-inlines-hidden
-Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++11
-fno-rtti
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include
-isystem../../third_party/android_tools/ndk/sources/android/support/include
-fno-exceptions -Wno-deprecated

with gyp
========
     cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long -pthread 
-pedantic -Wmissing-field-initializers 
    -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32
-fvisibility=hidden -fcolor-diagnostics 
    -B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
-Wno-undefined-var-template 
    -B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
-Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2 
    -fdata-sections -ffunction-sections -O2 
cflags_c =
cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11

But I see the code uses the definitions from this file, so I am not sure if
these flags would affect directly in the case of v8 builds :

File : src/arm/assembler-arm.cc

#ifdef CAN_USE_ARMV8_INSTRUCTIONS
  if (FLAG_enable_armv8) {
    answer |= 1u << ARMv8;
    // ARMv8 always features VFP and NEON.
    answer |= 1u << ARMv7 | 1u << VFP3 | 1u << NEON | 1u << VFP32DREGS;
    answer |= 1u << SUDIV | 1u << MLS;
  }
#endif  // CAN_USE_ARMV8_INSTRUCTIONS

Regards,
Khasim

> > 
> > Regards,
> > Khasim
> > 
> > > https://codereview.chromium.org/1888763002/
> > >
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email to mailto:chromium-reviews+unsubscribe@chromium.org.
> >

jochen (gone - plz use gerrit)

On 2016/05/12 at 07:08:51, khasim.mohammed wrote: > On 2016/05/10 05:57:51, jochen (soon OOO) wrote: > ...

4 years, 7 months ago (2016-05-12 07:29:47 UTC) #23

On 2016/05/12 at 07:08:51, khasim.mohammed wrote:
> On 2016/05/10 05:57:51, jochen (soon OOO) wrote:
> > On 2016/05/10 at 05:45:01, khasim.mohammed wrote:
> > > On 10 May 2016 at 10:48, <mailto:jochen@chromium.org> wrote:
> > > 
> > > >
> > > >
https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni
> > > > File build/config/arm.gni (right):
> > > >
> > > >
> > > >
> >
https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni#ne...
> > > > build/config/arm.gni:75: arm_fpu = "cortex-a57+simd+crypto+fp+crc"
> > > > Can you check that this results in the same flags for v8 as the gyp
> > > > build would?
> > > >
> > > > I used gn for building these, do you want me to try gyp ? I thought it
was
> > > outdated so was avoiding gyp.
> > 
> > V8 still uses gyp for the standalone build and has full arm64 support, and
it's
> > key that the gn configuration results in the exact same set of CPU specific
> > flags and defines
> > 
> > Building is not enough. You have to manually compare the flags.
> > 
> > If you check out a version of chromium that still has gyp support, you can
use
> > the script in tools/gn/bin to compare the flags
> > 
> > Otherwise, I'd recommend looking at the ninja files and compare the flags
> > manually
> > 
> 
> Didn't I reply to this ? Not sure why I don't see my message :
> 
> My observations are captured below :
> 
> with gn
> =======
> 
> cflags = -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector
-funwind-tables -fPIC -pipe -fcolor-diagnostics
-fdebug-prefix-map=/home/chromium/src=. -ffunction-sections -fno-short-enums
-target aarch64-linux-android -mcpu=cortex-a57+simd+crypto+fp+crc -Os
-fdata-sections -ffunction-sections -g1
--sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
-fvisibility=hidden -Xclang -load -Xclang
../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang
-add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs
-Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang
follow-macro-expansion -Wheader-hygiene -Wstring-conversion -Werror -Wall
-Wno-unused-variable -Wno-asm-operand-widths -Wno-missing-field-initializers
-Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
-Wno-deprecated-register -Wno-unneeded-internal-declaration
-Wno-inconsistent-missing-override -Wno-shift-negative-value
> cflags_cc = -fno-threadsafe-statics -fvisibility-inlines-hidden
-Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++11
-fno-rtti
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include
-isystem../../third_party/android_tools/ndk/sources/android/support/include
-fno-exceptions -Wno-deprecated
> 
> with gyp
> ========
>      cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long -pthread 
-pedantic -Wmissing-field-initializers 
>     -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32
-fvisibility=hidden -fcolor-diagnostics 
>     -B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
-Wno-undefined-var-template 
>     -B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
-Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2 
>     -fdata-sections -ffunction-sections -O2 
> cflags_c =
> cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11

it looks like you're comparing x64 with arm64 flags?

> 
> But I see the code uses the definitions from this file, so I am not sure if
these flags would affect directly in the case of v8 builds :
> 
> File : src/arm/assembler-arm.cc
> 
> #ifdef CAN_USE_ARMV8_INSTRUCTIONS

for those, it's also important to compare the defines (-D...) flags

>   if (FLAG_enable_armv8) {
>     answer |= 1u << ARMv8;
>     // ARMv8 always features VFP and NEON.
>     answer |= 1u << ARMv7 | 1u << VFP3 | 1u << NEON | 1u << VFP32DREGS;
>     answer |= 1u << SUDIV | 1u << MLS;
>   }
> #endif  // CAN_USE_ARMV8_INSTRUCTIONS
> 
> Regards,
> Khasim
> 
> > > 
> > > Regards,
> > > Khasim
> > > 
> > > > https://codereview.chromium.org/1888763002/
> > > >
> > > 
> > > -- 
> > > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an
> > email to mailto:chromium-reviews+unsubscribe@chromium.org.
> > >

khasim.mohammed

On 2016/05/12 07:29:47, jochen (soon OOO) wrote: > On 2016/05/12 at 07:08:51, khasim.mohammed wrote: > ...

4 years, 7 months ago (2016-05-12 07:41:41 UTC) #24

On 2016/05/12 07:29:47, jochen (soon OOO) wrote:
> On 2016/05/12 at 07:08:51, khasim.mohammed wrote:
> > On 2016/05/10 05:57:51, jochen (soon OOO) wrote:
> > > On 2016/05/10 at 05:45:01, khasim.mohammed wrote:
> > > > On 10 May 2016 at 10:48, <mailto:jochen@chromium.org> wrote:
> > > > 
> > > > >
> > > > >
> https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni
> > > > > File build/config/arm.gni (right):
> > > > >
> > > > >
> > > > >
> > >
>
https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni#ne...
> > > > > build/config/arm.gni:75: arm_fpu = "cortex-a57+simd+crypto+fp+crc"
> > > > > Can you check that this results in the same flags for v8 as the gyp
> > > > > build would?
> > > > >
> > > > > I used gn for building these, do you want me to try gyp ? I thought it
> was
> > > > outdated so was avoiding gyp.
> > > 
> > > V8 still uses gyp for the standalone build and has full arm64 support, and
> it's
> > > key that the gn configuration results in the exact same set of CPU
specific
> > > flags and defines
> > > 
> > > Building is not enough. You have to manually compare the flags.
> > > 
> > > If you check out a version of chromium that still has gyp support, you can
> use
> > > the script in tools/gn/bin to compare the flags
> > > 
> > > Otherwise, I'd recommend looking at the ninja files and compare the flags
> > > manually
> > > 
> > 
> > Didn't I reply to this ? Not sure why I don't see my message :
> > 
> > My observations are captured below :
> > 
> > with gn
> > =======
> > 
> > cflags = -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector
> -funwind-tables -fPIC -pipe -fcolor-diagnostics
> -fdebug-prefix-map=/home/chromium/src=. -ffunction-sections -fno-short-enums
> -target aarch64-linux-android -mcpu=cortex-a57+simd+crypto+fp+crc -Os
> -fdata-sections -ffunction-sections -g1
> --sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
> -fvisibility=hidden -Xclang -load -Xclang
> ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so
-Xclang
> -add-plugin -Xclang find-bad-constructs -Xclang
-plugin-arg-find-bad-constructs
> -Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang
> follow-macro-expansion -Wheader-hygiene -Wstring-conversion -Werror -Wall
> -Wno-unused-variable -Wno-asm-operand-widths -Wno-missing-field-initializers
> -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
> -Wno-deprecated-register -Wno-unneeded-internal-declaration
> -Wno-inconsistent-missing-override -Wno-shift-negative-value
> > cflags_cc = -fno-threadsafe-statics -fvisibility-inlines-hidden
> -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare
-std=gnu++11
> -fno-rtti
>
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include
>
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include
> -isystem../../third_party/android_tools/ndk/sources/android/support/include
> -fno-exceptions -Wno-deprecated
> > 
> > with gyp
> > ========
> >      cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long -pthread 
> -pedantic -Wmissing-field-initializers 
> >     -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32
> -fvisibility=hidden -fcolor-diagnostics 
> >     -B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
> -Wno-undefined-var-template 
> >     -B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
> -Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2 
> >     -fdata-sections -ffunction-sections -O2 
> > cflags_c =
> > cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11
> 
> it looks like you're comparing x64 with arm64 flags?
> 

No, it is for ARM 64, see the complete log from file
Release/obj/src/v8_base.ninja

defines = -DCR_CLANG_REVISION=268813-1 -DV8_TARGET_ARCH_ARM64 $
    -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS $
    -DV8_I18N_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA $
    -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DU_USING_ICU_NAMESPACE=0 $
    -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION $
    -DENABLE_HANDLE_ZAPPING
includes = -I../.. -I../../.. -I../../third_party/icu/source/i18n $
    -I../../third_party/icu/source/common
cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long -pthread $
    -pedantic -Wmissing-field-initializers $
    -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32 $
    -fvisibility=hidden -fcolor-diagnostics $
   
-B/home/khasim/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin $
    -Wno-undefined-var-template $
   
-B/home/khasim/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin $
    -Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2 $
    -fdata-sections -ffunction-sections -O2
cflags_c =
cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11
arflags =


> > 
> > But I see the code uses the definitions from this file, so I am not sure if
> these flags would affect directly in the case of v8 builds :
> > 
> > File : src/arm/assembler-arm.cc
> > 
> > #ifdef CAN_USE_ARMV8_INSTRUCTIONS
> 
> for those, it's also important to compare the defines (-D...) flags

defines (-D) for gn
==================

defines = -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DV8_DEPRECATION_WARNINGS
-DCLD_VERSION=2 -DENABLE_NOTIFICATIONS -DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1
-DENABLE_BASIC_PRINTING=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1
-DUSE_OPENSSL=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1
-DDISABLE_NACL -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
-DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
-DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
-DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1 -D_FILE_OFFSET_BITS=64
-DANDROID -DHAVE_SYS_UIO_H -DCOMPONENT_BUILD -D__GNU_SOURCE=1
-D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -D_DEBUG
-DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DV8_SHARED
-DBUILDING_V8_SHARED -DV8_I18N_SUPPORT -DENABLE_HANDLE_ZAPPING
-DV8_USE_EXTERNAL_STARTUP_DATA -DV8_TARGET_ARCH_ARM64 -DENABLE_DISASSEMBLER
-DV8_ENABLE_CHECKS -DOBJECT_PRINT -DVERIFY_HEAP -DDEBUG -DOPTIMIZED_DEBUG
-DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0
include_dirs = -I../.. -Igen -I../../v8 -I../../third_party/icu/source/common
-I../../third_party/icu/source/i18n

Defines -D for gyp
==================
defines = -DCR_CLANG_REVISION=268813-1 -DV8_TARGET_ARCH_ARM64 $
    -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS $
    -DV8_I18N_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA $
    -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DU_USING_ICU_NAMESPACE=0 $
    -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION $
    -DENABLE_HANDLE_ZAPPING
includes = -I../.. -I../../.. -I../../third_party/icu/source/i18n $
    -I../../third_party/icu/source/common


> 
> >   if (FLAG_enable_armv8) {
> >     answer |= 1u << ARMv8;
> >     // ARMv8 always features VFP and NEON.
> >     answer |= 1u << ARMv7 | 1u << VFP3 | 1u << NEON | 1u << VFP32DREGS;
> >     answer |= 1u << SUDIV | 1u << MLS;
> >   }
> > #endif  // CAN_USE_ARMV8_INSTRUCTIONS
> > 
> > Regards,
> > Khasim
> > 
> > > > 
> > > > Regards,
> > > > Khasim
> > > > 
> > > > > https://codereview.chromium.org/1888763002/
> > > > >
> > > > 
> > > > -- 
> > > > You received this message because you are subscribed to the Google
Groups
> > > "Chromium-reviews" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send
an
> > > email to mailto:chromium-reviews+unsubscribe@chromium.org.
> > > >

jochen (gone - plz use gerrit)

On 2016/05/12 at 07:41:41, khasim.mohammed wrote: > On 2016/05/12 07:29:47, jochen (soon OOO) wrote: > ...

4 years, 7 months ago (2016-05-12 07:46:07 UTC) #25

On 2016/05/12 at 07:41:41, khasim.mohammed wrote:
> On 2016/05/12 07:29:47, jochen (soon OOO) wrote:
> > On 2016/05/12 at 07:08:51, khasim.mohammed wrote:
> > > On 2016/05/10 05:57:51, jochen (soon OOO) wrote:
> > > > On 2016/05/10 at 05:45:01, khasim.mohammed wrote:
> > > > > On 10 May 2016 at 10:48, <mailto:jochen@chromium.org> wrote:
> > > > > 
> > > > > >
> > > > > >
> > https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni
> > > > > > File build/config/arm.gni (right):
> > > > > >
> > > > > >
> > > > > >
> > > >
> >
https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni#ne...
> > > > > > build/config/arm.gni:75: arm_fpu = "cortex-a57+simd+crypto+fp+crc"
> > > > > > Can you check that this results in the same flags for v8 as the gyp
> > > > > > build would?
> > > > > >
> > > > > > I used gn for building these, do you want me to try gyp ? I thought
it
> > was
> > > > > outdated so was avoiding gyp.
> > > > 
> > > > V8 still uses gyp for the standalone build and has full arm64 support,
and
> > it's
> > > > key that the gn configuration results in the exact same set of CPU
specific
> > > > flags and defines
> > > > 
> > > > Building is not enough. You have to manually compare the flags.
> > > > 
> > > > If you check out a version of chromium that still has gyp support, you
can
> > use
> > > > the script in tools/gn/bin to compare the flags
> > > > 
> > > > Otherwise, I'd recommend looking at the ninja files and compare the
flags
> > > > manually
> > > > 
> > > 
> > > Didn't I reply to this ? Not sure why I don't see my message :
> > > 
> > > My observations are captured below :
> > > 
> > > with gn
> > > =======
> > > 
> > > cflags = -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector
> > -funwind-tables -fPIC -pipe -fcolor-diagnostics
> > -fdebug-prefix-map=/home/chromium/src=. -ffunction-sections -fno-short-enums
> > -target aarch64-linux-android -mcpu=cortex-a57+simd+crypto+fp+crc -Os
> > -fdata-sections -ffunction-sections -g1
> >
--sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
> > -fvisibility=hidden -Xclang -load -Xclang
> > ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so
-Xclang
> > -add-plugin -Xclang find-bad-constructs -Xclang
-plugin-arg-find-bad-constructs
> > -Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang
> > follow-macro-expansion -Wheader-hygiene -Wstring-conversion -Werror -Wall
> > -Wno-unused-variable -Wno-asm-operand-widths -Wno-missing-field-initializers
> > -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
> > -Wno-deprecated-register -Wno-unneeded-internal-declaration
> > -Wno-inconsistent-missing-override -Wno-shift-negative-value
> > > cflags_cc = -fno-threadsafe-statics -fvisibility-inlines-hidden
> > -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare
-std=gnu++11
> > -fno-rtti
> >
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include
> >
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include
> > -isystem../../third_party/android_tools/ndk/sources/android/support/include
> > -fno-exceptions -Wno-deprecated
> > > 
> > > with gyp
> > > ========
> > >      cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long -pthread 
> > -pedantic -Wmissing-field-initializers 
> > >     -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32
> > -fvisibility=hidden -fcolor-diagnostics 
> > >     -B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
> > -Wno-undefined-var-template 
> > >     -B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
> > -Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2 
> > >     -fdata-sections -ffunction-sections -O2 
> > > cflags_c =
> > > cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11
> > 
> > it looks like you're comparing x64 with arm64 flags?
> > 
> 
> No, it is for ARM 64, see the complete log from file
Release/obj/src/v8_base.ninja
> 
> defines = -DCR_CLANG_REVISION=268813-1 -DV8_TARGET_ARCH_ARM64 $
>     -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS $
>     -DV8_I18N_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA $
>     -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DU_USING_ICU_NAMESPACE=0 $
>     -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION $
>     -DENABLE_HANDLE_ZAPPING
> includes = -I../.. -I../../.. -I../../third_party/icu/source/i18n $
>     -I../../third_party/icu/source/common
> cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long -pthread $
>     -pedantic -Wmissing-field-initializers $
>     -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32 $
>     -fvisibility=hidden -fcolor-diagnostics $
>    
-B/home/khasim/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin $
>     -Wno-undefined-var-template $
>    
-B/home/khasim/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin $
>     -Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2 $
>     -fdata-sections -ffunction-sections -O2
> cflags_c =
> cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11
> arflags =
> 

The gyp flags don't look anything like the gn flags... e.g. -m64 is the only cpu
flag in gyp, while in gn, you now have -target aarch64-linux-android
-mcpu=cortex-a57+simd+crypto+fp+crc

also the missing -sysroot makes me suspect that you're not looking at the right
flags

> 
> > > 
> > > But I see the code uses the definitions from this file, so I am not sure
if
> > these flags would affect directly in the case of v8 builds :
> > > 
> > > File : src/arm/assembler-arm.cc
> > > 
> > > #ifdef CAN_USE_ARMV8_INSTRUCTIONS
> > 
> > for those, it's also important to compare the defines (-D...) flags
> 
> defines (-D) for gn
> ==================
> 
> defines = -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DV8_DEPRECATION_WARNINGS
-DCLD_VERSION=2 -DENABLE_NOTIFICATIONS -DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1
-DENABLE_BASIC_PRINTING=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1
-DUSE_OPENSSL=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1
-DDISABLE_NACL -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
-DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
-DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
-DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1 -D_FILE_OFFSET_BITS=64
-DANDROID -DHAVE_SYS_UIO_H -DCOMPONENT_BUILD -D__GNU_SOURCE=1
-D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -D_DEBUG
-DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DV8_SHARED
-DBUILDING_V8_SHARED -DV8_I18N_SUPPORT -DENABLE_HANDLE_ZAPPING
-DV8_USE_EXTERNAL_STARTUP_DATA -DV8_TARGET_ARCH_ARM64 -DENABLE_DISASSEMBLER
-DV8_ENABLE_CHECKS -DOBJECT_PRINT -DVERIFY_HEAP -DDEBUG -DOPTIMIZED_DEBUG
-DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0
> include_dirs = -I../.. -Igen -I../../v8 -I../../third_party/icu/source/common
-I../../third_party/icu/source/i18n


VERIFY_HEAP, DEBUG, OBJECT_PRINT etc.. are all debug flags, but gyp appears to
be a release build?

> 
> Defines -D for gyp
> ==================
> defines = -DCR_CLANG_REVISION=268813-1 -DV8_TARGET_ARCH_ARM64 $
>     -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS $
>     -DV8_I18N_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA $
>     -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DU_USING_ICU_NAMESPACE=0 $
>     -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION $
>     -DENABLE_HANDLE_ZAPPING
> includes = -I../.. -I../../.. -I../../third_party/icu/source/i18n $
>     -I../../third_party/icu/source/common
> 
> 
> > 
> > >   if (FLAG_enable_armv8) {
> > >     answer |= 1u << ARMv8;
> > >     // ARMv8 always features VFP and NEON.
> > >     answer |= 1u << ARMv7 | 1u << VFP3 | 1u << NEON | 1u << VFP32DREGS;
> > >     answer |= 1u << SUDIV | 1u << MLS;
> > >   }
> > > #endif  // CAN_USE_ARMV8_INSTRUCTIONS
> > > 
> > > Regards,
> > > Khasim
> > > 
> > > > > 
> > > > > Regards,
> > > > > Khasim
> > > > > 
> > > > > > https://codereview.chromium.org/1888763002/
> > > > > >
> > > > > 
> > > > > -- 
> > > > > You received this message because you are subscribed to the Google
Groups
> > > > "Chromium-reviews" group.
> > > > > To unsubscribe from this group and stop receiving emails from it, send
an
> > > > email to mailto:chromium-reviews+unsubscribe@chromium.org.
> > > > >

khasim.mohammed

On 2016/05/12 07:46:07, jochen (soon OOO) wrote: > On 2016/05/12 at 07:41:41, khasim.mohammed wrote: > ...

4 years, 7 months ago (2016-05-12 07:54:31 UTC) #26

On 2016/05/12 07:46:07, jochen (soon OOO) wrote:
> On 2016/05/12 at 07:41:41, khasim.mohammed wrote:
> > On 2016/05/12 07:29:47, jochen (soon OOO) wrote:
> > > On 2016/05/12 at 07:08:51, khasim.mohammed wrote:
> > > > On 2016/05/10 05:57:51, jochen (soon OOO) wrote:
> > > > > On 2016/05/10 at 05:45:01, khasim.mohammed wrote:
> > > > > > On 10 May 2016 at 10:48, <mailto:jochen@chromium.org> wrote:
> > > > > > 
> > > > > > >
> > > > > > >
> > > https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni
> > > > > > > File build/config/arm.gni (right):
> > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> > >
>
https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni#ne...
> > > > > > > build/config/arm.gni:75: arm_fpu = "cortex-a57+simd+crypto+fp+crc"
> > > > > > > Can you check that this results in the same flags for v8 as the
gyp
> > > > > > > build would?
> > > > > > >
> > > > > > > I used gn for building these, do you want me to try gyp ? I
thought
> it
> > > was
> > > > > > outdated so was avoiding gyp.
> > > > > 
> > > > > V8 still uses gyp for the standalone build and has full arm64 support,
> and
> > > it's
> > > > > key that the gn configuration results in the exact same set of CPU
> specific
> > > > > flags and defines
> > > > > 
> > > > > Building is not enough. You have to manually compare the flags.
> > > > > 
> > > > > If you check out a version of chromium that still has gyp support, you
> can
> > > use
> > > > > the script in tools/gn/bin to compare the flags
> > > > > 
> > > > > Otherwise, I'd recommend looking at the ninja files and compare the
> flags
> > > > > manually
> > > > > 
> > > > 
> > > > Didn't I reply to this ? Not sure why I don't see my message :
> > > > 
> > > > My observations are captured below :
> > > > 
> > > > with gn
> > > > =======
> > > > 
> > > > cflags = -fno-strict-aliasing --param=ssp-buffer-size=4
-fstack-protector
> > > -funwind-tables -fPIC -pipe -fcolor-diagnostics
> > > -fdebug-prefix-map=/home/chromium/src=. -ffunction-sections
-fno-short-enums
> > > -target aarch64-linux-android -mcpu=cortex-a57+simd+crypto+fp+crc -Os
> > > -fdata-sections -ffunction-sections -g1
> > >
> --sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
> > > -fvisibility=hidden -Xclang -load -Xclang
> > > ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so
> -Xclang
> > > -add-plugin -Xclang find-bad-constructs -Xclang
> -plugin-arg-find-bad-constructs
> > > -Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang
> > > follow-macro-expansion -Wheader-hygiene -Wstring-conversion -Werror -Wall
> > > -Wno-unused-variable -Wno-asm-operand-widths
-Wno-missing-field-initializers
> > > -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
> > > -Wno-deprecated-register -Wno-unneeded-internal-declaration
> > > -Wno-inconsistent-missing-override -Wno-shift-negative-value
> > > > cflags_cc = -fno-threadsafe-statics -fvisibility-inlines-hidden
> > > -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare
> -std=gnu++11
> > > -fno-rtti
> > >
>
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include
> > >
>
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include
> > >
-isystem../../third_party/android_tools/ndk/sources/android/support/include
> > > -fno-exceptions -Wno-deprecated
> > > > 
> > > > with gyp
> > > > ========
> > > >      cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long
-pthread 
> > > -pedantic -Wmissing-field-initializers 
> > > >     -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32
> > > -fvisibility=hidden -fcolor-diagnostics 
> > > >    
-B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
> > > -Wno-undefined-var-template 
> > > >    
-B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
> > > -Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2 
> > > >     -fdata-sections -ffunction-sections -O2 
> > > > cflags_c =
> > > > cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11
> > > 
> > > it looks like you're comparing x64 with arm64 flags?
> > > 
> > 
> > No, it is for ARM 64, see the complete log from file
> Release/obj/src/v8_base.ninja
> > 
> > defines = -DCR_CLANG_REVISION=268813-1 -DV8_TARGET_ARCH_ARM64 $
> >     -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS $
> >     -DV8_I18N_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA $
> >     -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DU_USING_ICU_NAMESPACE=0 $
> >     -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION $
> >     -DENABLE_HANDLE_ZAPPING
> > includes = -I../.. -I../../.. -I../../third_party/icu/source/i18n $
> >     -I../../third_party/icu/source/common
> > cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long -pthread $
> >     -pedantic -Wmissing-field-initializers $
> >     -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32 $
> >     -fvisibility=hidden -fcolor-diagnostics $
> >    
> -B/home/khasim/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
$
> >     -Wno-undefined-var-template $
> >    
> -B/home/khasim/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
$
> >     -Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2
$
> >     -fdata-sections -ffunction-sections -O2
> > cflags_c =
> > cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11
> > arflags =
> > 
> 
> The gyp flags don't look anything like the gn flags... e.g. -m64 is the only
cpu
> flag in gyp, while in gn, you now have -target aarch64-linux-android
> -mcpu=cortex-a57+simd+crypto+fp+crc
> 
> also the missing -sysroot makes me suspect that you're not looking at the
right
> flags

Here is what I have done, 

Downloaded the v8 separately
https://developers.google.com/v8/build#download-the-source-code

Followed the building steps from
https://github.com/v8/v8/wiki/Building%20with%20Gyp

export GYP_DEFINES="v8_target_arch=arm64"
gypfiles/gyp_v8 ...

export GYP_GENERATORS=ninja
gypfiles/gyp_v8
ninja -C out/Debug d8

Then reading the flags from .ninja files.

> 
> > 
> > > > 
> > > > But I see the code uses the definitions from this file, so I am not sure
> if
> > > these flags would affect directly in the case of v8 builds :
> > > > 
> > > > File : src/arm/assembler-arm.cc
> > > > 
> > > > #ifdef CAN_USE_ARMV8_INSTRUCTIONS
> > > 
> > > for those, it's also important to compare the defines (-D...) flags
> > 
> > defines (-D) for gn
> > ==================
> > 
> > defines = -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DV8_DEPRECATION_WARNINGS
> -DCLD_VERSION=2 -DENABLE_NOTIFICATIONS -DENABLE_BROWSER_CDMS
-DENABLE_PRINTING=1
> -DENABLE_BASIC_PRINTING=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1
> -DUSE_OPENSSL=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1
> -DDISABLE_NACL -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
> -DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
> -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
> -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1
-D_FILE_OFFSET_BITS=64
> -DANDROID -DHAVE_SYS_UIO_H -DCOMPONENT_BUILD -D__GNU_SOURCE=1
> -D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -D_DEBUG
> -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DV8_SHARED
> -DBUILDING_V8_SHARED -DV8_I18N_SUPPORT -DENABLE_HANDLE_ZAPPING
> -DV8_USE_EXTERNAL_STARTUP_DATA -DV8_TARGET_ARCH_ARM64 -DENABLE_DISASSEMBLER
> -DV8_ENABLE_CHECKS -DOBJECT_PRINT -DVERIFY_HEAP -DDEBUG -DOPTIMIZED_DEBUG
> -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0
> > include_dirs = -I../.. -Igen -I../../v8
-I../../third_party/icu/source/common
> -I../../third_party/icu/source/i18n
> 
> 
> VERIFY_HEAP, DEBUG, OBJECT_PRINT etc.. are all debug flags, but gyp appears to
> be a release build?

I will give a try with DEBUG build.


> 
> > 
> > Defines -D for gyp
> > ==================
> > defines = -DCR_CLANG_REVISION=268813-1 -DV8_TARGET_ARCH_ARM64 $
> >     -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS $
> >     -DV8_I18N_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA $
> >     -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DU_USING_ICU_NAMESPACE=0 $
> >     -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION $
> >     -DENABLE_HANDLE_ZAPPING
> > includes = -I../.. -I../../.. -I../../third_party/icu/source/i18n $
> >     -I../../third_party/icu/source/common
> > 
> > 
> > > 
> > > >   if (FLAG_enable_armv8) {
> > > >     answer |= 1u << ARMv8;
> > > >     // ARMv8 always features VFP and NEON.
> > > >     answer |= 1u << ARMv7 | 1u << VFP3 | 1u << NEON | 1u << VFP32DREGS;
> > > >     answer |= 1u << SUDIV | 1u << MLS;
> > > >   }
> > > > #endif  // CAN_USE_ARMV8_INSTRUCTIONS
> > > > 
> > > > Regards,
> > > > Khasim
> > > > 
> > > > > > 
> > > > > > Regards,
> > > > > > Khasim
> > > > > > 
> > > > > > > https://codereview.chromium.org/1888763002/
> > > > > > >
> > > > > > 
> > > > > > -- 
> > > > > > You received this message because you are subscribed to the Google
> Groups
> > > > > "Chromium-reviews" group.
> > > > > > To unsubscribe from this group and stop receiving emails from it,
send
> an
> > > > > email to mailto:chromium-reviews+unsubscribe@chromium.org.
> > > > > >

khasim.mohammed

> > > > VERIFY_HEAP, DEBUG, OBJECT_PRINT etc.. are all debug flags, but gyp appears ...

4 years, 7 months ago (2016-05-12 08:18:18 UTC) #27

jochen (gone - plz use gerrit)

On 2016/05/12 at 07:54:31, khasim.mohammed wrote: > On 2016/05/12 07:46:07, jochen (soon OOO) wrote: > ...

4 years, 7 months ago (2016-05-12 09:10:04 UTC) #28

On 2016/05/12 at 07:54:31, khasim.mohammed wrote:
> On 2016/05/12 07:46:07, jochen (soon OOO) wrote:
> > On 2016/05/12 at 07:41:41, khasim.mohammed wrote:
> > > On 2016/05/12 07:29:47, jochen (soon OOO) wrote:
> > > > On 2016/05/12 at 07:08:51, khasim.mohammed wrote:
> > > > > On 2016/05/10 05:57:51, jochen (soon OOO) wrote:
> > > > > > On 2016/05/10 at 05:45:01, khasim.mohammed wrote:
> > > > > > > On 10 May 2016 at 10:48, <mailto:jochen@chromium.org> wrote:
> > > > > > > 
> > > > > > > >
> > > > > > > >
> > > >
https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni
> > > > > > > > File build/config/arm.gni (right):
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> >
https://codereview.chromium.org/1888763002/diff/20001/build/config/arm.gni#ne...
> > > > > > > > build/config/arm.gni:75: arm_fpu =
"cortex-a57+simd+crypto+fp+crc"
> > > > > > > > Can you check that this results in the same flags for v8 as the
gyp
> > > > > > > > build would?
> > > > > > > >
> > > > > > > > I used gn for building these, do you want me to try gyp ? I
thought
> > it
> > > > was
> > > > > > > outdated so was avoiding gyp.
> > > > > > 
> > > > > > V8 still uses gyp for the standalone build and has full arm64
support,
> > and
> > > > it's
> > > > > > key that the gn configuration results in the exact same set of CPU
> > specific
> > > > > > flags and defines
> > > > > > 
> > > > > > Building is not enough. You have to manually compare the flags.
> > > > > > 
> > > > > > If you check out a version of chromium that still has gyp support,
you
> > can
> > > > use
> > > > > > the script in tools/gn/bin to compare the flags
> > > > > > 
> > > > > > Otherwise, I'd recommend looking at the ninja files and compare the
> > flags
> > > > > > manually
> > > > > > 
> > > > > 
> > > > > Didn't I reply to this ? Not sure why I don't see my message :
> > > > > 
> > > > > My observations are captured below :
> > > > > 
> > > > > with gn
> > > > > =======
> > > > > 
> > > > > cflags = -fno-strict-aliasing --param=ssp-buffer-size=4
-fstack-protector
> > > > -funwind-tables -fPIC -pipe -fcolor-diagnostics
> > > > -fdebug-prefix-map=/home/chromium/src=. -ffunction-sections
-fno-short-enums
> > > > -target aarch64-linux-android -mcpu=cortex-a57+simd+crypto+fp+crc -Os
> > > > -fdata-sections -ffunction-sections -g1
> > > >
> >
--sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
> > > > -fvisibility=hidden -Xclang -load -Xclang
> > > > ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so
> > -Xclang
> > > > -add-plugin -Xclang find-bad-constructs -Xclang
> > -plugin-arg-find-bad-constructs
> > > > -Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang
> > > > follow-macro-expansion -Wheader-hygiene -Wstring-conversion -Werror
-Wall
> > > > -Wno-unused-variable -Wno-asm-operand-widths
-Wno-missing-field-initializers
> > > > -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
> > > > -Wno-deprecated-register -Wno-unneeded-internal-declaration
> > > > -Wno-inconsistent-missing-override -Wno-shift-negative-value
> > > > > cflags_cc = -fno-threadsafe-statics -fvisibility-inlines-hidden
> > > > -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare
> > -std=gnu++11
> > > > -fno-rtti
> > > >
> >
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include
> > > >
> >
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include
> > > >
-isystem../../third_party/android_tools/ndk/sources/android/support/include
> > > > -fno-exceptions -Wno-deprecated
> > > > > 
> > > > > with gyp
> > > > > ========
> > > > >      cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long
-pthread 
> > > > -pedantic -Wmissing-field-initializers 
> > > > >     -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32
> > > > -fvisibility=hidden -fcolor-diagnostics 
> > > > >    
-B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
> > > > -Wno-undefined-var-template 
> > > > >    
-B/home/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin
> > > > -Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections  -O2 
> > > > >     -fdata-sections -ffunction-sections -O2 
> > > > > cflags_c =
> > > > > cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11
> > > > 
> > > > it looks like you're comparing x64 with arm64 flags?
> > > > 
> > > 
> > > No, it is for ARM 64, see the complete log from file
> > Release/obj/src/v8_base.ninja
> > > 
> > > defines = -DCR_CLANG_REVISION=268813-1 -DV8_TARGET_ARCH_ARM64 $
> > >     -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS $
> > >     -DV8_I18N_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA $
> > >     -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DU_USING_ICU_NAMESPACE=0 $
> > >     -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION $
> > >     -DENABLE_HANDLE_ZAPPING
> > > includes = -I../.. -I../../.. -I../../third_party/icu/source/i18n $
> > >     -I../../third_party/icu/source/common
> > > cflags = -Wall -Werror -Wno-unused-parameter -Wno-long-long -pthread $
> > >     -pedantic -Wmissing-field-initializers $
> > >     -Wno-gnu-zero-variadic-macro-arguments -Wshorten-64-to-32 $
> > >     -fvisibility=hidden -fcolor-diagnostics $
> > >    
> >
-B/home/khasim/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin $
> > >     -Wno-undefined-var-template $
> > >    
> >
-B/home/khasim/projects/temp_v8/v8/third_party/binutils/Linux_x64/Release/bin $
> > >     -Wno-format-pedantic -m64 -m64 -fdata-sections -ffunction-sections 
-O2 $
> > >     -fdata-sections -ffunction-sections -O2
> > > cflags_c =
> > > cflags_cc = -Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11
> > > arflags =
> > > 
> > 
> > The gyp flags don't look anything like the gn flags... e.g. -m64 is the only
cpu
> > flag in gyp, while in gn, you now have -target aarch64-linux-android
> > -mcpu=cortex-a57+simd+crypto+fp+crc
> > 
> > also the missing -sysroot makes me suspect that you're not looking at the
right
> > flags
> 
> Here is what I have done, 
> 
> Downloaded the v8 separately
https://developers.google.com/v8/build#download-the-source-code
> 
> Followed the building steps from
https://github.com/v8/v8/wiki/Building%20with%20Gyp
> 
> export GYP_DEFINES="v8_target_arch=arm64"
> gypfiles/gyp_v8 ...
> 
> export GYP_GENERATORS=ninja
> gypfiles/gyp_v8
> ninja -C out/Debug d8

you also need target_arch=arm64

> 
> Then reading the flags from .ninja files.
> 
> > 
> > > 
> > > > > 
> > > > > But I see the code uses the definitions from this file, so I am not
sure
> > if
> > > > these flags would affect directly in the case of v8 builds :
> > > > > 
> > > > > File : src/arm/assembler-arm.cc
> > > > > 
> > > > > #ifdef CAN_USE_ARMV8_INSTRUCTIONS
> > > > 
> > > > for those, it's also important to compare the defines (-D...) flags
> > > 
> > > defines (-D) for gn
> > > ==================
> > > 
> > > defines = -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE
-DV8_DEPRECATION_WARNINGS
> > -DCLD_VERSION=2 -DENABLE_NOTIFICATIONS -DENABLE_BROWSER_CDMS
-DENABLE_PRINTING=1
> > -DENABLE_BASIC_PRINTING=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1
> > -DUSE_OPENSSL=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1
> > -DDISABLE_NACL -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
> > -DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
> > -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
> > -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1
-D_FILE_OFFSET_BITS=64
> > -DANDROID -DHAVE_SYS_UIO_H -DCOMPONENT_BUILD -D__GNU_SOURCE=1
> > -D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -D_DEBUG
> > -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DV8_SHARED
> > -DBUILDING_V8_SHARED -DV8_I18N_SUPPORT -DENABLE_HANDLE_ZAPPING
> > -DV8_USE_EXTERNAL_STARTUP_DATA -DV8_TARGET_ARCH_ARM64 -DENABLE_DISASSEMBLER
> > -DV8_ENABLE_CHECKS -DOBJECT_PRINT -DVERIFY_HEAP -DDEBUG -DOPTIMIZED_DEBUG
> > -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0
> > > include_dirs = -I../.. -Igen -I../../v8
-I../../third_party/icu/source/common
> > -I../../third_party/icu/source/i18n
> > 
> > 
> > VERIFY_HEAP, DEBUG, OBJECT_PRINT etc.. are all debug flags, but gyp appears
to
> > be a release build?
> 
> I will give a try with DEBUG build.

ideally, you'd compare the release builds for both.

> 
> 
> > 
> > > 
> > > Defines -D for gyp
> > > ==================
> > > defines = -DCR_CLANG_REVISION=268813-1 -DV8_TARGET_ARCH_ARM64 $
> > >     -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS $
> > >     -DV8_I18N_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA $
> > >     -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -DU_USING_ICU_NAMESPACE=0 $
> > >     -DU_ENABLE_DYLOAD=0 -DU_NOEXCEPT= -DU_STATIC_IMPLEMENTATION $
> > >     -DENABLE_HANDLE_ZAPPING
> > > includes = -I../.. -I../../.. -I../../third_party/icu/source/i18n $
> > >     -I../../third_party/icu/source/common
> > > 
> > > 
> > > > 
> > > > >   if (FLAG_enable_armv8) {
> > > > >     answer |= 1u << ARMv8;
> > > > >     // ARMv8 always features VFP and NEON.
> > > > >     answer |= 1u << ARMv7 | 1u << VFP3 | 1u << NEON | 1u <<
VFP32DREGS;
> > > > >     answer |= 1u << SUDIV | 1u << MLS;
> > > > >   }
> > > > > #endif  // CAN_USE_ARMV8_INSTRUCTIONS
> > > > > 
> > > > > Regards,
> > > > > Khasim
> > > > > 
> > > > > > > 
> > > > > > > Regards,
> > > > > > > Khasim
> > > > > > > 
> > > > > > > > https://codereview.chromium.org/1888763002/
> > > > > > > >
> > > > > > > 
> > > > > > > -- 
> > > > > > > You received this message because you are subscribed to the Google
> > Groups
> > > > > > "Chromium-reviews" group.
> > > > > > > To unsubscribe from this group and stop receiving emails from it,
send
> > an
> > > > > > email to mailto:chromium-reviews+unsubscribe@chromium.org.
> > > > > > >

khasim.mohammed

Please ignore all the above discussions on gyp flags, I took a fresh look at ...

4 years, 7 months ago (2016-05-12 10:56:22 UTC) #29

Please ignore all the above discussions on gyp flags, I took a fresh look at
these,

gyp :
=====
echo "{ 'GYP_DEFINES': 'OS=android target_arch=arm64 clang=1', }" >
chromium.gyp_env

File read : out/Release/obj/v8/tools/gyp/v8_base.ninja

defines = -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -D_FILE_OFFSET_BITS=64 $
    -DNO_TCMALLOC -DDISABLE_NACL -DCHROMIUM_BUILD $
    -DCR_CLANG_REVISION=263324-1 -DUSE_LIBJPEG_TURBO=1 -DENABLE_WEBRTC=1 $
    -DENABLE_MEDIA_ROUTER=1 -DENABLE_BROWSER_CDMS $
    -DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS $
    -DFIELDTRIAL_TESTING_ENABLED -DENABLE_AUTOFILL_DIALOG=1 $
    -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_SPELLCHECK=1 $
    -DUSE_BROWSER_SPELLCHECKER=1 -DENABLE_SUPERVISED_USERS=1 -DVIDEO_HOLE=1 $
    -DV8_USE_EXTERNAL_STARTUP_DATA -DENABLE_WEBVR -DSAFE_BROWSING_DB_REMOTE $
    -DV8_TARGET_ARCH_ARM64 -DV8_I18N_SUPPORT $
    -DV8_IMMINENT_DEPRECATION_WARNINGS $
    -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DU_USING_ICU_NAMESPACE=0 $
    -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DUSE_LIBPCI=1 $
    -DUSE_OPENSSL=1 -DUSE_OPENSSL_CERTS=1 -DANDROID -D__GNU_SOURCE=1 $
    '-DCHROME_BUILD_ID=""' -DHAVE_SYS_UIO_H -DNDEBUG -DNVALGRIND $
    -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DENABLE_HANDLE_ZAPPING
includes = -I../../v8 -I../.. -Igen -I../../third_party/icu/source/i18n $
    -I../../third_party/icu/source/common
cflags = --param=ssp-buffer-size=4 -Werror -fno-strict-aliasing -Wall $
    -Wno-unused-parameter -Wno-missing-field-initializers $
    -fvisibility=hidden -pipe -fPIC -Xclang -load -Xclang $
   
/home/khasim/chromium/src/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so
$
    -Xclang -add-plugin -Xclang find-bad-constructs -Xclang $
    -plugin-arg-find-bad-constructs -Xclang check-templates -Xclang $
    -plugin-arg-find-bad-constructs -Xclang follow-macro-expansion $
    -fcolor-diagnostics -Wheader-hygiene -Wno-char-subscripts $
    -Wno-unneeded-internal-declaration -Wno-covered-switch-default $
    -Wstring-conversion -Wno-c++11-narrowing -Wno-deprecated-register $
    -Wno-inconsistent-missing-override -Wno-shift-negative-value $
    -Wno-unused-variable -ffunction-sections -funwind-tables -g $
    -fno-short-enums $
   
--sysroot=../../third_party/android_tools/ndk//platforms/android-21/arch-arm64 $
    -D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -target $
    aarch64-linux-androideabi -m64 -fno-ident -fdata-sections $
    -ffunction-sections -funwind-tables -fdata-sections -ffunction-sections $
    -O2

gn:
===

File read : out/Default/obj/v8/v8_base.ninja 

defines = -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DV8_DEPRECATION_WARNINGS
-DCLD_VERSION=2 -DENABLE_NOTIFICATIONS -DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1
-DENABLE_BASIC_PRINTING=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1
-DUSE_OPENSSL=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1
-DDISABLE_NACL -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
-DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
-DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
-DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1 -D_FILE_OFFSET_BITS=64
-DANDROID -DHAVE_SYS_UIO_H -DCOMPONENT_BUILD -D__GNU_SOURCE=1
-D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -D_DEBUG
-DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DV8_SHARED
-DBUILDING_V8_SHARED -DV8_I18N_SUPPORT -DENABLE_HANDLE_ZAPPING
-DV8_USE_EXTERNAL_STARTUP_DATA -DV8_TARGET_ARCH_ARM64 -DENABLE_DISASSEMBLER
-DV8_ENABLE_CHECKS -DOBJECT_PRINT -DVERIFY_HEAP -DDEBUG -DOPTIMIZED_DEBUG
-DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0
include_dirs = -I../.. -Igen -I../../v8 -I../../third_party/icu/source/common
-I../../third_party/icu/source/i18n

cflags = -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector
-funwind-tables -fPIC -pipe -fcolor-diagnostics
-fdebug-prefix-map=/home/khasim/chromium/src=. -ffunction-sections
-fno-short-enums -target aarch64-linux-android
-mcpu=cortex-a57+simd+crypto+fp+crc -Os -fdata-sections -ffunction-sections -g1
--sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
-fvisibility=hidden -Xclang -load -Xclang
../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang
-add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs
-Xclang check-templates -Xclang -plugin-arg-find-bad-constructs -Xclang
follow-macro-expansion -Wheader-hygiene -Wstring-conversion -Werror -Wall
-Wno-unused-variable -Wno-asm-operand-widths -Wno-missing-field-initializers
-Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default
-Wno-deprecated-register -Wno-unneeded-internal-declaration
-Wno-inconsistent-missing-override -Wno-shift-negative-value
cflags_cc = -fno-threadsafe-statics -fvisibility-inlines-hidden
-Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++11
-fno-rtti
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include
-isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include
-isystem../../third_party/android_tools/ndk/sources/android/support/include
-fno-exceptions -Wno-deprecated

Let me know if you see any major issues in these.

jochen (gone - plz use gerrit)

the gyp version runs with -m64 -O2 while the gn version specifies -mcpu=cortex-a57+simd+crypto+fp+crc -Os that ...

4 years, 7 months ago (2016-05-12 11:03:14 UTC) #30

khasim.mohammed

On 2016/05/12 11:03:14, jochen (soon OOO) wrote: > the gyp version runs with -m64 -O2 ...

4 years, 7 months ago (2016-05-12 11:32:50 UTC) #31

jochen (gone - plz use gerrit)

On 2016/05/12 at 11:32:50, khasim.mohammed wrote: > On 2016/05/12 11:03:14, jochen (soon OOO) wrote: > ...

4 years, 7 months ago (2016-05-12 11:38:14 UTC) #32

khasim.mohammed

On 2016/05/12 11:38:14, jochen (soon OOO) wrote: > On 2016/05/12 at 11:32:50, khasim.mohammed wrote: > ...

4 years, 7 months ago (2016-05-12 12:45:03 UTC) #33

jochen (gone - plz use gerrit)

thanks for filing the bug. I cc'd Rodolph to verify the flags. We shouldn't land ...

4 years, 7 months ago (2016-05-12 12:55:40 UTC) #34

khasim.mohammed

On 2016/05/12 12:55:40, jochen (soon OOO) wrote: > thanks for filing the bug. I cc'd ...

4 years, 7 months ago (2016-05-12 13:23:05 UTC) #35

jochen (gone - plz use gerrit)

On 2016/05/12 at 13:23:05, khasim.mohammed wrote: > On 2016/05/12 12:55:40, jochen (soon OOO) wrote: > ...

4 years, 7 months ago (2016-05-12 13:44:28 UTC) #36

khasim.mohammed

On 2016/05/12 13:44:28, jochen (soon OOO) wrote: > On 2016/05/12 at 13:23:05, khasim.mohammed wrote: > ...

4 years, 7 months ago (2016-05-12 14:05:44 UTC) #37

Nico

One last comment, else looks good. https://codereview.chromium.org/1888763002/diff/20001/build/config/compiler/BUILD.gn File build/config/compiler/BUILD.gn (right): https://codereview.chromium.org/1888763002/diff/20001/build/config/compiler/BUILD.gn#newcode921 build/config/compiler/BUILD.gn:921: cflags += [ ...

4 years, 7 months ago (2016-05-12 14:09:04 UTC) #38

khasim.mohammed

On 2016/05/12 14:09:04, Nico wrote: > One last comment, else looks good. > > https://codereview.chromium.org/1888763002/diff/20001/build/config/compiler/BUILD.gn ...

4 years, 7 months ago (2016-05-12 18:14:26 UTC) #39

Nico

https://codereview.chromium.org/1888763002/diff/40001/build/config/compiler/BUILD.gn File build/config/compiler/BUILD.gn (right): https://codereview.chromium.org/1888763002/diff/40001/build/config/compiler/BUILD.gn#newcode981 build/config/compiler/BUILD.gn:981: "-Wno-asm-operand-widths", as requested above, can you say what warns ...

4 years, 7 months ago (2016-05-12 18:43:04 UTC) #40

khasim.mohammed

On 2016/05/12 18:43:04, Nico wrote: > https://codereview.chromium.org/1888763002/diff/40001/build/config/compiler/BUILD.gn > File build/config/compiler/BUILD.gn (right): > > https://codereview.chromium.org/1888763002/diff/40001/build/config/compiler/BUILD.gn#newcode981 > ...

4 years, 7 months ago (2016-05-12 20:45:48 UTC) #41

Nico

https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/BUILD.gn File third_party/boringssl/BUILD.gn (right): https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/BUILD.gn#newcode72 third_party/boringssl/BUILD.gn:72: # asmflags += [ "-fno-integrated-as" ] ?

4 years, 7 months ago (2016-05-12 20:46:43 UTC) #42

khasim.mohammed

On 2016/05/12 20:46:43, Nico wrote: > https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/BUILD.gn > File third_party/boringssl/BUILD.gn (right): > > https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/BUILD.gn#newcode72 > ...

4 years, 7 months ago (2016-05-12 21:24:43 UTC) #43

On 2016/05/12 20:46:43, Nico wrote:
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> File third_party/boringssl/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> third_party/boringssl/BUILD.gn:72: # asmflags += [ "-fno-integrated-as" ]
> ?

I get these errors if I enable -fno-integrated-as, I am not sure if this is
still needed for armv7 (32bit).

[2987/8045] ASM obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
FAILED: obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o 
../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF
obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o.d
-DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -DENABLE_NOTIFICATIONS
-DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1
-DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DUSE_OPENSSL=1
-DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1 -DDISABLE_NACL
-DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
-DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
-DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
-DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1 -D_FILE_OFFSET_BITS=64
-DANDROID -DHAVE_SYS_UIO_H -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
-DCOMPONENT_BUILD -D__GNU_SOURCE=1 -D__compiler_offsetof=__builtin_offsetof
-Dnan=__builtin_nan -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1
-DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../../third_party/boringssl/src/include
-I../.. -Igen -fno-integrated-as
-B../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin
-march=armv8-a+crypto -fno-strict-aliasing --param=ssp-buffer-size=4
-fstack-protector -funwind-tables -fPIC -pipe -fcolor-diagnostics
-fdebug-prefix-map=/home/khasim/chromium/src=. -ffunction-sections
-fno-short-enums -target aarch64-linux-android
-mcpu=cortex-a57+simd+crypto+fp+crc -g1
--sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64 -c
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S -o
obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:
Assembler messages:
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:32:
Error: selected processor does not support `pmull v0.1q,v20.1d,v20.1d'
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:34:
Error: selected processor does not support `pmull2 v2.1q,v20.2d,v20.2d'
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:35:
Error: selected processor does not support `pmull v1.1q,v16.1d,v16.1d'
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:41:
Error: selected processor does not support `pmull v18.1q,v0.1d,v19.1d'
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:48:
Error: selected processor does not support `pmull v0.1q,v0.1d,v19.1d'
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:73:
Error: selected processor does not support `pmull v0.1q,v20.1d,v3.1d'
../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:75:
Error: selected processor does not support `pmull2 v2.1q,v20.2d,v3.2d'

Nico

The CQ bit was checked by thakis@chromium.org to run a CQ dry run

4 years, 7 months ago (2016-05-12 21:26:52 UTC) #44

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1888763002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1888763002/60001

4 years, 7 months ago (2016-05-12 21:27:14 UTC) #45

Nico

Yes, but you probably don't want to check in a commented-out line? I know that ...

4 years, 7 months ago (2016-05-12 21:28:03 UTC) #46

Yes, but you probably don't want to check in a commented-out line?

I know that this used to be needed for 32-bit arm for boringssl. If that's
no longer the case, please remove this line in a separate CL. Or, you can
change this CL to only pass -fno-integrated-as for 32-bit arm for now (and
then possibly remove that later).

But please don't check in commented-out code.

On Thu, May 12, 2016 at 5:24 PM, <khasim.mohammed@linaro.org> wrote:

> On 2016/05/12 20:46:43, Nico wrote:
> >
>
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> > File third_party/boringssl/BUILD.gn (right):
> >
> >
>
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> > third_party/boringssl/BUILD.gn:72: # asmflags += [ "-fno-integrated-as" ]
> > ?
>
> I get these errors if I enable -fno-integrated-as, I am not sure if this is
> still needed for armv7 (32bit).
>
> [2987/8045] ASM obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> FAILED: obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> ../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF
> obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o.d
> -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -DENABLE_NOTIFICATIONS
> -DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1
> -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DUSE_OPENSSL=1
> -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1 -DDISABLE_NACL
> -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
> -DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
> -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
> -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1
> -D_FILE_OFFSET_BITS=64
> -DANDROID -DHAVE_SYS_UIO_H -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
> -DCOMPONENT_BUILD -D__GNU_SOURCE=1 -D__compiler_offsetof=__builtin_offsetof
> -Dnan=__builtin_nan -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1
> -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../../third_party/boringssl/src/include
> -I../.. -Igen -fno-integrated-as
>
>
-B../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin
> -march=armv8-a+crypto -fno-strict-aliasing --param=ssp-buffer-size=4
> -fstack-protector -funwind-tables -fPIC -pipe -fcolor-diagnostics
> -fdebug-prefix-map=/home/khasim/chromium/src=. -ffunction-sections
> -fno-short-enums -target aarch64-linux-android
> -mcpu=cortex-a57+simd+crypto+fp+crc -g1
> --sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
> -c
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S -o
> obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:
> Assembler messages:
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:32:
> Error: selected processor does not support `pmull v0.1q,v20.1d,v20.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:34:
> Error: selected processor does not support `pmull2 v2.1q,v20.2d,v20.2d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:35:
> Error: selected processor does not support `pmull v1.1q,v16.1d,v16.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:41:
> Error: selected processor does not support `pmull v18.1q,v0.1d,v19.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:48:
> Error: selected processor does not support `pmull v0.1q,v0.1d,v19.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:73:
> Error: selected processor does not support `pmull v0.1q,v20.1d,v3.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:75:
> Error: selected processor does not support `pmull2 v2.1q,v20.2d,v3.2d'
>
>
>
>
>
> https://codereview.chromium.org/1888763002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

Nico

Yes, but you probably don't want to check in a commented-out line? I know that ...

4 years, 7 months ago (2016-05-12 21:28:03 UTC) #47

Yes, but you probably don't want to check in a commented-out line?

I know that this used to be needed for 32-bit arm for boringssl. If that's
no longer the case, please remove this line in a separate CL. Or, you can
change this CL to only pass -fno-integrated-as for 32-bit arm for now (and
then possibly remove that later).

But please don't check in commented-out code.

On Thu, May 12, 2016 at 5:24 PM, <khasim.mohammed@linaro.org> wrote:

> On 2016/05/12 20:46:43, Nico wrote:
> >
>
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> > File third_party/boringssl/BUILD.gn (right):
> >
> >
>
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> > third_party/boringssl/BUILD.gn:72: # asmflags += [ "-fno-integrated-as" ]
> > ?
>
> I get these errors if I enable -fno-integrated-as, I am not sure if this is
> still needed for armv7 (32bit).
>
> [2987/8045] ASM obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> FAILED: obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> ../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF
> obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o.d
> -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -DENABLE_NOTIFICATIONS
> -DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1
> -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DUSE_OPENSSL=1
> -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1 -DDISABLE_NACL
> -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
> -DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
> -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
> -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1
> -D_FILE_OFFSET_BITS=64
> -DANDROID -DHAVE_SYS_UIO_H -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
> -DCOMPONENT_BUILD -D__GNU_SOURCE=1 -D__compiler_offsetof=__builtin_offsetof
> -Dnan=__builtin_nan -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1
> -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../../third_party/boringssl/src/include
> -I../.. -Igen -fno-integrated-as
>
>
-B../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin
> -march=armv8-a+crypto -fno-strict-aliasing --param=ssp-buffer-size=4
> -fstack-protector -funwind-tables -fPIC -pipe -fcolor-diagnostics
> -fdebug-prefix-map=/home/khasim/chromium/src=. -ffunction-sections
> -fno-short-enums -target aarch64-linux-android
> -mcpu=cortex-a57+simd+crypto+fp+crc -g1
> --sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
> -c
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S -o
> obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:
> Assembler messages:
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:32:
> Error: selected processor does not support `pmull v0.1q,v20.1d,v20.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:34:
> Error: selected processor does not support `pmull2 v2.1q,v20.2d,v20.2d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:35:
> Error: selected processor does not support `pmull v1.1q,v16.1d,v16.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:41:
> Error: selected processor does not support `pmull v18.1q,v0.1d,v19.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:48:
> Error: selected processor does not support `pmull v0.1q,v0.1d,v19.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:73:
> Error: selected processor does not support `pmull v0.1q,v20.1d,v3.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:75:
> Error: selected processor does not support `pmull2 v2.1q,v20.2d,v3.2d'
>
>
>
>
>
> https://codereview.chromium.org/1888763002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Nico

I kicked off a try run, that should tell us if 32-bit arm builds fine ...

4 years, 7 months ago (2016-05-12 21:28:20 UTC) #48

I kicked off a try run, that should tell us if 32-bit arm builds fine with
this.

On Thu, May 12, 2016 at 5:24 PM, <khasim.mohammed@linaro.org> wrote:

> On 2016/05/12 20:46:43, Nico wrote:
> >
>
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> > File third_party/boringssl/BUILD.gn (right):
> >
> >
>
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> > third_party/boringssl/BUILD.gn:72: # asmflags += [ "-fno-integrated-as" ]
> > ?
>
> I get these errors if I enable -fno-integrated-as, I am not sure if this is
> still needed for armv7 (32bit).
>
> [2987/8045] ASM obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> FAILED: obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> ../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF
> obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o.d
> -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -DENABLE_NOTIFICATIONS
> -DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1
> -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DUSE_OPENSSL=1
> -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1 -DDISABLE_NACL
> -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
> -DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
> -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
> -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1
> -D_FILE_OFFSET_BITS=64
> -DANDROID -DHAVE_SYS_UIO_H -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
> -DCOMPONENT_BUILD -D__GNU_SOURCE=1 -D__compiler_offsetof=__builtin_offsetof
> -Dnan=__builtin_nan -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1
> -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../../third_party/boringssl/src/include
> -I../.. -Igen -fno-integrated-as
>
>
-B../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin
> -march=armv8-a+crypto -fno-strict-aliasing --param=ssp-buffer-size=4
> -fstack-protector -funwind-tables -fPIC -pipe -fcolor-diagnostics
> -fdebug-prefix-map=/home/khasim/chromium/src=. -ffunction-sections
> -fno-short-enums -target aarch64-linux-android
> -mcpu=cortex-a57+simd+crypto+fp+crc -g1
> --sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
> -c
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S -o
> obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:
> Assembler messages:
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:32:
> Error: selected processor does not support `pmull v0.1q,v20.1d,v20.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:34:
> Error: selected processor does not support `pmull2 v2.1q,v20.2d,v20.2d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:35:
> Error: selected processor does not support `pmull v1.1q,v16.1d,v16.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:41:
> Error: selected processor does not support `pmull v18.1q,v0.1d,v19.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:48:
> Error: selected processor does not support `pmull v0.1q,v0.1d,v19.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:73:
> Error: selected processor does not support `pmull v0.1q,v20.1d,v3.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:75:
> Error: selected processor does not support `pmull2 v2.1q,v20.2d,v3.2d'
>
>
>
>
>
> https://codereview.chromium.org/1888763002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

Nico

I kicked off a try run, that should tell us if 32-bit arm builds fine ...

4 years, 7 months ago (2016-05-12 21:28:21 UTC) #49

I kicked off a try run, that should tell us if 32-bit arm builds fine with
this.

On Thu, May 12, 2016 at 5:24 PM, <khasim.mohammed@linaro.org> wrote:

> On 2016/05/12 20:46:43, Nico wrote:
> >
>
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> > File third_party/boringssl/BUILD.gn (right):
> >
> >
>
>
https://codereview.chromium.org/1888763002/diff/60001/third_party/boringssl/B...
> > third_party/boringssl/BUILD.gn:72: # asmflags += [ "-fno-integrated-as" ]
> > ?
>
> I get these errors if I enable -fno-integrated-as, I am not sure if this is
> still needed for armv7 (32bit).
>
> [2987/8045] ASM obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> FAILED: obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> ../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF
> obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o.d
> -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -DENABLE_NOTIFICATIONS
> -DENABLE_BROWSER_CDMS -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1
> -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DUSE_OPENSSL=1
> -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DENABLE_WEBRTC=1 -DDISABLE_NACL
> -DENABLE_CONFIGURATION_POLICY -DENABLE_SUPERVISED_USERS=1
> -DENABLE_AUTOFILL_DIALOG=1 -DVIDEO_HOLE=1 -DSAFE_BROWSING_DB_REMOTE
> -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR
> -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=263324-1
> -D_FILE_OFFSET_BITS=64
> -DANDROID -DHAVE_SYS_UIO_H -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
> -DCOMPONENT_BUILD -D__GNU_SOURCE=1 -D__compiler_offsetof=__builtin_offsetof
> -Dnan=__builtin_nan -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1
> -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../../third_party/boringssl/src/include
> -I../.. -Igen -fno-integrated-as
>
>
-B../../third_party/android_tools/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin
> -march=armv8-a+crypto -fno-strict-aliasing --param=ssp-buffer-size=4
> -fstack-protector -funwind-tables -fPIC -pipe -fcolor-diagnostics
> -fdebug-prefix-map=/home/khasim/chromium/src=. -ffunction-sections
> -fno-short-enums -target aarch64-linux-android
> -mcpu=cortex-a57+simd+crypto+fp+crc -g1
> --sysroot=../../third_party/android_tools/ndk/platforms/android-21/arch-arm64
> -c
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S -o
> obj/third_party/boringssl/boringssl_asm/ghashv8-armx64.o
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:
> Assembler messages:
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:32:
> Error: selected processor does not support `pmull v0.1q,v20.1d,v20.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:34:
> Error: selected processor does not support `pmull2 v2.1q,v20.2d,v20.2d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:35:
> Error: selected processor does not support `pmull v1.1q,v16.1d,v16.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:41:
> Error: selected processor does not support `pmull v18.1q,v0.1d,v19.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:48:
> Error: selected processor does not support `pmull v0.1q,v0.1d,v19.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:73:
> Error: selected processor does not support `pmull v0.1q,v20.1d,v3.1d'
> ../../third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S:75:
> Error: selected processor does not support `pmull2 v2.1q,v20.2d,v3.2d'
>
>
>
>
>
> https://codereview.chromium.org/1888763002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

commit-bot: I haz the power

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

4 years, 7 months ago (2016-05-12 21:43:47 UTC) #50

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: cast_shell_android on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_android/builds/65156)

4 years, 7 months ago (2016-05-12 21:43:49 UTC) #51

khasim.mohammed

On 2016/05/12 21:43:49, commit-bot: I haz the power wrote: > Dry run: Try jobs failed ...

4 years, 7 months ago (2016-05-12 21:54:43 UTC) #52

Nico

That's ok. Do you understand why the external assembler doesn't work for processing assembly files ...

4 years, 7 months ago (2016-05-12 21:57:44 UTC) #53

Nico

That's ok. Do you understand why the external assembler doesn't work for processing assembly files ...

4 years, 7 months ago (2016-05-12 21:57:44 UTC) #54

khasim.mohammed

On 2016/05/12 21:57:44, Nico wrote: > That's ok. > Thanks, posted now. > Do you ...

4 years, 7 months ago (2016-05-12 22:03:55 UTC) #55

khasim.mohammed

On 2016/05/12 22:03:55, khasim.mohammed wrote: > On 2016/05/12 21:57:44, Nico wrote: > > That's ok. ...

4 years, 7 months ago (2016-05-12 22:26:07 UTC) #56

On 2016/05/12 22:03:55, khasim.mohammed wrote:
> On 2016/05/12 21:57:44, Nico wrote:
> > That's ok.
> > 
> Thanks, posted now.
> 
> > Do you understand why the external assembler doesn't work for processing
> > assembly files with clang though? We do do arm64 builds with gcc which uses
> > the same external assembler, and it works there. Why would this be
> > different?
> >
> Agree, I am not sure why this would be different, I will look into this next
and
> update.
>  

Can you please consider the latest patch set for merge ? it might take some time
for me to understand the external assembler dependency, will work on that later
/ separately.

> > On Thu, May 12, 2016 at 5:54 PM, <mailto:khasim.mohammed@linaro.org> wrote:
> > 
> > > On 2016/05/12 21:43:49, commit-bot: I haz the power wrote:
> > > > Dry run: Try jobs failed on following builders:
> > > > cast_shell_android on tryserver.chromium.android (JOB_FAILED,
> > > >
> > >
> > >
> >
>
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...
> > > )
> > >
> > > diff --git a/third_party/boringssl/BUILD.gn
> > > b/third_party/boringssl/BUILD.gn
> > > index 9b9bada..2f8ccc6 100644
> > > --- a/third_party/boringssl/BUILD.gn
> > > +++ b/third_party/boringssl/BUILD.gn
> > > @@ -67,9 +67,11 @@ if (is_win && !is_msan) {
> > > asmflags = []
> > > include_dirs = [ "src/include" ]
> > >
> > > - if (current_cpu == "arm" && is_clang) {
> > > - # TODO(hans) Enable integrated-as (crbug.com/124610).
> > > - asmflags += [ "-fno-integrated-as" ]
> > > + if ((current_cpu == "arm" || current_cpu == "arm64") && is_clang) {
> > > + if (current_cpu == "arm") {
> > > + # TODO(hans) Enable integrated-as (crbug.com/124610).
> > > + asmflags += [ "-fno-integrated-as" ]
> > > + }
> > >
> > > Is this fine ? I can post this for now.
> > >
> > > https://codereview.chromium.org/1888763002/
> > >
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.

Nico

lgtm https://codereview.chromium.org/1888763002/diff/80001/third_party/boringssl/BUILD.gn File third_party/boringssl/BUILD.gn (right): https://codereview.chromium.org/1888763002/diff/80001/third_party/boringssl/BUILD.gn#newcode80 third_party/boringssl/BUILD.gn:80: asmflags += [ "-B${rebased_android_toolchain_root}/bin" ] The only other ...

4 years, 7 months ago (2016-05-12 22:40:23 UTC) #57

Nico

The CQ bit was checked by thakis@chromium.org to run a CQ dry run

4 years, 7 months ago (2016-05-12 22:40:31 UTC) #58

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1888763002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1888763002/80001

4 years, 7 months ago (2016-05-12 22:41:06 UTC) #59

commit-bot: I haz the power

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

4 years, 7 months ago (2016-05-13 00:07:22 UTC) #60

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 7 months ago (2016-05-13 00:07:23 UTC) #61

khasim.mohammed

On 2016/05/13 00:07:23, commit-bot: I haz the power wrote: > Dry run: This issue passed ...

4 years, 7 months ago (2016-05-13 02:59:09 UTC) #62

Nico

On 2016/05/13 02:59:09, khasim.mohammed wrote: > On 2016/05/13 00:07:23, commit-bot: I haz the power wrote: ...

4 years, 7 months ago (2016-05-13 13:26:59 UTC) #63

khasim.mohammed

The CQ bit was checked by khasim.mohammed@linaro.org

4 years, 7 months ago (2016-05-13 13:39:53 UTC) #64

khasim.mohammed

The patchset sent to the CQ was uploaded after l-g-t-m from urvang@chromium.org, jochen@chromium.org, rtoy@chromium.org Link ...

4 years, 7 months ago (2016-05-13 13:39:54 UTC) #65

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1888763002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1888763002/80001

4 years, 7 months ago (2016-05-13 13:40:06 UTC) #66

commit-bot: I haz the power

Description was changed from ========== Build 64bit browser for Android with clang for ARMv8 This ...

4 years, 7 months ago (2016-05-13 13:49:51 UTC) #68

commit-bot: I haz the power

Patchset 5 (id:??) landed as https://crrev.com/2c5ec51ea564705b02dcb6aeff6a56722cc3890f Cr-Commit-Position: refs/heads/master@{#393517}

4 years, 7 months ago (2016-05-13 13:49:52 UTC) #69

rmcilroy

rmcilroy@chromium.org changed reviewers: + rmcilroy@chromium.org

4 years, 7 months ago (2016-05-13 15:11:35 UTC) #70

rmcilroy

https://codereview.chromium.org/1888763002/diff/80001/build/config/android/BUILD.gn File build/config/android/BUILD.gn (right): https://codereview.chromium.org/1888763002/diff/80001/build/config/android/BUILD.gn#newcode93 build/config/android/BUILD.gn:93: # TODO: Enable clang support for Android Arm64. http://crbug.com/539781 ...

4 years, 7 months ago (2016-05-13 15:11:37 UTC) #71

Primiano Tucci (use gerrit)

primiano@chromium.org changed reviewers: + primiano@chromium.org

4 years, 7 months ago (2016-05-16 11:34:40 UTC) #72

Primiano Tucci (use gerrit)

https://codereview.chromium.org/1888763002/diff/80001/build/config/arm.gni File build/config/arm.gni (right): https://codereview.chromium.org/1888763002/diff/80001/build/config/arm.gni#newcode75 build/config/arm.gni:75: arm_fpu = "cortex-a57+simd+crypto+fp+crc" If I read this code correctly, ...

4 years, 7 months ago (2016-05-16 11:34:41 UTC) #73

rmcilroy

rmcilroy@chromium.org changed reviewers: + rodolph.perfetta@arm.com

4 years, 7 months ago (2016-05-16 11:47:42 UTC) #74

rmcilroy

Adding Rodolph - could you take a look at Primiano's last comment? Should we be ...

4 years, 7 months ago (2016-05-16 11:47:43 UTC) #75

martyn.capewell

Rodolph's away until next week, but I'll comment. Whilst all ARMv8 implementations should support SIMD ...

4 years, 7 months ago (2016-05-16 15:01:48 UTC) #76

Primiano Tucci (use gerrit)

On 2016/05/16 15:01:48, martyn.capewell wrote: > Rodolph's away until next week, but I'll comment. > ...

4 years, 7 months ago (2016-05-16 15:49:04 UTC) #77

martyn.capewell

On 2016/05/16 15:49:04, Primiano Tucci wrote: > So are you saying that worst cast the ...

4 years, 7 months ago (2016-05-16 16:18:10 UTC) #78

Nico

Thanks for catching that. khasim, can you revert the arm_fpu bit of this CL please?

4 years, 7 months ago (2016-05-16 16:21:58 UTC) #79

khasim.mohammed

On 16 May 2016 at 21:51, <thakis@chromium.org> wrote: > Thanks for catching that. khasim, can ...

4 years, 7 months ago (2016-05-16 16:59:23 UTC) #80

khasim.mohammed

On 16 May 2016 at 21:51, <thakis@chromium.org> wrote: > Thanks for catching that. khasim, can ...

4 years, 7 months ago (2016-05-16 16:59:24 UTC) #81

khasim.mohammed

On 2016/05/16 16:59:24, khasim.mohammed wrote: > On 16 May 2016 at 21:51, <mailto:thakis@chromium.org> wrote: > ...

4 years, 7 months ago (2016-05-17 15:46:19 UTC) #82

Primiano Tucci (use gerrit)

A general question: the title of this CL says "Build 64bit browser for Android with ...

4 years, 7 months ago (2016-05-19 15:59:00 UTC) #83

khasim.mohammed

On 19 May 2016 at 21:29, <primiano@chromium.org> wrote: > A general question: the title of ...

4 years, 7 months ago (2016-05-19 17:10:58 UTC) #84

khasim.mohammed

On 19 May 2016 at 21:29, <primiano@chromium.org> wrote: > A general question: the title of ...

4 years, 7 months ago (2016-05-19 17:10:58 UTC) #85

khasim.mohammed

On 19 May 2016 at 22:40, Khasim Syed Mohammed <khasim.mohammed@linaro.org> wrote: > > > On ...

4 years, 7 months ago (2016-05-19 17:14:00 UTC) #86

khasim.mohammed

On 19 May 2016 at 22:40, Khasim Syed Mohammed <khasim.mohammed@linaro.org> wrote: > > > On ...

4 years, 7 months ago (2016-05-19 17:14:01 UTC) #87

Raymond Toy

On 2016/05/19 at 17:14:01, khasim.mohammed wrote: > On 19 May 2016 at 22:40, Khasim Syed ...

4 years, 7 months ago (2016-05-19 17:21:53 UTC) #88

Rodolph.Perfetta_arm.com

I am out of office until the 24th of May 2016 I will reply to ...

4 years, 7 months ago (2016-05-19 17:21:59 UTC) #89

Primiano Tucci (use gerrit)

> On 2016/05/19 at 17:14:01, khasim.mohammed wrote: > > But target_cpu was arm, so it ...

4 years, 7 months ago (2016-05-19 17:26:30 UTC) #90

khasim.mohammed

On 19 May 2016 at 22:56, <primiano@chromium.org> wrote: > > On 2016/05/19 at 17:14:01, khasim.mohammed ...

4 years, 7 months ago (2016-05-19 17:29:03 UTC) #91

khasim.mohammed

On 19 May 2016 at 22:56, <primiano@chromium.org> wrote: > > On 2016/05/19 at 17:14:01, khasim.mohammed ...

4 years, 7 months ago (2016-05-19 17:29:03 UTC) #92

Primiano Tucci (use gerrit)

4 years, 7 months ago (2016-05-19 17:30:11 UTC) #93

Message was sent while issue was closed.

On 2016/05/19 17:29:03, khasim.mohammed wrote:
> > Correct, and we are not forcing anything, it's all controlled by gn args
> it can be arm or arm64.

SG it's all great. Thanks for the clarification.

Issue 1888763002: Build 64bit browser for Android with clang for ARMv8 (Closed)

Description

Patch Set 1 #

Patch Set 2 : Patch Set 2 - Build 64bit browser for Android with clang for ARMv8 #

Patch Set 3 : Patch 3 : Build 64bit browser for Android with clang for ARMv8 #

Patch Set 4 : Patch Set 4 : Removed Wno-asm-operand-widths #

Patch Set 5 : Patch Set 5: Keep -fno-integrated-as as ARMv7 needs it #

Messages