Fix "Q" and "R" inline assembly template modifiers for big-endian Arm

[Jackson]

Summary

Consider the endianness of the target when printing register names. This is in line with the documentation at http://llvm.org/docs/LangRef.html#asm-template-argument-modifiers

[thopre]

Hi Jackson, can you reupload the patch with more context line? Something like git diff -U99999 (arc diff does it automatically). Thanks.

[thopre]

lib/Target/ARM/ARMAsmPrinter.cpp

365–370

Alternatively: FirstHalf = (ExtraCode[0] == 'Q') == ATM.isLittleEndian(), your call on whether this is clearer or not to read.

366

Indentation looks off here.

[Jackson]

Thanks for the review Thomas. I have update the diff so it has context, and have fixed the formatting issue. I think the if statement is probably better as is, although I'm also not entirely sure.

[echristo]

Couple of comments.

lib/Target/ARM/ARMAsmPrinter.cpp

370

Nit: single statements aren't braced in llvm.

370

Block comment explaining why and how this affects things.

[efriedma]

(I think this needs to be rebased on top of D49984.)

[llvm-commits]

(Yep)

• msg-31211-130.txt162 BDownload

[Jackson]

Thanks for the reviews. I have rebased this patch, added a comment to explain what is going on, and removed the redundant curly braces.

[Jackson]

ping

[efriedma]

test/CodeGen/ARM/print-registers-be.ll

1

Could you add a RUN line for little-endian as well?

[Jackson]

Added a testcase for little-endian.

[efriedma]

LGTM

[Jackson]

As I do not have commit rights, could this be committed on my behalf?

[fhahn]

In D49778#1218814, @Jackson wrote:

As I do not have commit rights, could this be committed on my behalf?

I can do that for you.

lib/Target/ARM/ARMAsmPrinter.cpp

case 'Q': { // The least significant register of a pair.
         for (OpNum = InlineAsm::MIOp_FirstOperand; TiedIdx; --TiedIdx) {
           unsigned OpFlags = MI->getOperand(OpNum).getImm();
           OpNum += InlineAsm::getNumOperandRegisters(OpFlags) + 1;
         }
         Flags = MI->getOperand(OpNum).getImm();
 
         // Later code expects OpNum to be pointing at the register rather than
         // the flags.
         OpNum += 1;
       }
 
       unsigned NumVals = InlineAsm::getNumOperandRegisters(Flags);
       unsigned RC;
+      bool FirstHalf;
+      const ARMBaseTargetMachine &ATM =
+        static_cast<const ARMBaseTargetMachine &>(TM);
+
+      // 'Q' should correspond to the low order register and 'R' to the high
+      // order register.  Whether this corresponds to the upper or lower half
+      // depends on the endianess mode.
+      if (ExtraCode[0] == 'Q')
+        FirstHalf = ATM.isLittleEndian();
+      else
+        // ExtraCode[0] == 'R'.
+        FirstHalf = !ATM.isLittleEndian();
       const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
       if (InlineAsm::hasRegClassConstraint(Flags, RC) &&
           ARM::GPRPairRegClass.hasSubClassEq(TRI->getRegClass(RC))) {
         if (NumVals != 1)
           return true;
         const MachineOperand &MO = MI->getOperand(OpNum);
         if (!MO.isReg())
           return true;
         const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
-        unsigned Reg = TRI->getSubReg(MO.getReg(), ExtraCode[0] == 'Q' ?
+        unsigned Reg = TRI->getSubReg(MO.getReg(), FirstHalf ?
             ARM::gsub_0 : ARM::gsub_1);
         O << ARMInstPrinter::getRegisterName(Reg);
         return false;
       }
       if (NumVals != 2)
         return true;
-      unsigned RegOp = ExtraCode[0] == 'Q' ? OpNum : OpNum + 1;
+      unsigned RegOp = FirstHalf ? OpNum : OpNum + 1;
       if (RegOp >= MI->getNumOperands())
         return true;
       const MachineOperand &MO = MI->getOperand(RegOp);
       if (!MO.isReg())
         return true;
       unsigned Reg = MO.getReg();
       O << ARMInstPrinter::getRegisterName(Reg);
       return false;

test/CodeGen/ARM/print-registers-be.ll

+; RUN: llc -mtriple=armeb-arm-none-eabi < %s -o -| FileCheck %s
+
+define dso_local void @_Z3fooi(i32 %a) local_unnamed_addr #0 {
+entry:
+; CHECK: @ plain: [[LOW_REG:r[0-9]+]] Q: [[HIGH_REG:r[0-9]+]] R: [[LOW_REG]] H: [[HIGH_REG]]
+  tail call void asm sideeffect "// plain: $0 Q: ${0:Q} R: ${0:R} H: ${0:H}", "r"(i64 1) #1
+  ret void
+}