Deprecated instruction in arch v8r?

[llvmbot]

Bugzilla Link	45353
Version	8.0
OS	Linux
Attachments	Input file
Reporter	LLVM Bugzilla Contributor
CC	@DavidSpickett

Extended Description

Compiling with the command will generate 2 warnings. When the assembly file .s is checked, it is found that there are deprecated instructions for the architecture v8r. Actually this deprecated instruction was generated by ourselves.

Command: clang -c -Wall --target=arm-none-eabi -mcpu=cortex-r52 -mfpu=fpv5-sp-d16 -mthumb -fshort-enums -Os -save-temps ee_cortex_r_hal.i

ee_cortex_r_hal.s:150:2: warning: deprecated instruction in IT block movhi.w r0, #​384 ^ ee_cortex_r_hal.s:191:2: warning: deprecated instruction in IT block movhi.w r0, #​640

$ clang --version 4.0.0 clang version 8.0.1 (based on LLVM 8.0.1) Target: aarch64-unknown-linux-gnu Thread model: posix

[DavidSpickett]

Great. Sounds like you already know your way around but just in case, the warning is emitted here: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp#L10788

Further clarity on GCC's option -mrestrict-it. It's "on" by default but internally almost all the CPUs override it. So if you explicitly enable it, it's on otherwise it's almost always off. See: https://gcc.gnu.org/legacy-ml/gcc-patches/2019-12/msg00134.html

I think this fits with Sam Parker's direction in the linked patch.

[llvmbot]

Thanks a lot. This

I talked to my colleagues at Arm and it turns out that this deprecation has not been followed by the majority of cores. gas does not warn for this situation by default anymore for this reason. Thanks a lot. It's the same as what I think about. Control IT related warnings is the best way.

[DavidSpickett]

Relevant Binutils commit: commit 24f19ccb8907b8d2bafb905a5db1a3537084d522 Author: Andre Vieira andre.simoesdiasvieira@arm.com Date: Wed Dec 11 15:53:26 2019 +0000

[gas][arm] Add -mwarn-restrict-it

Add a -m{no-}warn-restrict-it option to control IT related warnings in
ARMv8-A and ARMv8-R.  This is disabled by default.

Committed on behalf of Wilco Dijkstra.

[DavidSpickett]

I talked to my colleagues at Arm and it turns out that this deprecation has not been followed by the majority of cores. gas does not warn for this situation by default anymore for this reason.

$ cat /tmp/test.s .text .syntax unified .thumb_func fn1: it hi movhi.w r0, #​640

b   fn1

(older gcc build) $ arm-none-eabi-gcc -c -march=armv8-a -mthumb /tmp/test.s -o /tmp/test.o /tmp/test.s: Assembler messages: /tmp/test.s:6: IT blocks containing 32-bit Thumb instructions are deprecated in ARMv8

(newer gcc build) $ ./arm-none-eabi-gcc -c -march=armv8-a -mthumb /tmp/test.s -o /tmp/test.o

