llvm-reduce: Do not try to delete frame instructions

[arsenm]

Summary

The verifier enforces these appearing as balanced pairs, so just
deleting one has no real chance of producing something valid.

[markus]

I recall running into this as well. So this patch will make sure that they are never deleted thus improving reduction speed. But wasn't there a chance that the oracle would indicated that both instructions of the pair were to be deleted which could result in a valid reduction? I guess this is fine though and a better solution would be to deal with the frame pairs in a separate reduction pass anyway.

[markus]

llvm/test/tools/llvm-reduce/mir/remove-frame-destroy.mir

5

Is this criteria for interestingness safe given that we have two S_NOP 0 instructions in the block? Could it be that the first one gets eliminated and then it is still considered interesting but the final RESULT-NEXT fails because there is now a S_NOP 0 in the middle?

[arsenm]

In D124383#3471743, @markus wrote:

I recall running into this as well. So this patch will make sure that they are never deleted thus improving reduction speed. But wasn't there a chance that the oracle would indicated that both instructions of the pair were to be deleted which could result in a valid reduction? I guess this is fine though and a better solution would be to deal with the frame pairs in a separate reduction pass anyway.

I was thinking a frame reduction pass would be better

[arsenm]

llvm/test/tools/llvm-reduce/mir/remove-frame-destroy.mir

5

With this condition both nops should be deleted (the first one won't be deleted until the other patch for first instruction handling)

[arsenm]

0b896b754e864951717731058f4b23875d617ca8

llvm/test/tools/llvm-reduce/mir/remove-frame-destroy.mir

+# REQUIRES: amdgpu-registered-target
+# RUN: llvm-reduce --delta-passes=instructions -mtriple=amdgcn-amd-amdhsa --test FileCheck --test-arg --check-prefix=CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t 2> %t.log
+# RUN: FileCheck --check-prefix=RESULT %s < %t
+
+# CHECK-INTERESTINGNESS: S_NOP 0
+
+# RESULT: ADJCALLSTACKUP
+# RESULT-NEXT: ADJCALLSTACKDOWN
+# RESULT-NEXT: S_ENDPGM 0
+
+---
+name: frame_setup_destroy
+tracksRegLiveness: true
+machineFunctionInfo:
+  stackPtrOffsetReg: '$sgpr32'
+body:             |
+  bb.0:
+    S_NOP 0
+    ADJCALLSTACKUP 0, 0, implicit-def dead $scc, implicit-def $sgpr32, implicit $sgpr32
+    S_NOP 0
+    ADJCALLSTACKDOWN 0, 0, implicit-def dead $scc, implicit-def $sgpr32, implicit $sgpr32
+    S_ENDPGM 0
+
+...

llvm/tools/llvm-reduce/deltas/ReduceInstructionsMIR.cpp

for (auto &MO : MI.operands()) {
       if (MRI->getRegClassOrRegBank(Reg) == RC && MRI->getType(Reg) == Ty &&
           !ExcludeMIs.count(MO.getParent()))
         return Reg;
     }
   }
   return 0;
 }
 
+static bool shouldNotRemoveInstruction(const TargetInstrInfo &TII,
+                                       const MachineInstr &MI) {
+  if (MI.isTerminator())
+    return true;
+
+  // The MIR is almost certainly going to be invalid if frame instructions are
+  // deleted individually since they need to come in balanced pairs, so don't
+  // try to delete them.
+  if (MI.getOpcode() == TII.getCallFrameSetupOpcode() ||
+      MI.getOpcode() == TII.getCallFrameDestroyOpcode())
+    return true;
+
+  return false;
+}
+
 static void extractInstrFromFunction(Oracle &O, MachineFunction &MF) {
   MachineDominatorTree MDT;
   MDT.runOnMachineFunction(MF);
 
   auto MRI = &MF.getRegInfo();
   SetVector<MachineInstr *> ToDelete;
 
   const TargetSubtargetInfo &STI = MF.getSubtarget();
   const TargetInstrInfo *TII = STI.getInstrInfo();
   MachineBasicBlock *EntryMBB = &*MF.begin();
   MachineBasicBlock::iterator EntryInsPt =
       EntryMBB->SkipPHIsLabelsAndDebug(EntryMBB->begin());
 
   // Mark MIs for deletion according to some criteria.
   for (auto &MBB : MF) {
     for (auto &MI : MBB) {
-      if (MI.isTerminator())
+      if (shouldNotRemoveInstruction(*TII, MI))
         continue;
       if (!O.shouldKeep())
         ToDelete.insert(&MI);
     }
   }
 
   // For each MI to be deleted update users of regs defined by that MI to use
   // some other dominating definition (that is not to be deleted).