[PATCH] D14390: [mips] Expansion of LI.S and LI.D

[Daniel Sanders via llvm-commits]

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

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:559
@@ -555,3 +558,3 @@
   /// Warn if RegIndex is the same as the current AT.
   void warnIfRegIndexIsAT(unsigned RegIndex, SMLoc Loc);
 
----------------
> RegKind_FGR is removed because we didn't find another way to distinguish RegKind_GPR and RegKind_FGR in li.s $4 1.12345 and li.s $f4 1.12345, for example.

This should work:
  bool isStrictlyFGRAsmReg() { return isRegIdx() && RegIdx.Kind == RegKind_FGR && RegIdx.Index <= 31; }
This will only be true when an FGR is the only option (i.e. the source said '$f4'). You'll also need to subclass FGR32/FGR64/AFGR64 in tablegen, override the predicate method to isStrictlyFGRAsmReg, and use those new operands in the li.s/li.d pseudos.

> Actually, for $4 in llvm is created RegKind_Numeric register, that is defined as RegKind_Numeric = RegKind_GPR | RegKind_FGR | RegKind_FCC ..., so condition isFGRAsmReg() in validateOperandClass() function (in MipsGenAsmMatcher.inc) is true for $4 and we get the expansion for RegKind_FGR instead of RegKind_GPR.

$4 is a RegKind_Numeric because $4 is ambiguous without additional context. It could be a `A0`, `F4`, `D4`, `D4_64`, `FCC4`, `COP24` (register 4 in the COP2 set, we should probably add an underscore to the name), etc. depending on the mnemonic and which operand it appears in. The same is true of $f4 and RegKind_FGR to a lesser extent. It's still ambiguous which register '$f4' refers to and could be `F4`, `D4`, or `D4_64` but it definitely isn't `A0`, `COP24`, etc.

The main thing here is that MipsOperand describes what the operand _might_ be rather than what it really is (we figure out what it _is_ at a later point in the assembler). With that in mind, MipsOperand::RegIdxOp::Kind is a bitfield representing all the possible interpretations of the operand by any instruction in our instruction set. There are some instructions where $4 is a floating point register, therefore RegKind_FGR must be part of RegKind_Numeric.

The ambiguity is resolved by the match table in the AsmMatcher. For each matchable, we call a particular predicate on each operand (specified in the tablegen definitions) and the first one to find that all the predicates are true is a match. When you have multiple matchables that can accept the same operands, the one that appears first in the table is chosen. I expect that LoadImmDoubleFGR appears first in your table so $4 is accepted there and LoadImmDoubleGPR never has the chance to match. We need to either make the two cases distinct (see isStrictlyFGRAsmReg() above) or control the sort order of the table.

================
Comment at: lib/Target/Mips/MipsInstrFPU.td:574-588
@@ -573,1 +573,17 @@
 
+def LoadImmSingleGPR : MipsAsmPseudoInst<(outs GPR32Opnd:$rd),
+                                         (ins imm64:$fpimm),
+                                         "li.s\t$rd, $fpimm">;
+
+def LoadImmSingleFGR : MipsAsmPseudoInst<(outs FGR32Opnd:$rd),
+                                         (ins imm64:$fpimm),
+                                         "li.s\t$rd, $fpimm">;
+
+def LoadImmDoubleGPR : MipsAsmPseudoInst<(outs GPR32Opnd:$rd),
+                                         (ins imm64:$fpimm),
+                                         "li.d\t$rd, $fpimm">;
+
+def LoadImmDoubleFGR : MipsAsmPseudoInst<(outs FGR32Opnd:$rd),
+                                         (ins imm64:$fpimm),
+                                         "li.d\t$rd, $fpimm">;
+
----------------
These should have the appropriate FGR_32, FGR_64, and HARDFLOAT adjectives.

================
Comment at: lib/Target/Mips/MipsInstrFPU.td:586
@@ +585,3 @@
+
+def LoadImmDoubleFGR : MipsAsmPseudoInst<(outs FGR32Opnd:$rd),
+                                         (ins imm64:$fpimm),
----------------
I don't think you mean FGR32Opnd here. You need FGR64Opnd for a 64-bit FPU and AFGR64Opnd for a 32-bit FPU.


http://reviews.llvm.org/D14390