[PATCH] D76445: [RISCV][GlobalISel] Select ALU GPR instructions

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVGISel.td:30
+class PatSExtOpWGI<SDPatternOperator OpNode, RVInst Inst>
+    : Pat<(i64 (sra (shl (OpNode GPR:$rs1, GPR:$rs2), (i64 32)), (i64 32))),
+          (Inst GPR:$rs1, GPR:$rs2)>;
----------------
Should sext_inreg be legal?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:46
+  getRegClassForTypeOnBank(LLT Ty, const RegisterBank &RB,
+                           bool GetAllRegSet = false) const;
   bool selectImpl(MachineInstr &I, CodeGenCoverage &CoverageInfo) const;
----------------
What is this parameter?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:109
+
+  if (Register::isPhysicalRegister(SrcReg) &&
+      Register::isPhysicalRegister(DstReg))
----------------
SrcReg.isPhysical()


================
Comment at: llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:120
+    if (!RBI.constrainGenericRegister(SrcReg, *SrcRC, MRI)) {
+      LLVM_DEBUG(dbgs() << "Failed to constrain " << TII.getName(I.getOpcode())
+                        << " operand\n");
----------------
Why not just print the whole instruction?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:145
+  int64_t Val = I.getOperand(1).getCImm()->getSExtValue();
+  MachineIRBuilder MIRBuilder(I);
+
----------------
Should not construct one off MachineIRBuilders. The selector usually just directly uses BuildMI


================
Comment at: llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:171
+    return false;
+
+  return true;
----------------
Didn't erase the instruction


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76445/new/

https://reviews.llvm.org/D76445