[amdgpu] Fix a crash case when `V_CNDMASK` could be simplified.

[hliao]

Summary

• Once an instruction is simplified, foldable candidates from it should be invalidated or skipped as the operand index is no longer valid.

[arsenm]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1260

Doesn't this add a second mechanism to avoid the same problem? We already check isUseMIInFoldList to avoid revisiting

[hliao]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1260

That one is used to prevent adding a commuted instr into the candidate list. But the case here is that both operands could be folded. Also, if used here, that check is too expensive? That candidate list will be scanned in square times.

[arsenm]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1260

Should we just use a SetVector for FoldList then?

[hliao]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1260

That list is a list of operand foldable, i.e., the pair MI and one of its operand being folded. If we change that to be keyed by MI, besides major data structure change, we may add extra overhead (set vs list) when building that candidate list. Considering that tryFoldInst only simplifies V_CNDMASK so far, is that overhead too big to justify the skip list here?

[arsenm]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1260

Hmmm. In principle tryFoldInst would erase / replace the instruction, but it just happens to not. I do think the way this pass works is backwards, but I guess this is fine for now

[hliao]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

1260

Thanks for the code review

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

if (FoldingImm) {
     }
   }
 
   MachineFunction *MF = MI.getParent()->getParent();
   // Make sure we add EXEC uses to any new v_mov instructions created.
   for (MachineInstr *Copy : CopiesToReplace)
     Copy->addImplicitDefUseOperands(*MF);
 
+  SmallPtrSet<MachineInstr *, 16> Folded;
   for (FoldCandidate &Fold : FoldList) {
     assert(!Fold.isReg() || Fold.OpToFold);
+    if (Folded.count(Fold.UseMI))
+      continue;
     if (Fold.isReg() && Fold.OpToFold->getReg().isVirtual()) {
       Register Reg = Fold.OpToFold->getReg();
       MachineInstr *DefMI = Fold.OpToFold->getParent();
       if (DefMI->readsRegister(AMDGPU::EXEC, TRI) &&
           execMayBeModifiedBeforeUse(*MRI, Reg, *DefMI, *Fold.UseMI))
         continue;
     }
     if (updateOperand(Fold, *TII, *TRI, *ST)) {
       // Clear kill flags.
       if (Fold.isReg()) {
         assert(Fold.OpToFold && Fold.OpToFold->isReg());
         // FIXME: Probably shouldn't bother trying to fold if not an
         // SGPR. PeepholeOptimizer can eliminate redundant VGPR->VGPR
         // copies.
         MRI->clearKillFlags(Fold.OpToFold->getReg());
       }
       LLVM_DEBUG(dbgs() << "Folded source from " << MI << " into OpNo "
                         << static_cast<int>(Fold.UseOpNo) << " of "
                         << *Fold.UseMI << '\n');
-      tryFoldInst(TII, Fold.UseMI);
+      if (tryFoldInst(TII, Fold.UseMI))
+        Folded.insert(Fold.UseMI);
     } else if (Fold.isCommuted()) {
       // Restoring instruction's original operand order if fold has failed.
       TII->commuteInstruction(*Fold.UseMI, false);
     }
   }
 }
 
 // Clamp patterns are canonically selected to v_max_* instructions, so only

llvm/test/CodeGen/AMDGPU/fold-cndmask-wave32.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=amdgcn -mcpu=gfx1030 -run-pass si-fold-operands -verify-machineinstrs -o - %s | FileCheck %s
+
+---
+name:            fold_cndmask
+tracksRegLiveness: true
+registers:
+body:             |
+  bb.0.entry:
+    ; CHECK-LABEL: name: fold_cndmask
+    ; CHECK: [[DEF:%[0-9]+]]:sreg_32_xm0_xexec = IMPLICIT_DEF
+    ; CHECK: [[S_MOV_B32_:%[0-9]+]]:sreg_32 = S_MOV_B32 0
+    ; CHECK: [[V_MOV_B32_e32_:%[0-9]+]]:vgpr_32 = V_MOV_B32_e32 0, implicit $exec
+    ; CHECK: [[COPY:%[0-9]+]]:vgpr_32 = COPY [[S_MOV_B32_]]
+    %0:sreg_32_xm0_xexec = IMPLICIT_DEF
+    %1:sreg_32 = S_MOV_B32 0
+    %2:vgpr_32 = COPY %1:sreg_32
+    %3:vgpr_32 = V_CNDMASK_B32_e64 0, %1:sreg_32, 0, %2:vgpr_32, %0:sreg_32_xm0_xexec, implicit $exec
+
+...