New issue
Advanced search Search tips

Issue 671089 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 537368



Sign in to add a comment

ARM Clang does not accept command line option -march=armv7ve

Project Member Reported by hashimoto@chromium.org, Dec 5 2016

Issue description

(From issue 670242)
GCC expects -march=armv7ve when -mcpu=cortex-a7/12/15.
Specifying -march=armv7-a with these -mcpu options results in a warning (which we treat as an error) as GCC thinks they are incompatible.

OTOH, clang does not accept -march=armv7ve as an valid option.
This means there is no way to satisfy both GCC and clang at the same time.
 
Clang output:

(sdk veyron_minnie R57-9047.0.0) hashimoto@hashimoto ~/chrome/src $ armv7a-cros-linux-gnueabi-clang++ --version
Chromium OS 3.9_pre265926-r16 clang version 3.9.0 (/var/cache/chromeos-cache/distfiles/host/egit-src/clang.git af6a0b98569cf7981fe27327ac4bf19bd0d6b162) (/var/cache/chromeos-cache/distfiles/host/egit-src/llvm.git 26a9873b72c6dbb425ae075fcf51caa9fc5e892b) (based on LLVM 3.9.0svn)
Target: armv7a-cros-linux-gnueabi
Thread model: posix
InstalledDir: /usr/local/ssd2/chrome/.cros_cache/chrome-sdk/tarballs/veyron_minnie+9047.0.0+target_toolchain/usr/bin

(sdk veyron_minnie R57-9047.0.0) hashimoto@hashimoto ~/chrome/src $ armv7a-cros-linux-gnueabi-clang++ -mfpu=neon-vfpv4 -march=armv7ve -c foo.cc
clang-3.9: error: the clang compiler does not support '-march=armv7ve
Owner: manojgupta@chromium.org
Hashimoto, Can you try to use mtune=cortex-a7/12/15 instead and see if it resolves your problem.

I did not see any warnings when using march + mtune.

