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

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 08:17:51 PDT 2023


reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVMCInstLower.cpp:226
+  if (OutNumOperands < OutMCID.getNumOperands()) {
+    assert(OutMI.getNumOperands() + 1 == OutMCID.getNumOperands());
+    if (OutMCID.operands()[OutNumOperands].RegClass == RISCV::VMV0RegClassID)
----------------
You can reuse OutNumOperands here.  Or remove it, and fold the call into it's single use.  It's mostly the inconsistency which is odd. 


================
Comment at: llvm/lib/Target/RISCV/RISCVMCInstLower.cpp:227
+    assert(OutMI.getNumOperands() + 1 == OutMCID.getNumOperands());
+    if (OutMCID.operands()[OutNumOperands].RegClass == RISCV::VMV0RegClassID)
+      OutMI.addOperand(MCOperand::createReg(RISCV::NoRegister));
----------------
I'm guessing this if should actually be an assert?  If the RegClass didn't match, we'd fail the operand check just below.

Or alternatively, maybe reorganize the whole clause as:

```
if (OutMI.getNumOperands() + 1 == OutMCID.getNumOperands() &&
   OutMCID.operands()[OutNumOperands].RegClass == RISCV::VMV0RegClassID)
  OutMI.addOperand(MCOperand::createReg(RISCV::NoRegister))
```

The single assert at the end of the function then covers all the other cases.  


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