[PATCH] D133868: [RISCV] Fix a silent miscompile in copyPhysReg

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: craig.topper, frasercrmck, asb, kito-cheng.
Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, bollu, simoncook, johnrusso, rbar, hiraditya, arichardson, mcrosier.
Herald added a project: All.
reames requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

Found this when adding verifier rules.  The case which arises is that we have a DefMBBI which has a VecPolicy operand.  The code was not expecting this, and the unconditional copy of the last two operands resulted in the SEW and VecPolicy fields being added to the VMV_V_V as AVL and SEW respectively.

Oddly, this appears to be a silent in practice.  There's no test change despite verifier changes proving that we definitely hit this in existing tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133868

Files:
  llvm/lib/Target/RISCV/RISCVInstrInfo.cpp


Index: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -235,13 +235,14 @@
           if (RISCVII::isRVVWideningReduction(TSFlags))
             return false;
 
-          // Found the definition.
-          FoundDef = true;
-          DefMBBI = MBBI;
           // If the producing instruction does not depend on vsetvli, do not
           // convert COPY to vmv.v.v. For example, VL1R_V or PseudoVRELOAD.
-          if (!RISCVII::hasSEWOp(TSFlags))
+          if (!RISCVII::hasSEWOp(TSFlags) || !RISCVII::hasVLOp(TSFlags))
             return false;
+
+          // Found the definition.
+          FoundDef = true;
+          DefMBBI = MBBI;
           break;
         }
       }
@@ -361,11 +362,9 @@
   if (IsScalableVector) {
     bool UseVMV_V_V = false;
     MachineBasicBlock::const_iterator DefMBBI;
-    unsigned DefExplicitOpNum;
     unsigned VIOpc;
     if (isConvertibleToVMV_V_V(STI, MBB, MBBI, DefMBBI, LMul)) {
       UseVMV_V_V = true;
-      DefExplicitOpNum = DefMBBI->getNumExplicitOperands();
       // We only need to handle LMUL = 1/2/4/8 here because we only define
       // vector register classes for LMUL = 1/2/4/8.
       switch (LMul) {
@@ -403,10 +402,9 @@
       else
         MIB = MIB.addReg(SrcReg, getKillRegState(KillSrc));
       if (UseVMV_V_V) {
-        // The last two arguments of vector instructions are
-        // AVL, SEW. We also need to append the implicit-use vl and vtype.
-        MIB.add(DefMBBI->getOperand(DefExplicitOpNum - 2)); // AVL
-        MIB.add(DefMBBI->getOperand(DefExplicitOpNum - 1)); // SEW
+        const MCInstrDesc &Desc = DefMBBI->getDesc();
+        MIB.add(DefMBBI->getOperand(RISCVII::getVLOpNum(Desc))); // AVL
+        MIB.add(DefMBBI->getOperand(RISCVII::getSEWOpNum(Desc))); // SEW
         MIB.addReg(RISCV::VL, RegState::Implicit);
         MIB.addReg(RISCV::VTYPE, RegState::Implicit);
       }
@@ -435,8 +433,9 @@
           MIB = MIB.addReg(TRI->getSubReg(SrcReg, SubRegIdx + I),
                            getKillRegState(KillSrc));
         if (UseVMV_V_V) {
-          MIB.add(DefMBBI->getOperand(DefExplicitOpNum - 2)); // AVL
-          MIB.add(DefMBBI->getOperand(DefExplicitOpNum - 1)); // SEW
+          const MCInstrDesc &Desc = DefMBBI->getDesc();
+          MIB.add(DefMBBI->getOperand(RISCVII::getVLOpNum(Desc))); // AVL
+          MIB.add(DefMBBI->getOperand(RISCVII::getSEWOpNum(Desc))); // SEW
           MIB.addReg(RISCV::VL, RegState::Implicit);
           MIB.addReg(RISCV::VTYPE, RegState::Implicit);
         }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133868.460116.patch
Type: text/x-patch
Size: 2671 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220914/34bb05a3/attachment.bin>