[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.

> Note: Didn't set the MayLoad flag to 1 in LWUPC DESC class, because with it, LWUPC tests with symbols cause an assertion fail: "Assertion `isReg() && "This is not a register operand!"' failed.". I didn't find a way around that.


Yes, when an instruction with mayLoad/myStore is processed in MipsAsmParser::expandLoadInst assumes that all load instructions have both a source and destination register. lwupc however doesn't have a source register, merely an enlarged offset or symbol.

Tablegen has inferred that lwupc doesn't load, and so doesn't mark it as a load, the symbol 'bar' gets handled with a relocation. This leads to a correct result but it has a mis-defined instruction.

You can fix this easily enough by making some fairly simple changes. Extend the MipsInst class to have a "isPCRelativeLoad" member in MipsInstrFormats.td, make the corresponding change in MipsInstrInfo.h, then only check normal load/store instructions for expansion in processInstruction. Look at isCTI for an example.

I have some more comments inline.

Thanks


================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:319
@@ +318,3 @@
+  list<dag> Pattern = [];
+}
+
----------------
This requires the instruction itinerary, II_DBITSWAP.

================
Comment at: lib/Target/Mips/MicroMips64r6InstrInfo.td:325
@@ +324,3 @@
+  string AsmString = "dlsa\t$rt, $rs, $rd, $sa";
+  list<dag> Pattern = [];
+}
----------------
This requires the instruction itinerary, II_DLSA.


================
Comment at: lib/Target/Mips/MipsInstrInfo.td:416-424
@@ -415,1 +415,11 @@
 
+class MipsSimmAsmOperandClass<int Bits, list<AsmOperandClass> Supers = [],
+                                  int Shift = 0> : AsmOperandClass {
+  let Name = "JumpTarget" # Bits # "_" # Shift;
+  let RenderMethod = "addImmOperands";
+  let ParserMethod = "parseJumpTarget";
+  let PredicateMethod = "isScaledSImm<" # Bits # ", " # Shift # ">";
+  let SuperClasses = Supers;
+  let DiagnosticType = "SImm" # Bits # "_Lsl" # Shift;
+}
+
----------------
Calll this SimmLslAsmOperandClass instead, change the Name to reflect the class and remove the ParserMethod, the generic parser can handle the necessary cases.

================
Comment at: lib/Target/Mips/MipsInstrInfo.td:631-632
@@ -613,1 +630,4 @@
 
+def MipsSimm19Lsl2AsmOperand
+    : MipsSimmAsmOperandClass<19, [], 2>;
+
----------------
Drop the 'Mips' from the names here.


https://reviews.llvm.org/D16452