[PATCH] D9524: [mips] [IAS] Add support for the DLA pseudo-instruction and fix problems with DLI

Vasileios Kalintiris Vasileios.Kalintiris at imgtec.com
Wed Jul 22 04:42:27 PDT 2015


vkalintiris added a comment.

LGTM with a small test case as described in the comments below.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1845
@@ +1844,3 @@
+
+  return x == x >> BitNum << BitNum && isUInt<N>(x >> BitNum);
+}
----------------
Nit: Parenthesize the expression.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1937
@@ +1936,3 @@
+      if (ImmValue == 0xffffffff) {
+        emitRI(Mips::LUi, TmpReg, 0xffff, IDLoc, Instructions);
+        emitRRI(Mips::DSRL32, TmpReg, TmpReg, 0, IDLoc, Instructions);
----------------
Nit: emitRI expects an int16_t which leads to an "overflow in implicit constant conversion" warning due to the hexadecimal literal.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2051-2055
@@ -2014,23 +2050,7 @@
+
+  // dla requires 64-bit addresses.
+  if (!Is32BitAddress && !ABI.ArePtrs64bit()) {
+    Error(IDLoc, "instruction requires a 64-bit architecture");
     return true;
-
-  return false;
-}
-
-bool
-MipsAsmParser::expandLoadAddressImm(MCInst &Inst, bool Is32BitImm, SMLoc IDLoc,
-                                    SmallVectorImpl<MCInst> &Instructions) {
-  const MCOperand &DstRegOp = Inst.getOperand(0);
-  assert(DstRegOp.isReg() && "expected register operand kind");
-
-  const MCOperand &ImmOp = Inst.getOperand(1);
-  assert((ImmOp.isImm() || ImmOp.isExpr()) &&
-         "expected immediate operand kind");
-  if (!ImmOp.isImm()) {
-    if (loadAndAddSymbolAddress(ImmOp.getExpr(), DstRegOp.getReg(),
-                                Mips::NoRegister, Is32BitImm, IDLoc,
-                                Instructions))
-      return true;
-
-    return false;
   }
 
----------------
Could you add a test for this case? Maybe in macro-la-bad.s?

Also, the following test case currently fails for N32:

```
dla $5, 0x000000000($6)
```

================
Comment at: test/MC/Mips/macro-la.s:10
@@ -9,1 +9,3 @@
 
+# N64 is should be acceptable too but we cannot convert la to dla yet.
+
----------------
Nit: Delete "is".


http://reviews.llvm.org/D9524







More information about the llvm-commits mailing list