[PATCH] D16452: [mips][microMIPS] Implement DBITSWAP, DLSA and LWUPC and add tests for AUI instructions

[Simon Dardis via llvm-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

Some small nits. There's only one significant change required for the predicate for lwupc. We need to see if we can evaluate it as something relocatable and that the constant part is scaled too.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1110-1116
@@ -1109,4 +1109,9 @@
   bool isScaledSImm() const {
+    // Operand can also be symbol in case of relocations.
+    // In this case we should just ignore checking of integer value.
+    if (isa<MCSymbolRefExpr>(getImm())) {
+      return true;
+    }
     return isConstantImm() &&
            isShiftedInt<Bits, ShiftLeftAmount>(getConstantImm());
   }
----------------
This almost correct. We need to check for constants, symbols and symbols + offsets. The offset then has to be in the scaled form. I've given this an attempt with:

    if (isConstantImm() && isShiftedInt<Bits, ShiftLeftAmount>(getConstantImm()))                                      
      return true; 
    // Operand can also be a symbol or symbol plus offset in case of relocations.
    if (Kind != k_Immediate)
      return false;
    MCValue Res;
    bool Success = getImm()->evaluateAsRelocatable(Res, nullptr, nullptr);
    return Success && isShiftedInt<Bits, ShiftLeftAmount>(Res.getConstant());

Which also checks the offset. The code would also need to check that it's an immediate.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrFormats.td:189
@@ +188,3 @@
+
+class POOL32A_2R_FM_MM64R6 {
+  bits<5> rt;
----------------
This should be called POOL32S_DBITSWAP_FM_MMR6, as it's in pool32s.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrFormats.td:207-215
@@ +206,11 @@
+  bits<5> rd;
+  bits<2> imm;
+
+  bits<32> Inst;
+
+  let Inst{31-26} = 0b010110;
+  let Inst{25-21} = rt;
+  let Inst{20-16} = rs;
+  let Inst{15-11} = rd;
+  let Inst{10-9} = imm;
+  let Inst{8-6} = 0b100;
----------------
'imm' should be 'sa' to match the documentation

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:257-258
@@ +256,4 @@
+  dag OutOperandList = (outs GPR64Opnd:$rd);
+  dag InOperandList = (ins GPR64Opnd:$rt, GPR64Opnd:$rs, uimm2_plus1:$imm);
+  string AsmString = "dlsa\t$rt, $rs, $rd, $imm";
+  list<dag> Pattern = [];
----------------
Nit: imm should be sa.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:262-267
@@ +261,8 @@
+
+class LWUPC_MM64R6_DESC : MMR6Arch<"lwupc">, MipsR6Inst {
+  dag OutOperandList = (outs GPR64Opnd:$rt);
+  dag InOperandList = (ins simm19_lsl2:$offset);
+  string AsmString = "lwupc\t$rt, $offset";
+  list<dag> Pattern = [];
+}
+
----------------
Needs MayLoad = 1.

================
Comment at: lib/Target/Mips/Mips32r6InstrInfo.td:769-772
@@ -768,4 +768,6 @@
 def LWPC : R6MMR6Rel, LWPC_ENC, LWPC_DESC, ISA_MIPS32R6;
-def LWUPC : LWUPC_ENC, LWUPC_DESC, ISA_MIPS32R6;
+let AdditionalPredicates = [NotInMicroMips] in {
+  def LWUPC : LWUPC_ENC, LWUPC_DESC, ISA_MIPS32R6;
+}
 let AdditionalPredicates = [NotInMicroMips] in {
   def MADDF_S : MADDF_S_ENC, MADDF_S_DESC, ISA_MIPS32R6, HARDFLOAT;
----------------
Join these "let AdditionalPredicates =" blocks together.

================
Comment at: lib/Target/Mips/MipsInstrInfo.td:409
@@ -408,1 +408,3 @@
 
+class MipsJumpTargetAsmOperandClass<int Bits, list<AsmOperandClass> Supers = [],
+                                  int Shift = 0> : AsmOperandClass {
----------------
I think this should have a different name, as lwupc doesn't jump.

================
Comment at: test/MC/Mips/micromips64r6/invalid.s:282
@@ +281,3 @@
+  lwupc $2, 5                  # CHECK: :[[@LINE]]:13: error: expected both 19-bit signed immediate and multiple of 4
+  lwupc $2, -262145            # CHECK: :[[@LINE]]:13: error: expected both 19-bit signed immediate and multiple of 4
+  aui $3, $4, 32768            # CHECK: :[[@LINE]]:15: error: expected 16-bit signed immediate
----------------
Check that symbol + offset where the offset is not a multiple of 4 is rejected as well and that register + register is rejected as well.

================
Comment at: test/MC/Mips/micromips64r6/valid.s:283
@@ -280,1 +282,3 @@
+        dlsa $3, $4, $5, 3       # CHECK: dlsa $3, $4, $5, 3      # encoding: [0x58,0x64,0x2d,0x04]
+        lwupc $2, 268            # CHECK: lwupc $2, 268           # encoding: [0x78,0x50,0x00,0x43]
 
----------------
Also check that a symbol and symbol + offset where the offset is a multiple of 4 is also accepted for lwupc.


http://reviews.llvm.org/D16452