ARM: Rd and Rm registers should be different in MUL on ARMv5

[llvmbot]

Bugzilla Link	2313
Resolution	FIXED
Resolved on	Jan 01, 2011 14:38
Version	trunk
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@asl

Extended Description

The ARMv6 CPU allows the Rd and Rm registers to be the same in the MUL instruction, but this isn't allowed on, for example the ARMv5 CPU. As a result, GAS will issue complaints about LLVM-generated ARM code. Here's a sample:

sample.s: Assembler messages:
sample.s:59: Rd and Rm should be different in mul
sample.s:63: Rd and Rm should be different in mul
sample.s:104: Rd and Rm should be different in mul
sample.s:108: Rd and Rm should be different in mul

These messages came from assembling the following LLVM-generated code:

.globl	rand_r
.align	2

rand_r:
ldr r3, [r0]
ldr r2, .LCPI3_0
cmp r3, #​0
moveq r3, r2
ldr r2, .LCPI3_1
umull r1, r2, r3, r2
sub r1, r3, r2
add r2, r2, r1, lsr #​1
mov r2, r2, lsr #​16
mov r1, #​197, 30 @ 788
orr r1, r1, #​2, 22 @ 2048
mul r1, r2, r1
ldr r12, .LCPI3_2
mul r2, r2, r12 ; <------------ Here is line 59.
sub r3, r3, r2
mov r2, #​167
orr r2, r2, #​65, 24 @ 16640
mul r3, r3, r2 ; <------------ Here is line 63.
sub r3, r3, r1
sub r2, r3, #​6, 2 @ -2147483647
cmp r3, #​0
movge r2, r3
str r2, [r0]
bic r0, r2, #​2, 2 @ -2147483648
bx lr
.LBB3_1:
.LCPI3_0:
.long 123459876

.LCPI3_1:
.long	110892733

.LCPI3_2:
.long	127773

Yes, this really is the rand_r() function from the standard C library.

This code is fine for ARMv6, but it doesn't work for the ARMv5 CPU I'm playing with, so here is the bug report.

[lattner]

We used to have a pass that fixed up multiplies. I'm not really sure what its state is though.

[llvmbot]

This pass was removed by Evan during the merge of apple code because the ARMv6 manual says that the most armv5 don't have this constraint.

The manual:
Specifying the same register for and was previously described as
producing UNPREDICTABLE results. There is no restriction in ARMv6, and it is
believed all relevant ARMv4 and ARMv5 implementations do not require this
restriction either, because high performance multipliers read all their operands prior to writing back any results.

[lattner]

Ah right. IIRC, the assemblers were rejecting it, even though it would work in the actual hardware. Have you tried assembling with v6 enabled?

[llvmbot]

The assembler is generating machine code, but I haven't tried executing it to see whether it behaves correctly on the ARM926ejs part I'm planning to use.

If I pass -march=armv6 to the assembler, then it stops giving me these messages and generates identical machine code, but the object file is slightly different: in the case where I get the warnings, the private flags in the ELF header correctly has the software floating point bit set, and in the case with the -march=armv6 flag, the software floating point bit is cleared.

If I pass -march=armv6 -mfloat-abi=soft, then it stops giving me these messages and generates an identical object file.

So, I guess I might have a work-around for what ultimately seems like a difficult to categorize problem, i.e. is it the assembler being overly pedantic about the ARMv5 spec? or, is it LLVM being overly aggressive about optimizing for the most common subset of ARMv5 parts in the wild?

I can certainly put a comment in my code that explains why I'm passing -mcpu=arm926ej-s to llc, and passing -march=armv6 -mfloat-abi=soft to the assembler. It will be a little while before I find out whether my CPU is exploiting this escape clause in the ARMv5 specification to keep its multiplier small. I'll come back when I have a functioning bootstrap loader, and I'll update this bug to reflect what I find.

[lattner]

FWIW, this should be easy to model now. Just use the early clobber MachineInstr constraint on the result. The problem is that tblgen doesn't support syntax for this yet, but it should be relatively easy to add if you're interested.

[llvmbot]

Could this be added for ARMv4 and ARMv5 ?

Even if some implementation support it, the arm arm manual say that it is unpredictable on these architectures. It is not worth triggering strange hardware behavior by using it.

[asl]

Could this be added for ARMv4 and ARMv5 ?

Even if some implementation support it, the arm arm manual say that it is
unpredictable on these architectures. It is not worth triggering strange
hardware behavior by using it.
Yes, I'm going to fix this. Using earlyclobber is not optimal, but it will provide a working code! :)

[asl]

Patch to test
Given that there is no testcase here, I cannot test stuff fully. Could please anyone try the patch ttached and let me know whether it works or not?

[llvmbot]

Hi Anton,

I applied the patch and it does get rid of the MUL messages that I was seeing. However, during tablegen I get:
Decoding Conflict:
____0000000.........____1001....
____0000000.............
____0000000.....................
____0000..0.....................
____0000........................
____00..........................
____............................
................................
MUL 0000000_____________1001
MULv5 0000000_____________1001
Decoding Conflict:
____0000100.........____1001....
____0000100.............
____0000100.....................
____0000..0.....................
____0000........................
____00..........................
____............................
................................
UMULL 0000100_____________1001
UMULLv5 0000100_____________1001
Decoding Conflict:
____0000110.........____1001....
____0000110.........________....
____0000110.....................
...

[asl]

Hi Anton,

I applied the patch and it does get rid of the MUL messages that I was seeing.
However, during tablegen I get:
Yeah, this is expected. Final patch will get rid of this.

[llvmbot]

Test case.

[asl]

Should be fixed in r122663

[DMG862]

mentioned in issue llvm/llvm-bugzilla-archive#48980

[asl]

mentioned in issue llvm/llvm-bugzilla-archive#8157