[clang] cb56fa2 - [MSP430] Update register names

Anton Korobeynikov via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 26 05:32:59 PDT 2020


Author: Anatoly Trosinenko
Date: 2020-06-26T15:32:07+03:00
New Revision: cb56fa2196c5d80ce72e8e8f6ee2a7ac80acca77

URL: https://github.com/llvm/llvm-project/commit/cb56fa2196c5d80ce72e8e8f6ee2a7ac80acca77
DIFF: https://github.com/llvm/llvm-project/commit/cb56fa2196c5d80ce72e8e8f6ee2a7ac80acca77.diff

LOG: [MSP430] Update register names

When writing a unit test on replacing standard epilogue sequences with `BR __mspabi_func_epilog_<N>`, by manually asm-clobbering `rN` - `r10` for N = 4..10, everything worked well except for seeming inability to clobber r4.

The problem was that MSP430 code generator of LLVM used an obsolete name FP for that register. Things were worse because when `llc` read an unknown register name, it silently ignored it.

That is, I cannot use `fp` register name from the C code because Clang does not accept it (exactly like GCC). But the accepted name `r4` is not recognised by `llc` (it can be used in listings passed to `llvm-mc` and even `fp` is replace to `r4` by `llvm-mc`). So I can specify any of `fp` or `r4` for the string literal of `asm(...)` but nothing in the clobber list.

