ARM thumb-2 instruction used for non-thumb2 CPUs

Hi,

I just realized that clang produces Thumb-2 instruction in code even when older CPU type which doesn't suport Thumb-2 is specified.

Here is output:

# /opt/llvm/bin/clang -S -ccc-host-triple arm-unknown-freebsd -mcpu=arm926ej-s -mfloat-abi=soft -v -o rrx.S rrx.c
clang version 3.0 (http://llvm.org/git/clang.git 98138cdfdee05c0afbab2b209ce8cfe4a52474e1)
Target: arm-unknown-freebsd
Thread model: posix
"/opt/llvm/bin/clang" -cc1 -triple armv5e-unknown-freebsd -S -disable-free -main-file-name rrx.c -mrelocation-model static -mdisable-fp-elim -mconstructor-aliases -target-abi apcs-gnu -target-cpu arm926ej-s -msoft-float -mfloat-abi soft -target-feature +soft-float -target-feature +soft-float-abi -target-feature -neon -target-linker-version 123.2 -momit-leaf-frame-pointer -v -coverage-file rrx.S -resource-dir /opt/llvm/bin/../lib/clang/3.0 -ferror-limit 19 -fmessage-length 162 -fno-signed-char -fgnu-runtime -fdiagnostics-show-option -fcolor-diagnostics -o rrx.S -x c rrx.c
clang -cc1 version 3.0 based upon llvm 3.0svn hosted on x86_64-apple-darwin10.7.0
#include "..." search starts here:
#include <...> search starts here:
/opt/llvm/bin/../lib/clang/3.0/include
/usr/include
End of search list.

Code:

void bla (long long a)
{
  if (a & ((unsigned long long)1 << 24))
          a >>= 1;
}

generated .S:

bla:
  str r11, [sp, #-4]!
  mov r11, sp
  sub sp, sp, #20
  bic sp, sp, #7
  str r1, [sp, #12]
  str r0, [sp, #8]
  ldrb r2, [sp, #11]
  tst r2, #1
  str r0, [sp, #4]
  str r1, [sp]
  beq .LBB0_2
  b .LBB0_1
.LBB0_1:
  ldr r0, [sp, #8]
  ldr r1, [sp, #12]
  asrs r1, r1, #1
  rrx r0, r0 << Thumb-2 instruction
  str r1, [sp, #12]
  str r0, [sp, #8]
.LBB0_2:
  mov sp, r11
  ldr r11, [sp], #4
  bx lr

rrx r0,r0 is thumb-2 instruction, however target-cpu is set to arm926ej-s which is ARMv5E architecture and it doesn't support Thumb-2.

Is this a known issue?

Thanks,

Damjan

Hi

I just realized that clang produces Thumb-2 instruction in code even when older CPU type which doesn't suport Thumb-2 is specified.

Here is output:

# /opt/llvm/bin/clang -S -ccc-host-triple arm-unknown-freebsd -mcpu=arm926ej-s -mfloat-abi=soft -v -o rrx.S rrx.c
clang version 3.0 (http://llvm.org/git/clang.git 98138cdfdee05c0afbab2b209ce8cfe4a52474e1)
Target: arm-unknown-freebsd
Thread model: posix
"/opt/llvm/bin/clang" -cc1 -triple armv5e-unknown-freebsd

It seems you're generating arm, not thumb code.

Exactly i want to generate ARM code and I got "RRX r0,r0" which is Thumb-2.

Even though you specified cpu as arm9, it's probably generating
generic ARM IR (use -emit-llvm -S and see), which defaults to ARM
instructions.

If you want thumb, use triple = thumb-unknown-freebsd, but still, that
will have problems. You can do it in two steps (clang -> IR, llc ->
ASM) and specify all options on both steps, or wait until the clang
patch for cross-compilation is reviewed and applied.

cheers,
--renato

Hi Renato,

I dont want to generate Thumb code, actually i missed in my previous email that "rrx r0,r0" is both ARM and thumb-2 instruction, but only defined in ARMv7.

llvm produces "rrx r0,r0" mnemonics for ARMv5 targets, however ARM Architecture Reference Manual for ARMv5 doesn't know about that mnemonics. There is equivalent mnemonics on ARMv5: "mov r0,r0, rrx"

both "rrx r0,r0" and "mov r0, r0, rrx" result in same opcode: 0xe1a00060.

Problem is that in case when old binutils are used (in my case freebsd is using old one due to license upgrade to GPLv3) AS doesn't understand new mnemonics and fails.

Can we change to old mnemonic at least when ARMv4 and ARMv5 code is generated?

Thanks,

Problem is that in case when old binutils are used (in my case freebsd is using old one due to license upgrade to GPLv3) AS doesn't understand new mnemonics and fails.

Indeed, this is new in ARM ARM v7.

Can we change to old mnemonic at least when ARMv4 and ARMv5 code is generated?

We definitely should. Feel free to send a patch if you already have,
or create a bug in bugzilla. Shouldn't be too hard to add it.

cheers,
--renato

I can try to create patch, however I'm still new to llvm.
Can you give me some brief hints where to start?

Thanks,

Damjan

LLVM always uses unified syntax, including updated mnemonics for older instructions. Older binutils aren't supported. This is unlikely to change.

-Jim

This sounds like a dead end as newer binutils are GPLv3.

What is the status of integrated-as for arm, is it stable?

Problem is that in case when old binutils are used (in my case freebsd is using old one due to license upgrade to GPLv3) AS doesn't understand new mnemonics and fails.

Indeed, this is new in ARM ARM v7.

Can we change to old mnemonic at least when ARMv4 and ARMv5 code is generated?

We definitely should. Feel free to send a patch if you already have,
or create a bug in bugzilla. Shouldn't be too hard to add it.

LLVM always uses unified syntax, including updated mnemonics for older instructions. Older binutils aren't supported. This is unlikely to change.

This sounds like a dead end as newer binutils are GPLv3.

Yeah, that's definitely a very real concern and a big motivation to get the MC based asm parser whipped into usable shape. We're much more in control of our own destiny then.

What is the status of integrated-as for arm, is it stable?

The encoding information in the back end is generally in pretty decent shape for direct object file emission. Some of the folks working on the ELF emitter would be better able to tell you how that part of the things is working. The asm parser, on the other hand, is still in pretty rough shape.

That said, I'm hoping to get back to that stuff near future, so it's still under active development.

-Jim

So, how do we solve the problem until then?

cheers,
--renato

In general, it's a tricky thing. If it were only a few instructions, we could just switch to the older mnemonic as a special case w/ a FIXME saying that we should change it when newer assemblers become available on FreeBSD. That's unfortunately not really the case, though. For example, all of the VFP instructions use newer mnemonics, and the predicated condition code setting arithmetic instructions have changed the order of the 's' and 'cc' suffices (e.g., addseq vs. addeqs).

I suspect we'll care about all of them, as we're wanting to handle userspace apps, not just kernel code, here, right? This is the system assembler that's too old.

If so, the best answer is to teach the assembler the new mnemonics. To help with that, the majority of the work for v7 was released under GPLv2, even though it didn't make it into an official binutils release until post-v3 changeover. There are GPLv2 CodeSourcery drops which contain v7 support, for example, including binutils support for unified syntax. While there are some bugs (Anton can probably give more details), they do generally work and would likely provide a good starting point so that the support wouldn't have to be done from scratch.

-Jim

I will try to find those pre-v3 patches.

In meantime I wrote a patch which changes to old mnemonics for shift instructions.
This fixes compiling on the freebsd.

--- a/contrib/llvm/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp
+++ b/contrib/llvm/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp
@@ -44,17 +44,18 @@ void ARMInstPrinter::printInst(const MCInst *MI, raw_ostream &O) {
     const MCOperand &MO2 = MI->getOperand(2);
     const MCOperand &MO3 = MI->getOperand(3);

- O << '\t' << ARM_AM::getShiftOpcStr(ARM_AM::getSORegShOp(MO3.getImm()));
+ O << '\t' << "mov";
     printSBitModifierOperand(MI, 6, O);
     printPredicateOperand(MI, 4, O);

     O << '\t' << getRegisterName(Dst.getReg())
- << ", " << getRegisterName(MO1.getReg());
+ << ", " << getRegisterName(MO1.getReg())
+ << ", " << ARM_AM::getShiftOpcStr(ARM_AM::getSORegShOp(MO3.getImm()));;

     if (ARM_AM::getSORegShOp(MO3.getImm()) == ARM_AM::rrx)
       return;

- O << ", ";
+ O << " ";

     if (MO2.getReg()) {
       O << getRegisterName(MO2.getReg());

This sounds like a dead end as newer binutils are GPLv3.

Yeah, that's definitely a very real concern and a big motivation to get the MC based asm parser whipped into usable shape. We're much more in control of our own destiny then.

So, how do we solve the problem until then?

In general, it's a tricky thing. If it were only a few instructions, we could just switch to the older mnemonic as a special case w/ a FIXME saying that we should change it when newer assemblers become available on FreeBSD. That's unfortunately not really the case, though. For example, all of the VFP instructions use newer mnemonics, and the predicated condition code setting arithmetic instructions have changed the order of the 's' and 'cc' suffices (e.g., addseq vs. addeqs).

I suspect we'll care about all of them, as we're wanting to handle userspace apps, not just kernel code, here, right? This is the system assembler that's too old.

If so, the best answer is to teach the assembler the new mnemonics. To help with that, the majority of the work for v7 was released under GPLv2, even though it didn't make it into an official binutils release until post-v3 changeover. There are GPLv2 CodeSourcery drops which contain v7 support, for example, including binutils support for unified syntax. While there are some bugs (Anton can probably give more details), they do generally work and would likely provide a good starting point so that the support wouldn't have to be done from scratch.

I will try to find those pre-v3 patches.

In meantime I wrote a patch which changes to old mnemonics for shift instructions.
This fixes compiling on the freebsd.

If this is really the only issue you're seeing, we may be lucky and your binutils already have support for lots of the changes necessary to handle llvm assembly. Let's cross our fingers that's the case.

--- a/contrib/llvm/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp
+++ b/contrib/llvm/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp
@@ -44,17 +44,18 @@ void ARMInstPrinter::printInst(const MCInst *MI, raw_ostream &O) {
    const MCOperand &MO2 = MI->getOperand(2);
    const MCOperand &MO3 = MI->getOperand(3);

- O << '\t' << ARM_AM::getShiftOpcStr(ARM_AM::getSORegShOp(MO3.getImm()));
+ O << '\t' << "mov";
    printSBitModifierOperand(MI, 6, O);
    printPredicateOperand(MI, 4, O);

    O << '\t' << getRegisterName(Dst.getReg())
- << ", " << getRegisterName(MO1.getReg());
+ << ", " << getRegisterName(MO1.getReg())
+ << ", " << ARM_AM::getShiftOpcStr(ARM_AM::getSORegShOp(MO3.getImm()));;

    if (ARM_AM::getSORegShOp(MO3.getImm()) == ARM_AM::rrx)
      return;

- O << ", ";
+ O << " ";

    if (MO2.getReg()) {
      O << getRegisterName(MO2.getReg());

Does your assembler support the other shift mnemonics (e.g., "LSL")? This will change the output for all of them.

-Jim

I will try to find those pre-v3 patches.

In meantime I wrote a patch which changes to old mnemonics for shift instructions.
This fixes compiling on the freebsd.

If this is really the only issue you're seeing, we may be lucky and your binutils already have support for lots of the changes necessary to handle llvm assembly. Let's cross our fingers that's the case.

At this point I will be happy to have ARMv5 assembly working, and seems that this issue is only show-stopper so far.

I also had problem with "internal_relocation (type: OFFSET_IMM) not fixed up" errors while compiling libcsu, but I fixed that with passing -O1 to clang.

Does your assembler support the other shift mnemonics (e.g., "LSL")? This will change the output for all of them.

Yes, it works for other shift mnemonics. Verified...

I will try to find those pre-v3 patches.

In meantime I wrote a patch which changes to old mnemonics for shift instructions.
This fixes compiling on the freebsd.

If this is really the only issue you're seeing, we may be lucky and your binutils already have support for lots of the changes necessary to handle llvm assembly. Let's cross our fingers that's the case.

At this point I will be happy to have ARMv5 assembly working, and seems that this issue is only show-stopper so far.

Cool. Glad to hear that.

I also had problem with "internal_relocation (type: OFFSET_IMM) not fixed up" errors while compiling libcsu, but I fixed that with passing -O1 to clang.

Does your assembler support the other shift mnemonics (e.g., "LSL")? This will change the output for all of them.

Yes, it works for other shift mnemonics. Verified...

OK. Would it be possible to just add this mnemonic to your assembler project? It's not unlikely just an inadvertent omission that it didn't have it already since it knows about the other shift aliases.

If we really need to change to compiler output instead, we should make sure to only change the rrx mnemonic, not all of the aliased mov instructions.

-Jim

Hi Damjan,

This sounds like a dead end as newer binutils are GPLv3.

Unfortunately, you have to live with this. Until recently binutils
were quite buggy wrt thumb2 code, so, most probably you will need new
binutils in any case.

Hi Anton,

It's not so simple. GPL3 can be quite a nuisance to change and even to
use. This is why LLVM is becoming to popular for ARM... :wink:

Jim, how much work would you reckon it'll take to have a working assembler?

cheers,
--renato

You mean to patch the old binutils to support this old instruction? Very easy, I'd expect (well, as easy as anything ever is in binutils).

To get -integrated-as working so that we don't need to go through the assembler at all should be mostly a matter of bug fixing, modulo inline assembly support. For non-trivial inline assembly, and to get a system assembler replacement based on the MC assembler, it'll be a bigger task. The ARM asm parser is currently in need of some tender loving care. That's coming soon, but it's a non-trivial task.

For -integrated-as, I would suggest giving it a try. I'm not completely sure how complete the support is for ELF, but I think it's in somewhat OK shape, at least. Worth trying. That would neatly avoid the problem if it works. If it doesn't, we should fix the bugs so it does. :wink:

-Jim

I tried with FreeBSD kernel. Outcome is 2 different crashes. Output is here:

http://web.me.com/dmarion/FreeBSD/integrated-as_crash.txt

I can provide more details if needed.

Damjan