[RISCV] Add an operand kind to the opcode/imm returned from RISCVMatInt.

[craig.topper]

Summary

Instead of matching opcodes to know the format to emit, use an
enum value that comes along with the opcode.

Change the consumers to use fully covered switches so that we get
a compiler warning if a new kind is added. With the opcode checks
it was easier to forget to update one of the 3 consumers.

[reames]

I want to suggest an alternate, and possibly simpler change.

Instead of adding a field to Inst, what if we added a helper routine instead which just did the switch on opcode internally and returned the Kind? The client code would look largely the same, but without needing to explicitly track the kind of each opcode at the construction site.

[craig.topper]

In D126317#3535274, @reames wrote:

I want to suggest an alternate, and possibly simpler change.

Instead of adding a field to Inst, what if we added a helper routine instead which just did the switch on opcode internally and returned the Kind? The client code would look largely the same, but without needing to explicitly track the kind of each opcode at the construction site.

I like it.

[craig.topper]

Add a method to convert opcode to operand kind. All opcodes listed with an
unreachable default to make sure it always gets updated if new opcodes are used.

[reames]

LGTM

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

 
 void RISCVAsmParser::emitLoadImm(MCRegister DestReg, int64_t Value,
                                  MCStreamer &Out) {
   RISCVMatInt::InstSeq Seq =
       RISCVMatInt::generateInstSeq(Value, getSTI().getFeatureBits());
 
   MCRegister SrcReg = RISCV::X0;
   for (RISCVMatInt::Inst &Inst : Seq) {
-    if (Inst.Opc == RISCV::LUI) {
+    switch (Inst.getOpndKind()) {
+    case RISCVMatInt::Imm:
+      emitToStreamer(Out,
+                     MCInstBuilder(Inst.Opc).addReg(DestReg).addImm(Inst.Imm));
+      break;
+    case RISCVMatInt::RegX0:
       emitToStreamer(
-          Out, MCInstBuilder(RISCV::LUI).addReg(DestReg).addImm(Inst.Imm));
-    } else if (Inst.Opc == RISCV::ADD_UW) {
-      emitToStreamer(Out, MCInstBuilder(RISCV::ADD_UW)
-                              .addReg(DestReg)
-                              .addReg(SrcReg)
-                              .addReg(RISCV::X0));
-    } else if (Inst.Opc == RISCV::SH1ADD || Inst.Opc == RISCV::SH2ADD ||
-               Inst.Opc == RISCV::SH3ADD) {
+          Out, MCInstBuilder(Inst.Opc).addReg(DestReg).addReg(SrcReg).addReg(
+                   RISCV::X0));
+      break;
+    case RISCVMatInt::RegReg:
       emitToStreamer(
           Out, MCInstBuilder(Inst.Opc).addReg(DestReg).addReg(SrcReg).addReg(
                    SrcReg));
-    } else {
+      break;
+    case RISCVMatInt::RegImm:
       emitToStreamer(
           Out, MCInstBuilder(Inst.Opc).addReg(DestReg).addReg(SrcReg).addImm(
                    Inst.Imm));
+      break;
     }
 
     // Only the first instruction has X0 as its source.
     SrcReg = DestReg;
   }
 }
 
 void RISCVAsmParser::emitAuipcInstPair(MCOperand DestReg, MCOperand TmpReg,

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.h

 #include "llvm/ADT/SmallVector.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include <cstdint>
 
 namespace llvm {
 class APInt;
 
 namespace RISCVMatInt {
+
+enum OpndKind {
+  RegImm, // ADDI/ADDIW/SLLI/SRLI/BSETI/BCLRI
+  Imm,    // LUI
+  RegReg, // SH1ADD/SH2ADD/SH3ADD
+  RegX0,  // ADD_UW
+};
+
 struct Inst {
   unsigned Opc;
   int64_t Imm;
 
   Inst(unsigned Opc, int64_t Imm) : Opc(Opc), Imm(Imm) {}
+
+  OpndKind getOpndKind() const;
 };
 using InstSeq = SmallVector<Inst, 8>;
 
 // Helper to generate an instruction sequence that will materialise the given
 // immediate value into a register. A sequence of instructions represented by a
 // simple struct is produced rather than directly emitting the instructions in
 // order to allow this helper to be used from both the MC layer and during
 // instruction selection.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp

int getIntMatCost(const APInt &Val, unsigned Size,
   int Cost = 0;
   for (unsigned ShiftVal = 0; ShiftVal < Size; ShiftVal += PlatRegSize) {
     APInt Chunk = Val.ashr(ShiftVal).sextOrTrunc(PlatRegSize);
     InstSeq MatSeq = generateInstSeq(Chunk.getSExtValue(), ActiveFeatures);
     Cost += getInstSeqCost(MatSeq, HasRVC);
   }
   return std::max(1, Cost);
 }
+
+OpndKind Inst::getOpndKind() const {
+  switch (Opc) {
+  default:
+    llvm_unreachable("Unexpected opcode!");
+  case RISCV::LUI:
+    return RISCVMatInt::Imm;
+  case RISCV::ADD_UW:
+    return RISCVMatInt::RegX0;
+  case RISCV::SH1ADD:
+  case RISCV::SH2ADD:
+  case RISCV::SH3ADD:
+    return RISCVMatInt::RegReg;
+  case RISCV::ADDI:
+  case RISCV::ADDIW:
+  case RISCV::SLLI:
+  case RISCV::SRLI:
+  case RISCV::SLLI_UW:
+  case RISCV::RORI:
+  case RISCV::BSETI:
+  case RISCV::BCLRI:
+    return RISCVMatInt::RegImm;
+  }
+}
+
 } // namespace RISCVMatInt
 } // namespace llvm

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

static SDNode *selectImm(SelectionDAG *CurDAG, const SDLoc &DL, const MVT VT,
   if (Subtarget.useConstantPoolForLargeInts() &&
       Seq.size() > Subtarget.getMaxBuildIntsCost())
     return selectImmWithConstantPool(CurDAG, DL, VT, Imm, Subtarget);
 
   SDNode *Result = nullptr;
   SDValue SrcReg = CurDAG->getRegister(RISCV::X0, XLenVT);
   for (RISCVMatInt::Inst &Inst : Seq) {
     SDValue SDImm = CurDAG->getTargetConstant(Inst.Imm, DL, XLenVT);
-    if (Inst.Opc == RISCV::LUI)
-      Result = CurDAG->getMachineNode(RISCV::LUI, DL, XLenVT, SDImm);
-    else if (Inst.Opc == RISCV::ADD_UW)
-      Result = CurDAG->getMachineNode(RISCV::ADD_UW, DL, XLenVT, SrcReg,
+    switch (Inst.getOpndKind()) {
+    case RISCVMatInt::Imm:
+      Result = CurDAG->getMachineNode(Inst.Opc, DL, XLenVT, SDImm);
+      break;
+    case RISCVMatInt::RegX0:
+      Result = CurDAG->getMachineNode(Inst.Opc, DL, XLenVT, SrcReg,
                                       CurDAG->getRegister(RISCV::X0, XLenVT));
-    else if (Inst.Opc == RISCV::SH1ADD || Inst.Opc == RISCV::SH2ADD ||
-             Inst.Opc == RISCV::SH3ADD)
+      break;
+    case RISCVMatInt::RegReg:
       Result = CurDAG->getMachineNode(Inst.Opc, DL, XLenVT, SrcReg, SrcReg);
-    else
+      break;
+    case RISCVMatInt::RegImm:
       Result = CurDAG->getMachineNode(Inst.Opc, DL, XLenVT, SrcReg, SDImm);
+      break;
+    }
 
     // Only the first instruction has X0 as its source.
     SrcReg = SDValue(Result, 0);
   }
 
   return Result;
 }
 

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

void RISCVInstrInfo::movImm(MachineBasicBlock &MBB,
   if (!STI.is64Bit() && !isInt<32>(Val))
     report_fatal_error("Should only materialize 32-bit constants for RV32");
 
   RISCVMatInt::InstSeq Seq =
       RISCVMatInt::generateInstSeq(Val, STI.getFeatureBits());
   assert(!Seq.empty());
 
   for (RISCVMatInt::Inst &Inst : Seq) {
-    if (Inst.Opc == RISCV::LUI) {
-      BuildMI(MBB, MBBI, DL, get(RISCV::LUI), DstReg)
+    switch (Inst.getOpndKind()) {
+    case RISCVMatInt::Imm:
+      BuildMI(MBB, MBBI, DL, get(Inst.Opc), DstReg)
           .addImm(Inst.Imm)
           .setMIFlag(Flag);
-    } else if (Inst.Opc == RISCV::ADD_UW) {
-      BuildMI(MBB, MBBI, DL, get(RISCV::ADD_UW), DstReg)
+      break;
+    case RISCVMatInt::RegX0:
+      BuildMI(MBB, MBBI, DL, get(Inst.Opc), DstReg)
           .addReg(SrcReg, RegState::Kill)
           .addReg(RISCV::X0)
           .setMIFlag(Flag);
-    } else if (Inst.Opc == RISCV::SH1ADD || Inst.Opc == RISCV::SH2ADD ||
-               Inst.Opc == RISCV::SH3ADD) {
+      break;
+    case RISCVMatInt::RegReg:
       BuildMI(MBB, MBBI, DL, get(Inst.Opc), DstReg)
           .addReg(SrcReg, RegState::Kill)
           .addReg(SrcReg, RegState::Kill)
           .setMIFlag(Flag);
-    } else {
+      break;
+    case RISCVMatInt::RegImm:
       BuildMI(MBB, MBBI, DL, get(Inst.Opc), DstReg)
           .addReg(SrcReg, RegState::Kill)
           .addImm(Inst.Imm)
           .setMIFlag(Flag);
+      break;
     }
+
     // Only the first instruction has X0 as its source.
     SrcReg = DstReg;
   }
 }
 
 static RISCVCC::CondCode getCondFromBranchOpc(unsigned Opc) {
   switch (Opc) {
   default: