[PATCH] D152050: [RISCV] Begin removing hasDummyMask.

Fraser Cormack via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 5 01:58:28 PDT 2023


frasercrmck added a comment.

Makes sense to me as an approach.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:3186
+  // If this instruction is tail agnostic, the unmasked instruction should not
+  // have a // merge op.
   uint64_t TSFlags = TII.get(Opc).TSFlags;
----------------
this comment's mis-formatted


================
Comment at: llvm/lib/Target/RISCV/RISCVMCInstLower.cpp:226
+  if (OutNumOperands != OutMCID.getNumOperands()) {
+    if (OutMCID.operands()[OutNumOperands].RegClass == RISCV::VMV0RegClassID)
+      OutMI.addOperand(MCOperand::createReg(RISCV::NoRegister));
----------------
This could maybe be a bit safer? If `OutMI` had more operands than the MCID accounts for, this access would do bad things.

Maybe rejigging that following assert so that it first asserts that `OutMI` has one fewer operand than `OutMCID`? That doesn't negate the need for the later assert, if we're conditionally adding the extra operand. Maybe moving that later assert to just before the return statement as a more global check that we've done the right thing throughout the course of the function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152050



More information about the llvm-commits mailing list