Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Looks mostly ok. See my inlined comment about handling sltu.
lib/Target/Mips/Mips64InstrInfo.td | ||
---|---|---|
723–734 ↗ | (On Diff #89638) | Rather than providing an alias which covers the imm64 case, instead provide an pseudo instruction like the old version of NORImm which takes which takes GPR64Opnds and imm64. Then unconditionally expand it like NORImm. and set the final opcode to the corresponding comparison. You will be able to provide a two operand alias then. |
LGTM with the inline comments addressed. The logic error with the (outs) and (ins) appears to be doing the right thing, but should be corrected anyway.
Thanks for doing these patches.
lib/Target/Mips/Mips64InstrInfo.td | ||
---|---|---|
695 ↗ | (On Diff #89723) | This entire hunk should be under "Assembler Pseudo Instructions". |
698–700 ↗ | (On Diff #89723) | Logic error: the GPR64Opnd:$rs should be with the outs. Formatting. Keep the MipsAsmPseudoInst on the same line as the 'def'. When the line exceeds 80 chars, break it at the (ins ) construct, and align the (ins ..) with the outs above. E.g: def SLTImm64 : MipsAsmPseudoInst<(outs GPR64Opnd:$rs), (ins GPR64Opnd:$rt, imm64:$imm), "slt\t$rs, $rt, $imm">, GPR_64; |
703–705 ↗ | (On Diff #89723) | Logic error: the GPR64Opnd:$rs should be with the outs. Formatting as well. |
707 ↗ | (On Diff #89723) | imm64:$imm should be aligned under the GPR64Opnd:$rs. |
lib/Target/Mips/MipsInstrInfo.td | ||
2520 ↗ | (On Diff #89723) | Logic error: RO: $rs should be with the 'outs'. |
If this looks good now could you please commit for me?
lib/Target/Mips/MipsInstrInfo.td | ||
---|---|---|
2520 ↗ | (On Diff #89723) | That makes sense, I was a bit surprised when I copied it from the existing NORImm. |