This patch replaces `MSP430::FP` with `MSP430::R4` in the backend code (even [MSP430 EABI](http://www.ti.com/lit/an/slaa534/slaa534.pdf) doesn't mention FP as a register name). The R0 - R3 registers, on the other hand, are left as is in the backend code (after all, they have some special meaning on the ISA level). It is just ensured clang is renaming them as expected by the downstream tools. There is probably not much sense in **marking them clobbered** but rename them //just in case// for use at potentially different contexts.

Differential Revision: https://reviews.llvm.org/D82184

Added: 
    clang/test/CodeGen/msp430-register-names.c
    llvm/test/CodeGen/MSP430/inline-asm-register-names.ll

Modified: 
    clang/lib/Basic/Targets/MSP430.h
    llvm/lib/Target/MSP430/AsmParser/MSP430AsmParser.cpp
    llvm/lib/Target/MSP430/Disassembler/MSP430Disassembler.cpp
    llvm/lib/Target/MSP430/MSP430FrameLowering.cpp
    llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
    llvm/lib/Target/MSP430/MSP430RegisterInfo.cpp
    llvm/lib/Target/MSP430/MSP430RegisterInfo.td
    llvm/test/CodeGen/MSP430/asm-clobbers.ll

Removed: 
    


################################################################################
diff  --git a/clang/lib/Basic/Targets/MSP430.h b/clang/lib/Basic/Targets/MSP430.h
index 620f12d2b8e3..9d42e4d4bb18 100644
--- a/clang/lib/Basic/Targets/MSP430.h
+++ b/clang/lib/Basic/Targets/MSP430.h
@@ -64,8 +64,14 @@ class LLVM_LIBRARY_VISIBILITY MSP430TargetInfo : public TargetInfo {
   ArrayRef<const char *> getGCCRegNames() const override;
 
   ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override {
-    // No aliases.
-    return None;
+    // Make r0 - r3 be recognized by llc (f.e., in clobber list)
+    static const TargetInfo::GCCRegAlias GCCRegAliases[] = {
+        {{"r0"}, "pc"},
+        {{"r1"}, "sp"},
+        {{"r2"}, "sr"},
+        {{"r3"}, "cg"},
+    };
+    return llvm::makeArrayRef(GCCRegAliases);
   }
 
   bool validateAsmConstraint(const char *&Name,

diff  --git a/clang/test/CodeGen/msp430-register-names.c b/clang/test/CodeGen/msp430-register-names.c
new file mode 100644
index 000000000000..106b6c933a49
--- /dev/null
+++ b/clang/test/CodeGen/msp430-register-names.c
@@ -0,0 +1,105 @@
+// Registers R0 - R3 have 
diff erent names inside the LLVM MSP430 target code.
+// Test that they are handled properly when used inside clobber lists.
+// At the time of writing, llc silently ignores unknown register names.
+
+// REQUIRES: msp430-registered-target
+// RUN: %clang -target msp430 -c %s -mllvm -stop-after=finalize-isel -o- | FileCheck %s
+
+void test_function(void) {
+  asm volatile(""
+               :
+               :
+               : "r0");
+  asm volatile(""
+               :
+               :
+               : "r1");
+  asm volatile(""
+               :
+               :
+               : "r2");
+  asm volatile(""
+               :
+               :
+               : "r3");
+  asm volatile(""
+               :
+               :
+               : "r4");
+  asm volatile(""
+               :
+               :
+               : "r5");
+  asm volatile(""
+               :
+               :
+               : "r6");
+  asm volatile(""
+               :
+               :
+               : "r7");
+  asm volatile(""
+               :
+               :
+               : "r8");
+  asm volatile(""
+               :
+               :
+               : "r9");
+  asm volatile(""
+               :
+               :
+               : "r10");
+  asm volatile(""
+               :
+               :
+               : "r11");
+  asm volatile(""
+               :
+               :
+               : "r12");
+  asm volatile(""
+               :
+               :
+               : "r13");
+  asm volatile(""
+               :
+               :
+               : "r14");
+  asm volatile(""
+               :
+               :
+               : "r15");
+  // CHECK: call void asm sideeffect "", "~{pc}"()
+  // CHECK: call void asm sideeffect "", "~{sp}"()
+  // CHECK: call void asm sideeffect "", "~{sr}"()
+  // CHECK: call void asm sideeffect "", "~{cg}"()
+  // CHECK: call void asm sideeffect "", "~{r4}"()
+  // CHECK: call void asm sideeffect "", "~{r5}"()
+  // CHECK: call void asm sideeffect "", "~{r6}"()
+  // CHECK: call void asm sideeffect "", "~{r7}"()
+  // CHECK: call void asm sideeffect "", "~{r8}"()
+  // CHECK: call void asm sideeffect "", "~{r9}"()
+  // CHECK: call void asm sideeffect "", "~{r10}"()
+  // CHECK: call void asm sideeffect "", "~{r11}"()
+  // CHECK: call void asm sideeffect "", "~{r12}"()
+  // CHECK: call void asm sideeffect "", "~{r13}"()
+  // CHECK: call void asm sideeffect "", "~{r14}"()
+  // CHECK: call void asm sideeffect "", "~{r15}"()
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $pc
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $sp
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $sr
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $cg
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r4
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r5
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r6
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r7
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r8
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r9
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r10
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r11
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r12
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r13
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r14
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $r15
+}

diff  --git a/llvm/lib/Target/MSP430/AsmParser/MSP430AsmParser.cpp b/llvm/lib/Target/MSP430/AsmParser/MSP430AsmParser.cpp
index 5079f16ea734..cbff8d1773ff 100644
--- a/llvm/lib/Target/MSP430/AsmParser/MSP430AsmParser.cpp
+++ b/llvm/lib/Target/MSP430/AsmParser/MSP430AsmParser.cpp
@@ -562,7 +562,7 @@ static unsigned convertGR16ToGR8(unsigned Reg) {
   case MSP430::SP:  return MSP430::SPB;
   case MSP430::SR:  return MSP430::SRB;
   case MSP430::CG:  return MSP430::CGB;
-  case MSP430::FP:  return MSP430::FPB;
+  case MSP430::R4:  return MSP430::R4B;
   case MSP430::R5:  return MSP430::R5B;
   case MSP430::R6:  return MSP430::R6B;
   case MSP430::R7:  return MSP430::R7B;

diff  --git a/llvm/lib/Target/MSP430/Disassembler/MSP430Disassembler.cpp b/llvm/lib/Target/MSP430/Disassembler/MSP430Disassembler.cpp
index 6aa76156bf14..d2902189ec40 100644
--- a/llvm/lib/Target/MSP430/Disassembler/MSP430Disassembler.cpp
+++ b/llvm/lib/Target/MSP430/Disassembler/MSP430Disassembler.cpp
@@ -65,7 +65,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeMSP430Disassembler() {
 
 static const unsigned GR8DecoderTable[] = {
   MSP430::PCB,  MSP430::SPB,  MSP430::SRB,  MSP430::CGB,
-  MSP430::FPB,  MSP430::R5B,  MSP430::R6B,  MSP430::R7B,
+  MSP430::R4B,  MSP430::R5B,  MSP430::R6B,  MSP430::R7B,
   MSP430::R8B,  MSP430::R9B,  MSP430::R10B, MSP430::R11B,
   MSP430::R12B, MSP430::R13B, MSP430::R14B, MSP430::R15B
 };
@@ -83,7 +83,7 @@ static DecodeStatus DecodeGR8RegisterClass(MCInst &MI, uint64_t RegNo,
 
 static const unsigned GR16DecoderTable[] = {
   MSP430::PC,  MSP430::SP,  MSP430::SR,  MSP430::CG,
-  MSP430::FP,  MSP430::R5,  MSP430::R6,  MSP430::R7,
+  MSP430::R4,  MSP430::R5,  MSP430::R6,  MSP430::R7,
   MSP430::R8,  MSP430::R9,  MSP430::R10, MSP430::R11,
   MSP430::R12, MSP430::R13, MSP430::R14, MSP430::R15
 };

diff  --git a/llvm/lib/Target/MSP430/MSP430FrameLowering.cpp b/llvm/lib/Target/MSP430/MSP430FrameLowering.cpp
index 567180476ae0..4be8d0760e68 100644
--- a/llvm/lib/Target/MSP430/MSP430FrameLowering.cpp
+++ b/llvm/lib/Target/MSP430/MSP430FrameLowering.cpp
@@ -64,16 +64,16 @@ void MSP430FrameLowering::emitPrologue(MachineFunction &MF,
 
     // Save FP into the appropriate stack slot...
     BuildMI(MBB, MBBI, DL, TII.get(MSP430::PUSH16r))
-      .addReg(MSP430::FP, RegState::Kill);
+      .addReg(MSP430::R4, RegState::Kill);
 
     // Update FP with the new base value...
-    BuildMI(MBB, MBBI, DL, TII.get(MSP430::MOV16rr), MSP430::FP)
+    BuildMI(MBB, MBBI, DL, TII.get(MSP430::MOV16rr), MSP430::R4)
       .addReg(MSP430::SP);
 
     // Mark the FramePtr as live-in in every block except the entry.
     for (MachineFunction::iterator I = std::next(MF.begin()), E = MF.end();
          I != E; ++I)
-      I->addLiveIn(MSP430::FP);
+      I->addLiveIn(MSP430::R4);
 
   } else
     NumBytes = StackSize - MSP430FI->getCalleeSavedFrameSize();
@@ -132,7 +132,7 @@ void MSP430FrameLowering::emitEpilogue(MachineFunction &MF,
     NumBytes = FrameSize - CSSize;
 
     // pop FP.
-    BuildMI(MBB, MBBI, DL, TII.get(MSP430::POP16r), MSP430::FP);
+    BuildMI(MBB, MBBI, DL, TII.get(MSP430::POP16r), MSP430::R4);
   } else
     NumBytes = StackSize - CSSize;
 
@@ -154,7 +154,7 @@ void MSP430FrameLowering::emitEpilogue(MachineFunction &MF,
 
   if (MFI.hasVarSizedObjects()) {
     BuildMI(MBB, MBBI, DL,
-            TII.get(MSP430::MOV16rr), MSP430::SP).addReg(MSP430::FP);
+            TII.get(MSP430::MOV16rr), MSP430::SP).addReg(MSP430::R4);
     if (CSSize) {
       MachineInstr *MI =
         BuildMI(MBB, MBBI, DL,

diff  --git a/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp b/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
index 34f3d8ea61f6..821339f50355 100644
--- a/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
+++ b/llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
@@ -1300,7 +1300,7 @@ SDValue MSP430TargetLowering::LowerFRAMEADDR(SDValue Op,
   SDLoc dl(Op);  // FIXME probably not meaningful
   unsigned Depth = cast<ConstantSDNode>(Op.getOperand(0))->getZExtValue();
   SDValue FrameAddr = DAG.getCopyFromReg(DAG.getEntryNode(), dl,
-                                         MSP430::FP, VT);
+                                         MSP430::R4, VT);
   while (Depth--)
     FrameAddr = DAG.getLoad(VT, dl, DAG.getEntryNode(), FrameAddr,
                             MachinePointerInfo());

diff  --git a/llvm/lib/Target/MSP430/MSP430RegisterInfo.cpp b/llvm/lib/Target/MSP430/MSP430RegisterInfo.cpp
index bec357a1548d..5583ebee2f31 100644
--- a/llvm/lib/Target/MSP430/MSP430RegisterInfo.cpp
+++ b/llvm/lib/Target/MSP430/MSP430RegisterInfo.cpp
@@ -39,7 +39,7 @@ MSP430RegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
   const MSP430FrameLowering *TFI = getFrameLowering(*MF);
   const Function* F = &MF->getFunction();
   static const MCPhysReg CalleeSavedRegs[] = {
-    MSP430::FP, MSP430::R5, MSP430::R6, MSP430::R7,
+    MSP430::R4, MSP430::R5, MSP430::R6, MSP430::R7,
     MSP430::R8, MSP430::R9, MSP430::R10,
     0
   };
@@ -49,7 +49,7 @@ MSP430RegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
     0
   };
   static const MCPhysReg CalleeSavedRegsIntr[] = {
-    MSP430::FP,  MSP430::R5,  MSP430::R6,  MSP430::R7,
+    MSP430::R4,  MSP430::R5,  MSP430::R6,  MSP430::R7,
     MSP430::R8,  MSP430::R9,  MSP430::R10, MSP430::R11,
     MSP430::R12, MSP430::R13, MSP430::R14, MSP430::R15,
     0
@@ -86,8 +86,8 @@ BitVector MSP430RegisterInfo::getReservedRegs(const MachineFunction &MF) const {
 
   // Mark frame pointer as reserved if needed.
   if (TFI->hasFP(MF)) {
-    Reserved.set(MSP430::FPB);
-    Reserved.set(MSP430::FP);
+    Reserved.set(MSP430::R4B);
+    Reserved.set(MSP430::R4);
   }
 
   return Reserved;
@@ -112,7 +112,7 @@ MSP430RegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
   DebugLoc dl = MI.getDebugLoc();
   int FrameIndex = MI.getOperand(FIOperandNum).getIndex();
 
-  unsigned BasePtr = (TFI->hasFP(MF) ? MSP430::FP : MSP430::SP);
+  unsigned BasePtr = (TFI->hasFP(MF) ? MSP430::R4 : MSP430::SP);
   int Offset = MF.getFrameInfo().getObjectOffset(FrameIndex);
 
   // Skip the saved PC
@@ -156,5 +156,5 @@ MSP430RegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
 
 Register MSP430RegisterInfo::getFrameRegister(const MachineFunction &MF) const {
   const MSP430FrameLowering *TFI = getFrameLowering(MF);
-  return TFI->hasFP(MF) ? MSP430::FP : MSP430::SP;
+  return TFI->hasFP(MF) ? MSP430::R4 : MSP430::SP;
 }

diff  --git a/llvm/lib/Target/MSP430/MSP430RegisterInfo.td b/llvm/lib/Target/MSP430/MSP430RegisterInfo.td
index 0ba8e1c7ebb6..61cc72d494b5 100644
--- a/llvm/lib/Target/MSP430/MSP430RegisterInfo.td
+++ b/llvm/lib/Target/MSP430/MSP430RegisterInfo.td
@@ -36,7 +36,7 @@ def PCB  : MSP430Reg<0,  "r0", ["pc"]>;
 def SPB  : MSP430Reg<1,  "r1", ["sp"]>;
 def SRB  : MSP430Reg<2,  "r2", ["sr"]>;
 def CGB  : MSP430Reg<3,  "r3", ["cg"]>;
-def FPB  : MSP430Reg<4,  "r4", ["fp"]>;
+def R4B  : MSP430Reg<4,  "r4", ["fp"]>;
 def R5B  : MSP430Reg<5,  "r5">;
 def R6B  : MSP430Reg<6,  "r6">;
 def R7B  : MSP430Reg<7,  "r7">;
@@ -56,7 +56,7 @@ def PC  : MSP430RegWithSubregs<0,  "r0",  [PCB], ["pc"]>;
 def SP  : MSP430RegWithSubregs<1,  "r1",  [SPB], ["sp"]>;
 def SR  : MSP430RegWithSubregs<2,  "r2",  [SRB], ["sr"]>;
 def CG  : MSP430RegWithSubregs<3,  "r3",  [CGB], ["cg"]>;
-def FP  : MSP430RegWithSubregs<4,  "r4",  [FPB], ["fp"]>;
+def R4  : MSP430RegWithSubregs<4,  "r4",  [R4B], ["fp"]>;
 def R5  : MSP430RegWithSubregs<5,  "r5",  [R5B]>;
 def R6  : MSP430RegWithSubregs<6,  "r6",  [R6B]>;
 def R7  : MSP430RegWithSubregs<7,  "r7",  [R7B]>;
@@ -74,7 +74,7 @@ def GR8 : RegisterClass<"MSP430", [i8], 8,
    // Volatile registers
   (add R12B, R13B, R14B, R15B, R11B, R10B, R9B, R8B, R7B, R6B, R5B,
    // Frame pointer, sometimes allocable
-   FPB,
+   R4B,
    // Volatile, but not allocable
    PCB, SPB, SRB, CGB)>;
 
@@ -82,6 +82,6 @@ def GR16 : RegisterClass<"MSP430", [i16], 16,
    // Volatile registers
   (add R12, R13, R14, R15, R11, R10, R9, R8, R7, R6, R5,
    // Frame pointer, sometimes allocable
-   FP,
+   R4,
    // Volatile, but not allocable
    PC, SP, SR, CG)>;

diff  --git a/llvm/test/CodeGen/MSP430/asm-clobbers.ll b/llvm/test/CodeGen/MSP430/asm-clobbers.ll
index 0a0335057f1b..bd6f88cc2436 100644
--- a/llvm/test/CodeGen/MSP430/asm-clobbers.ll
+++ b/llvm/test/CodeGen/MSP430/asm-clobbers.ll
@@ -3,9 +3,52 @@
 target datalayout = "e-m:e-p:16:16-i32:16:32-a:16-n8:16"
 target triple = "msp430---elf"
 
-define void @test() {
+define void @test_no_clobber() {
 entry:
-; CHECK-LABEL: test:
+; CHECK-LABEL: test_no_clobber
+; CHECK-NOT: push
+  call void asm sideeffect "", ""()
+; CHECK-NOT: pop
+  ret void
+; CHECK: -- End function
+}
+
+define void @test_1() {
+entry:
+; CHECK-LABEL: test_1:
+; CHECK: push r8
+; CHECK: push r6
+; CHECK: push r4
+  call void asm sideeffect "", "~{r4},~{r6},~{r8}"()
+; CHECK: pop r4
+; CHECK: pop r6
+; CHECK: pop r8
+  ret void
+}
+
+define void @test_2() {
+entry:
+; CHECK-LABEL: test_2:
+; CHECK: push r9
+; CHECK: push r7
+; CHECK: push r5
+  call void asm sideeffect "", "~{r5},~{r7},~{r9}"()
+; CHECK: pop r5
+; CHECK: pop r7
+; CHECK: pop r9
+  ret void
+}
+
+; The r10 register is special because the sequence
+;   pop r10
+;   ret
+; can be replaced with
+;   jmp __mspabi_func_epilog_1
+; or other such function (depending on previous instructions).
+; Still, it is not replaced *yet*.
+define void @test_r10() {
+entry:
+; CHECK-LABEL: test_r10:
 ; CHECK: push r10
   call void asm sideeffect "", "~{r10}"()
 ; CHECK: pop r10

diff  --git a/llvm/test/CodeGen/MSP430/inline-asm-register-names.ll b/llvm/test/CodeGen/MSP430/inline-asm-register-names.ll
new file mode 100644
index 000000000000..5b8accdcc74e
--- /dev/null
+++ b/llvm/test/CodeGen/MSP430/inline-asm-register-names.ll
@@ -0,0 +1,59 @@
+; RUN: llc < %s | FileCheck %s
+target datalayout = "e-p:16:8:8-i8:8:8-i16:8:8-i32:8:8"
+target triple = "msp430-generic-generic"
+
+; Test that correct register names are accepted *inside* inline asm listings.
+; Tested with PUSH instruction since it does not support memory operands.
+
+define void @accepted_rN() nounwind {
+; CHECK-LABEL: accepted_rN
+  call void asm sideeffect "push r0", ""() nounwind
+; CHECK: push r0
+  call void asm sideeffect "push r1", ""() nounwind
+; CHECK: push r1
+  call void asm sideeffect "push r2", ""() nounwind
+; CHECK: push r2
+  call void asm sideeffect "push r3", ""() nounwind
+; CHECK: push r3
+  call void asm sideeffect "push r4", ""() nounwind
+; CHECK: push r4
+  call void asm sideeffect "push r5", ""() nounwind
+; CHECK: push r5
+  call void asm sideeffect "push r6", ""() nounwind
+; CHECK: push r6
+  call void asm sideeffect "push r7", ""() nounwind
+; CHECK: push r7
+  call void asm sideeffect "push r8", ""() nounwind
+; CHECK: push r8
+  call void asm sideeffect "push r9", ""() nounwind
+; CHECK: push r9
+  call void asm sideeffect "push r10", ""() nounwind
+; CHECK: push r10
+  call void asm sideeffect "push r11", ""() nounwind
+; CHECK: push r11
+  call void asm sideeffect "push r12", ""() nounwind
+; CHECK: push r12
+  call void asm sideeffect "push r13", ""() nounwind
+; CHECK: push r13
+  call void asm sideeffect "push r14", ""() nounwind
+; CHECK: push r14
+  call void asm sideeffect "push r15", ""() nounwind
+; CHECK: push r15
+  ret void
+}
+
+define void @accepted_reg_aliases() nounwind {
+; CHECK-LABEL: accepted_reg_aliases
+; Ensure register aliases are renamed as expected
+call void asm sideeffect "push pc", ""() nounwind
+; CHECK: push r0
+call void asm sideeffect "push sp", ""() nounwind
+; CHECK: push r1
+call void asm sideeffect "push sr", ""() nounwind
+; CHECK: push r2
+call void asm sideeffect "push cg", ""() nounwind
+; CHECK: push r3
+call void asm sideeffect "push fp", ""() nounwind
+; CHECK: push r4
+        ret void
+}


        


More information about the cfe-commits mailing list