[PATCH] [mips] [IAS] Add support for the DLA pseudo-instruction.

Daniel Sanders daniel.sanders at imgtec.com
Wed May 13 05:50:38 PDT 2015


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:821
@@ -820,2 +820,3 @@
 
-    Inst.addOperand(MCOperand::CreateReg(getMemBase()->getGPR32Reg()));
+    Inst.addOperand(MCOperand::CreateReg(AsmParser.isGP64bit()
+                                             ? getMemBase()->getGPR64Reg()
----------------
This should test for N64, preferably using MipsABIInfo::ArePtrs64bit()

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1959-1963
@@ -1950,5 +1958,7 @@
                                       SmallVectorImpl<MCInst> &Instructions) {
   if (Is32BitSym && isGP64bit())
     Warning(IDLoc,
             "instruction loads a 32-bit address on a 64-bit architecture");
 
+  if (!Is32BitSym && !isGP64bit()) {
+    Error(IDLoc, "instruction requires a 64-bit architecture");
----------------
Similarly, these two isGP64bit() checks should be checks for 64-bit pointers via the MipsABIInfo class

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1994-1999
@@ -1980,8 +1993,8 @@
     // If it's a 64-bit architecture, expand to:
     // la d,sym => lui  d,highest(sym)
     //             ori  d,d,higher(sym)
     //             dsll d,d,16
     //             ori  d,d,hi16(sym)
     //             dsll d,d,16
     //             ori  d,d,lo16(sym)
     const MCExpr *HighestExpr = evaluateRelocExpr(SymOp->getExpr(), "highest");
----------------
This comment is only correct for the 'if (UseSrcReg && (SrcReg == DstReg))' path. Could you move/update it to cover the other path?

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2026
@@ +2025,3 @@
+      return false;
+    } else if (!UseSrcReg || (UseSrcReg && (SrcReg != DstReg))) {
+
----------------
Strangely, GAS only seems to expand this way for:
  dla $2, foo($3)
and not:
  dla $2, 0x0001000200030004($3)
which generates the longer code above. I think we should mention this to them.

Also, I don't get this expansion when running this instruction through the GCC driver:
  dla $2, sym($8)
Instead I get:
  8c:	df820000 	ld	v0,0(gp)
			8c: R_MIPS_GOT_DISP	.text
			8c: R_MIPS_NONE	*ABS*
			8c: R_MIPS_NONE	*ABS*
  90:	0048102d 	daddu	v0,v0,a4
I think it needs to depend on -mno-abicalls.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2027
@@ +2026,3 @@
+    } else if (!UseSrcReg || (UseSrcReg && (SrcReg != DstReg))) {
+
+      // This only happens if !UseSrcReg.
----------------
Nit: Blank line

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2054-2063
@@ +2053,12 @@
+
+      // Make place in DstReg for the contents of AT by doing a 32-bit shift:
+      createLShiftOri<32>(0, DstReg, SMLoc(), Instructions);
+
+      // Merge DstReg and AT:
+      tmpInst.clear();
+      tmpInst.setOpcode(Mips::DADDu);
+      tmpInst.addOperand(MCOperand::CreateReg(DstReg));
+      tmpInst.addOperand(MCOperand::CreateReg(DstReg));
+      tmpInst.addOperand(MCOperand::CreateReg(ATReg));
+      Instructions.push_back(tmpInst);
+
----------------
(Phabricator won't let me delete this comment)

http://reviews.llvm.org/D9524

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






More information about the llvm-commits mailing list