[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