[PATCH] [mips] Add assembler support for the .cprestore directive.

Daniel Sanders daniel.sanders at imgtec.com
Thu Feb 12 08:18:38 PST 2015


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:361
@@ -354,1 +360,3 @@
+
+    IsCpRestoreSet = false;
   }
----------------
Just to be on the safe side, could you also initialize CpRestoreOffset in the constructor?

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1454
@@ +1453,3 @@
+
+    if (ExpandedJalSym == false)
+      return false;
----------------
Nit: if(!ExpandedJalSym)

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1654-1688
@@ -1638,2 +1653,37 @@
+
+  if ((Inst.getOpcode() == Mips::JalOneReg ||
+       Inst.getOpcode() == Mips::JalTwoReg || ExpandedJalSym) &&
+      inPicMode() && !(isABI_N32() || isABI_N64())) {
+    if (IsCpRestoreSet) {
+      // Emit a NOP after the JALR, even if .set reorder has not been used.
+      if (!AssemblerOptions.back()->isReorder()) {
+        MCInst NopInst;
+        if (hasShortDelaySlot(Inst.getOpcode()) ||
+            ((Inst.getOpcode() == Mips::JalOneReg ||
+              Inst.getOpcode() == Mips::JalTwoReg) &&
+             inMicroMipsMode())) {
+          NopInst.setOpcode(Mips::MOVE16_MM);
+          NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
+          NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
+        } else {
+          NopInst.setOpcode(Mips::SLL);
+          NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
+          NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
+          NopInst.addOperand(MCOperand::CreateImm(0));
+        }
+        Instructions.push_back(NopInst);
+      }
+
+      // Load the $gp from the stack.
+      SmallVector<MCInst, 3> LoadInsts;
+      createCpRestoreMemOp(true /*IsLoad*/, CpRestoreOffset /*StackOffset*/,
+                           IDLoc, LoadInsts);
+
+      for (const MCInst &Inst : LoadInsts)
+        Instructions.push_back(Inst);
 
+    } else {
+      Warning(IDLoc, "no .cprestore used in PIC mode");
+    }
+  }
   return false;
----------------
I'm a bit confused by this code. Emitting a nop after a jal/jalr/jalrs makes sense to me, but I don't understand why the presence of the nop depends on inPicMode() and (!N32 && !N64).

I'm also not sure why the non-macro versions of jalr/jalrs (which set ExpandedJalSym to true) are causing createCpRestoreMemOp() to be called.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1658
@@ +1657,3 @@
+    if (IsCpRestoreSet) {
+      // Emit a NOP after the JALR, even if .set reorder has not been used.
+      if (!AssemblerOptions.back()->isReorder()) {
----------------
This comment isn't quite right. It's emitting a nop if we are in '.set noreorder' mode.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1806-1816
@@ -1750,9 +1805,13 @@
   if (AssemblerOptions.back()->isReorder()) {
-    // This is a 32-bit NOP because these 2 pseudo-instructions
-    // do not have a short delay slot.
     MCInst NopInst;
-    NopInst.setOpcode(Mips::SLL);
-    NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
-    NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
-    NopInst.addOperand(MCOperand::CreateImm(0));
+    if (hasShortDelaySlot(JalrInst.getOpcode())) {
+      NopInst.setOpcode(Mips::MOVE16_MM);
+      NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
+      NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
+    } else {
+      NopInst.setOpcode(Mips::SLL);
+      NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
+      NopInst.addOperand(MCOperand::CreateReg(Mips::ZERO));
+      NopInst.addOperand(MCOperand::CreateImm(0));
+    }
     Instructions.push_back(NopInst);
----------------
We ought to factor out the nop emission at some point

http://reviews.llvm.org/D6267

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list