[PATCH] D11219: [mips][microMIPS] Implement JALRC16, JRCADDIUSP and JRC16 instructions

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 2 03:33:39 PDT 2015


dsanders requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:974
@@ +973,3 @@
+    int64_t val = getConstantImm();
+    return val >= 0 && val <= 32;
+  }
----------------
This doesn't sound right. uimm5 is indeed 0-32 but nothing accounts for the Lsl2 part of the name.

Shouldn't this be testing for multiples of 4 between 0 and 128?

================
Comment at: lib/Target/Mips/MicroMipsInstrFormats.td:40-45
@@ -39,2 +39,8 @@
 
+// Class used for separating microMIPSr6 and microMIPS (r3) instruction.
+// It can be used only on instructions that doesn't inherit PredicateControl.
+class MicroMipsNotMips32r6 : PredicateControl {
+  let InsnPredicates = [InMicroMips, NotMips32r6];
+}
+
 //===----------------------------------------------------------------------===//
----------------
All instructions should be inheriting from PredicateControl. There's no reason not to do so and not doing so leaves us with two incompatible ways of managing predicates. If you look at InstSE/MipsR6Inst/etc. (which inherit from PredicateControl) you'll see that all the instruction formats inherit from one of them. It seems microMIPS(R3) (and MIPS16?) still needs to have PredicateControl added to the formats but we should ensure that microMIPSR6 is consistent with the better practice used by everything else.

Also, please do this as an adjective (like ISA_MIPS64_NOT_64R6) and put it with the others in MipsInstrInfo.td 


http://reviews.llvm.org/D11219





More information about the llvm-commits mailing list