(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-g++ -mfpu=neon-vfpv4 -march=armv7-a -mtune=cortex-a7 -c foo.cc
(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-g++ -mfpu=neon-vfpv4 -march=armv7-a -mtune=cortex-a12 -c foo.cc
(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-g++ -mfpu=neon-vfpv4 -march=armv7-a -mtune=cortex-a15 -c foo.cc

(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-clang++ -mfpu=neon-vfpv4 -march=armv7-a -mtune=cortex-a7 -c foo.cc
(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-clang++ -mfpu=neon-vfpv4 -march=armv7-a -mtune=cortex-a12 -c foo.cc
(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-clang++ -mfpu=neon-vfpv4 -march=armv7-a -mtune=cortex-a15 -c foo.cc


Summary: ARM Clang does not accept command line option -march=armv7ve (was: ARM Clang does not accept command line option -march=arcv7ve)
you use "armv7a" every time -- the request is to support "armv7ve"
I see, I got confused by the description where the issue seems to be gcc throwing warning when using armv7-a with mcpu. 
For issue 670242, we need to use -march=armv7ve and -mcpu=coretex-a7/12/15.
-mtune doesn't change the set of instructions used by compilers.

If clang supports -march=armv7ve, we'll be able to get rid of the workaround logic in https://chromium-review.googlesource.com/#/c/415810/.
Hashimoto, Mike: 
I have made a change to sysroot_wrapper to make clang accept -march=armv7ve.
HW divide instruction is generated with clang with the change.
is it sufficient for you or do you prefer a real clang change to support armv7ve? 

I'll ask upstream anyways if they have plans to support this flag but any upstream change may take time.

With sysroot_wrapper change:

(cr) manojgupta@manojgupta ~ $ cat foo.cc
int divide(int b, int c) {
	return b/c;
}

(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-g++ -mfpu=neon-vfpv4 -march=armv7ve -mcpu=cortex-a15 -c foo.cc 
(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-objdump  -D foo.o -j .text
foo.o:     file format elf32-littlearm
Disassembly of section .text:
00000000 <_Z6divideii>:
   0:	f84d 7d04 	str.w	r7, [sp, #-4]!
   4:	b083      	sub	sp, #12
   6:	af00      	add	r7, sp, #0
   8:	6078      	str	r0, [r7, #4]
   a:	6039      	str	r1, [r7, #0]
   c:	687a      	ldr	r2, [r7, #4]
   e:	683b      	ldr	r3, [r7, #0]
  10:	fb92 f3f3 	sdiv	r3, r2, r3
  14:	4618      	mov	r0, r3
  16:	370c      	adds	r7, #12
  18:	46bd      	mov	sp, r7
  1a:	f85d 7b04 	ldr.w	r7, [sp], #4
  1e:	4770      	bx	lr

(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-clang++ -mfpu=neon-vfpv4 -march=armv7ve -mcpu=cortex-a15 -c foo.cc 
(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-objdump  -D foo.o -j 
.text

foo.o:     file format elf32-littlearm
Disassembly of section .text:
00000000 <_Z6divideii>:
   0:	b084      	sub	sp, #16
   2:	460a      	mov	r2, r1
   4:	4603      	mov	r3, r0
   6:	9003      	str	r0, [sp, #12]
   8:	9102      	str	r1, [sp, #8]
   a:	9803      	ldr	r0, [sp, #12]
   c:	fb90 f0f1 	sdiv	r0, r0, r1
  10:	9201      	str	r2, [sp, #4]
  12:	9300      	str	r3, [sp, #0]
  14:	b004      	add	sp, #16
  16:	4770      	bx	lr

i don't think it's appropriate for the sysroot wrapper to do things like this.  clang should be responsible for flag processing, not the wrapper.
We are going to send an email to arm to see if they can add this to clang. 

Btw, you can just drop the -march=armv7ve totally and only use mcpu=cortex-a15. I verified that this still generates the HW divide instruction.

(cr) manojgupta@manojgupta ~ $ cat foo.cc 
int divide(int b, int c) {
	return b/c;
}
(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-gcc -mcpu=cortex-a15 -c foo.cc 
(cr) manojgupta@manojgupta ~ $ armv7a-cros-linux-gnueabi-objdump -D foo.o -j .text

foo.o:     file format elf32-littlearm


Disassembly of section .text:

00000000 <_Z6divideii>:
   0:	f84d 7d04 	str.w	r7, [sp, #-4]!
   4:	b083      	sub	sp, #12
   6:	af00      	add	r7, sp, #0
   8:	6078      	str	r0, [r7, #4]
   a:	6039      	str	r1, [r7, #0]
   c:	687a      	ldr	r2, [r7, #4]
   e:	683b      	ldr	r3, [r7, #0]
  10:	fb92 f3f3 	sdiv	r3, r2, r3
  14:	4618      	mov	r0, r3
  16:	370c      	adds	r7, #12
  18:	46bd      	mov	sp, r7
  1a:	f85d 7b04 	ldr.w	r7, [sp], #4
  1e:	4770      	bx	lr



Comment 10 by lloz...@google.com, Dec 10 2016

I will ask ARM if they will support adding -march=armv7ve.

but I think we can avoid this problem if we use -mcpu=cortex-a15 without specifying -march. 


Thanks!

Currently, with clang not supporting -march=armv7ve, we ended up having a workaround logic to set -march=armv7ve only when GCC is used.
(https://chromium-review.googlesource.com/#/c/415810/7/chromeos-base/chromeos-chrome/chromeos-chrome-9999.ebuild)
This is because Chrome's GN prefers -march to -mcpu (maybe it makes development easier for x86/x64?) so it doesn't allow leaving -march unspecified.
Opened upstream bug https://llvm.org/bugs/show_bug.cgi?id=31358 and posted a sample patch.
Cc: hashimoto@chromium.org
Sent to upstream llvm for review @ https://reviews.llvm.org/D29472
Corresponding clang change is reviewed @ https://reviews.llvm.org/D29773
Owner: llozano@chromium.org
The changes have been merged upstream. 

Clang upstream commit hash: b7945c107e8ac83138ec52e4d0815ef9249df39b
LLVM upstream commit hash: e79ec29260ee601961d073f0fa43db7be81b51cc

Assigning to Luis to decide on the next course of action for this: 
1. Do we want to pick the upstream changes right now to enable this?
2) Wait for next llvm upgrade?

hashimoto@, Can you pick the two upstream commits and verify if they work for you.
I am hoping we will do a roll to new LLVM soon (a month or so). So, I dont think we need to cherry pick these changes to our current LLVM.

Thanks manojgupta@, with ToT clang I verified with the following command that clang accepts -march=armv7ve:
  clang -target arm-linux-gnueabi -march=armv7ve -mcpu=cortex-a15 -c foo.cc
Owner: manojgupta@chromium.org
Cc: manojgupta@chromium.org
Blocking: 537368
Owner: hashimoto@chromium.org
hashimoto@, Please verify if this is fixed by update.
Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)

Sign in to add a comment