llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.97k stars 11.54k forks source link

The Arm `.thumb_func` directive should imply `.thumb` (at least for non-Mach-O targets) #37907

Open iains opened 6 years ago

iains commented 6 years ago
Bugzilla Link 38559
Version trunk
OS All
Attachments Recognise -mthumb for assembler tasks.
CC @efriedma-quic,@TNorthover

Extended Description

Ran into this trying to build newlib.

case 1;

we invoke with

 ./bin/clang -target arm-none-linux-gnu -mcpu=cortex-a7 -mfpu=neon-vfpv4 -mfloat-abi=hard -mthumb /src/test-arm/thumb-mode.S -c

The driver constructs two jobs (1) to pre-process the .S file and (2) to assemble the resulting .s file.

the ivocation for the first job is correct ; the "triple" is updated to "thumbv7".

./bin/clang -cc1 -triple thumbv7-none-linux-gnu -E -save-temps=cwd 

the invocation for the second is not : the triple is not updated :

./bin/clang -cc1as -triple armv7-none-linux-gnu

case 2:

./bin/clang -target arm-none-linux-gnu -mcpu=cortex-a7 -mfpu=neon-vfpv4 -mfloat-abi=hard -mthumb thumb-mode.s -c -save-temps

the 'triple' is again not updated to include the thumb component - the cc1as mode does not recognise -mthumb (but only -Wa/Xassembler -mthumb).

output in both cases:

/src/test-arm/thumb-mode.S:16:2: error: instruction requires: thumb
 cbnz r0, .Ltail

===

proposed fix (attached) - recognise the -mthumb flag for assembler tasks this is consistent with other tools.

alternate fix ; the driver could intercept the thumb flag and push back the Xassembler -mthumb option. This seems more complex.

===

However, since the current behaviour seems deliberate (there is a driver test case matching it) - so it's not clear what the intent should be (e.g. does we expect the user to enter -mthumb -Wa,-mthumb ?)

===

test-case

$ more /src/test-arm/thumb-mode.S
#define synd            r0

        .syntax unified
        .arch   armv7-a
        .fpu    neon

        .text
        .thumb_func

        .global foo
        .type foo,%function
        .cfi_startproc

foo:

        cbnz synd, .Ltail
        nop
        nop
        nop
.Ltail:
        nop

        bx      lr

        .cfi_endproc
        .size   foo, . - foo
iains commented 6 years ago

Recognize .thumb_func as implying .thumb (for non-Mach-o) Well, "odd behaviour" is somewhat subjective. From the User's PoV it might appear odd that a function marked as thumb code could be assembled for Arm (at present, it will only error on thumb-specific constructs).

===

The binutils on-line documentation is here: https://sourceware.org/binutils/docs/as/ARM-Directives.html#ARM-Directives

and, clearly, newlib (from which this example was abstracted) builds with the GCC/BINUTILS toolchain.

===

So I will re-title and the replacement patch attached does the following:

1) no change for Mach-O (maybe Tim or someone should review and decide if it's needed there).

2) for all other cases (mimic the .thumb behaviour)

The example code compiles with this, and no changes in the testsuite on the 700rc1.

efriedma-quic commented 6 years ago

Yes, ARMAsmParser::parseDirectiveThumbFunc doesn't switch to Thumb mode. Sure, I'd consider that a bug if LLVM doesn't match binutils.

Granted, that's a pretty weird rule, but assembler has a lot of weird rules.

iains commented 6 years ago

"-mthumb" means "translate C code to Thumb code, unless overridden with an attribute on a specific function". "-Wa,-mthumb" means "pretend there's a .code 16 directive at the beginning of assembly files". If you want both, yes, you should write "-mthumb -Wa,-mthumb".

gcc behaves the same way.

Well, I see what you mean - but actually GCC does not require -Wa,-mthumb to preprocess/assemble this code. There is already an equivent directive in the source.

It seems that the discrepancy is about how .thumb_func is treated.

Quoting from the GAS manual

.thumb_func This directive specifies that the following symbol is the name of a Thumb encoded function. This information is necessary in order to allow the assembler and linker to generate correct code for interworking between Arm and Thumb instructions and should be used even if interworking is not going to be performed. The presence of this directive also implies .thumb

===

(since .thumb is equivalent to .code 16)

It seems that our current Arm backend requires either an explicit .thumb or to be invoked as thumbxx-yyyy-zzzz

So, perhaps the bug should be revised to "arm backend does not treat '.thumb_func' as implying '.thumb'"?

(unless there's some ruling from the Arm folks that this is no longer true, and the original code should be amended to have an explicit '.thumb'/'.code 16').

efriedma-quic commented 6 years ago

-mthumb means "translate C code to Thumb code, unless overridden with an attribute on a specific function". -Wa,-mthumb means "pretend there's a .code 16 directive at the beginning of assembly files". If you want both, yes, you should write -mthumb -Wa,-mthumb.

gcc behaves the same way.

llvmbot commented 1 month ago

@llvm/issue-subscribers-backend-arm

Author: Iain Sandoe (iains)

| | | | --- | --- | | Bugzilla Link | [38559](https://llvm.org/bz38559) | | Version | trunk | | OS | All | | Attachments | [Recognise -mthumb for assembler tasks.](https://user-images.githubusercontent.com/4039407/143757936-a44363bb-419a-41fa-a2fc-9096325e63bc.gz) | | CC | @efriedma-quic,@TNorthover | ## Extended Description Ran into this trying to build newlib. case 1; we invoke with ``` ./bin/clang -target arm-none-linux-gnu -mcpu=cortex-a7 -mfpu=neon-vfpv4 -mfloat-abi=hard -mthumb /src/test-arm/thumb-mode.S -c ``` The driver constructs two jobs (1) to pre-process the `.S` file and (2) to assemble the resulting `.s` file. the ivocation for the first job is correct ; the "triple" is updated to "thumbv7". ``` ./bin/clang -cc1 -triple thumbv7-none-linux-gnu -E -save-temps=cwd ``` the invocation for the second is not : the triple is not updated : ``` ./bin/clang -cc1as -triple armv7-none-linux-gnu ``` case 2: ``` ./bin/clang -target arm-none-linux-gnu -mcpu=cortex-a7 -mfpu=neon-vfpv4 -mfloat-abi=hard -mthumb thumb-mode.s -c -save-temps ``` the 'triple' is again not updated to include the thumb component - the `cc1as` mode does not recognise `-mthumb` (but only `-Wa/Xassembler -mthumb`). output in both cases: ``` /src/test-arm/thumb-mode.S:16:2: error: instruction requires: thumb cbnz r0, .Ltail ``` === proposed fix (attached) - recognise the `-mthumb` flag for assembler tasks this is consistent with other tools. alternate fix ; the driver could intercept the thumb flag and push back the `Xassembler -mthumb` option. This seems more complex. === However, since the current behaviour seems deliberate (there is a driver test case matching it) - so it's not clear what the intent should be (e.g. does we expect the user to enter `-mthumb -Wa,-mthumb` ?) === test-case ```console $ more /src/test-arm/thumb-mode.S ``` ```asm #define synd r0 .syntax unified .arch armv7-a .fpu neon .text .thumb_func .global foo .type foo,%function .cfi_startproc foo: cbnz synd, .Ltail nop nop nop .Ltail: nop bx lr .cfi_endproc .size foo, . - foo ```