You can opt into the checking by adding an option to GAS: (there's -mrestrict-it for code gen for GCC) $ ./arm-none-eabi-gcc -c -march=armv8-a -mthumb /tmp/test.s -o /tmp/test.o -Wa,-mwarn-restrict-it /tmp/test.s: Assembler messages: /tmp/test.s:6: IT blocks containing 32-bit Thumb instructions are performance deprecated in ARMv8-A and ARMv8-R There is a mention in the ARMARM of a bit that can be set in hardware to disable these instructions but as far as I can tell that is purely for debugging purposes in the case that there were an implementation that cared about this. So I think a reasonable way forward would be to make this restriction off by default in clang as it is in gas, for assembly. No 32->16 bit conversion necessary. On the codegen side of things one of my colleagues posted this while back if you're interested: https://reviews.llvm.org/D71992 Maybe it fits with your current direction, maybe not. Do you agree that simply disabling the warning when assembling makes sense at least?

[llvmbot]

Split it into 16 bit versions is not a good way, we could not confirm that every instr splited is eligible. Maybe we should check if we should produce IT Block more earlier? AND Is there any other better way?

[llvmbot]

I try to split it into 16 bit versions, like here:

t2MOVi(imm > 256) -> tMOVi16 + tMOVr. (tMOVr is eligible)

but tMOVi16 (conditional) is also not eligible. So we get another same warning... movhi.w r2, #​380

Another problem is that we should one virtual register(tGPR) to get the imm, but it core dump on TraceDefUses()

May have something wrong.

[DavidSpickett]

Doh. My understanding of IT blocks is lacking. With my suggested change: example.s:41:2: error: predicated instructions must be in IT block movgt.w r2, #​640 ^

The instruction must be in a block. So yes you're right, splitting it into 16 bit versions would be the way to go.

[DavidSpickett]

We can at least use the same check the assembler does to not build an IT block around the wide instruction once it has been selected. (isV8EligibleForIT)

I'm trying that locally.

Splitting them into 16 bit ops is going to depend on what we're doing. As the other v8 IT restriction is there can only be one conditional instruction in the block. lib/Target/ARM/Thumb2ITBlockPass.cpp: // v8 IT blocks are limited to one conditional op unless

movhis seem better candidates than the movgt for example. Would be interesting to see what the effects are.

[llvmbot]

It seems that we missed some instruction's judgement.

There are two ways:

1. As you say, to check that the instruction we're putting in the block is not a wide encoding, but how could we get the useful 16bit instructions?

2. We can specially reduce the 32bit instruction in RuduceSpecial(), but we don't know the other instructions we miss to check. Like here, I am not sure it's correct: t2MOVi(imm > 256) -> tMOVi16 + tMOVr. (tMOVr is eligible).

[DavidSpickett]

Yes it looks like restrictIT is on but by the time we make the IT block, the wide instruction is already there and it gets wrapped up.

lib/Target/ARM/Thumb2ITBlockPass.cpp: // v8 IT blocks are limited to one conditional op unless -arm-no-restrict-it // is set: skip the loop if (!restrictIT) {

This means we only get one conditional in the IT block, but it's the wide encoding from earlier and as you said we already tried to use the smaller encoding. So we can't just replace it.

One way to go is to also check that the instruction we're putting in the block is not a wide encoding.

This might have some performance implications on certain cores, though if it does it's because we're relying on this implicit detail. I'll ask around.

[llvmbot]

We made some progress:

When we get MI ARM::t2MOVi #imm , Thumb2SizeReduction.cpp try to reduce the size of MI.

But when imm > 255, we could not reduce to tMOVi8. t2MOVi is reserved.

However, t2MOVi is deprecated in IT instruction in v8. Bugpoint might help there.

Notes in ARMv8-A Architecture Reference Manual, section "Partial deprecation of IT" https://usermanual.wiki/Pdf/ARM20Architecture20Reference20ManualARMv8.1667877052.pdf

"ARMv8-A deprecates some uses of the T32 IT instruction. All uses of IT that apply to instructions other than a single subsequent 16-bit instruction from a restricted set are deprecated, as are explicit references to the PC within that single 16-bit instruction. This permits the non-deprecated forms of IT and subsequent instructions to be treated as a single 32-bit conditional instruction."

[DavidSpickett]

Reproduced on trunk clang.

I creduced this, looking for movhi.w in the warnings and got:

$ cat example.i a; fn1() { if (fn1 >= 2) { fn2(640, a); } else fn2(0, a); }

(C warnings removed) $ /work/open_source/nightly-llvm-debug/llvm-build/bin/clang -c -Wall --target=arm-none-eabi -mcpu=cortex-r52 -mfpu=fpv5-sp-d16 -mthumb -fshort-enums -Os -save-temps example.i <...> 6 warnings generated. example.s:41:2: warning: deprecated instruction in IT block movhi.w r0, #​640 ^

Fixing those C warnings will get you a movgt.w: $ cat example.i int a; void fn2(int, int); void fn1(int b) { if (b >= 2) { fn2(640, a); } else fn2(0, a); } $ /work/open_source/nightly-llvm-debug/llvm-build/bin/clang -c -Wall --target=arm-none-eabi -mcpu=cortex-r52 -mfpu=fpv5-sp-d16 -mthumb -fshort-enums -Os -save-temps example.i example.s:39:2: warning: deprecated instruction in IT block movgt.w r2, #​640 ^

We can guess that these are coming from the same decision point. I have not looked for that yet, bugpoint might help there.

[llvmbot]

@llvm/issue-subscribers-backend-arm

Author: None (llvmbot)

| | | | --- | --- | | Bugzilla Link | [45353](https://llvm.org/bz45353) | | Version | 8.0 | | OS | Linux | | Attachments | [Input file](https://user-images.githubusercontent.com/60944935/143760541-1961fa5d-407d-45fb-a433-74e58881d916.gz) | | Reporter | LLVM Bugzilla Contributor | | CC | @DavidSpickett | ## Extended Description Compiling with the command will generate 2 warnings. When the assembly file .s is checked, it is found that there are deprecated instructions for the architecture v8r. Actually this deprecated instruction was generated by ourselves. Command: clang -c -Wall --target=arm-none-eabi -mcpu=cortex-r52 -mfpu=fpv5-sp-d16 -mthumb -fshort-enums -Os -save-temps ee_cortex_r_hal.i ee_cortex_r_hal.s:150:2: warning: deprecated instruction in IT block movhi.w r0, #&#8203;384 ^ ee_cortex_r_hal.s:191:2: warning: deprecated instruction in IT block movhi.w r0, #&#8203;640 $ clang --version 4.0.0 clang version 8.0.1 (based on LLVM 8.0.1) Target: aarch64-unknown-linux-gnu Thread model: posix