[AMDGPU] Moving SI_RETURN_TO_EPILOG handling out of SIInsertSkips.

[cdevadas]

Summary

For now, moving it to SIPreEmitPeephole.
Should find a right place to have this code.

[arsenm]

I would expect there to be a test change? Did we not have a MIR test of this before?

llvm/lib/Target/AMDGPU/SIPreEmitPeephole.cpp

166

Should elaborate that this is not an optimization

[arsenm]

What's the status of this?

[cdevadas]

Rebase + added lit test

[cdevadas]

Ping

[nhaehnle]

LGTM

llvm/lib/Target/AMDGPU/SIInsertSkips.cpp

 
 bool SIInsertSkips::runOnMachineFunction(MachineFunction &MF) {
   const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
   TII = ST.getInstrInfo();
   TRI = &TII->getRegisterInfo();
   MDT = &getAnalysis<MachineDominatorTree>();
   SkipThreshold = SkipThresholdFlag;
 
-  MachineBasicBlock *EmptyMBBAtEnd = nullptr;
   SmallVector<MachineInstr *, 4> KillInstrs;
   bool MadeChange = false;
 
   for (MachineBasicBlock &MBB : MF) {
     MachineBasicBlock::iterator I, Next;
     for (I = MBB.begin(); I != MBB.end(); I = Next) {
       Next = std::next(I);
       MachineInstr &MI = *I;
for (I = MBB.begin(); I != MBB.end(); I = Next) {
           // change because it modifies the CFG.
           KillInstrs.push_back(&MI);
         } else {
           MI.eraseFromParent();
         }
         break;
       }
 
-      case AMDGPU::SI_RETURN_TO_EPILOG:
-        // FIXME: Should move somewhere else
-        assert(!MF.getInfo<SIMachineFunctionInfo>()->returnsVoid());
-
-        // Graphics shaders returning non-void shouldn't contain S_ENDPGM,
-        // because external bytecode will be appended at the end.
-        if (&MBB != &MF.back() || &MI != &MBB.back()) {
-          // SI_RETURN_TO_EPILOG is not the last instruction. Add an empty block at
-          // the end and jump there.
-          if (!EmptyMBBAtEnd) {
-            EmptyMBBAtEnd = MF.CreateMachineBasicBlock();
-            MF.insert(MF.end(), EmptyMBBAtEnd);
-          }
-
-          MBB.addSuccessor(EmptyMBBAtEnd);
-          BuildMI(MBB, &MI, MI.getDebugLoc(), TII->get(AMDGPU::S_BRANCH))
-            .addMBB(EmptyMBBAtEnd);
-          MI.eraseFromParent();
-
-          MDT->getBase().insertEdge(&MBB, EmptyMBBAtEnd);
-        }
-        break;
-
       default:
         break;
       }
     }
   }
 
   for (MachineInstr *Kill : KillInstrs) {
     skipIfDead(*Kill->getParent(), std::next(Kill->getIterator()),
                Kill->getDebugLoc());
     Kill->eraseFromParent();
   }
   KillInstrs.clear();
 
   return MadeChange;
 }

llvm/lib/Target/AMDGPU/SIPreEmitPeephole.cpp

bool SIPreEmitPeephole::optimizeVccBranch(MachineInstr &MI) const {
   return true;
 }
 
 bool SIPreEmitPeephole::runOnMachineFunction(MachineFunction &MF) {
   const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
   TII = ST.getInstrInfo();
   TRI = &TII->getRegisterInfo();
   bool Changed = false;
+  MachineBasicBlock *EmptyMBBAtEnd = nullptr;
 
   for (MachineBasicBlock &MBB : MF) {
     MachineBasicBlock::iterator MBBI = MBB.getFirstTerminator();
     if (MBBI == MBB.end())
       continue;
 
     MachineInstr &MI = *MBBI;
     switch (MI.getOpcode()) {
     case AMDGPU::S_CBRANCH_VCCZ:
     case AMDGPU::S_CBRANCH_VCCNZ:
       Changed |= optimizeVccBranch(MI);
       break;
+
+    case AMDGPU::SI_RETURN_TO_EPILOG:
+      // FIXME: Should move somewhere else
+      assert(!MF.getInfo<SIMachineFunctionInfo>()->returnsVoid());
+
+      // Graphics shaders returning non-void shouldn't contain S_ENDPGM,
+      // because external bytecode will be appended at the end.
+      if (&MBB != &MF.back() || &MI != &MBB.back()) {
+        // SI_RETURN_TO_EPILOG is not the last instruction. Add an empty block
+        // at the end and jump there.
+        if (!EmptyMBBAtEnd) {
+          EmptyMBBAtEnd = MF.CreateMachineBasicBlock();
+          MF.insert(MF.end(), EmptyMBBAtEnd);
+        }
+
+        MBB.addSuccessor(EmptyMBBAtEnd);
+        BuildMI(MBB, &MI, MI.getDebugLoc(), TII->get(AMDGPU::S_BRANCH))
+            .addMBB(EmptyMBBAtEnd);
+        MI.eraseFromParent();
+      }
+      break;
+
     default:
       break;
     }
   }
 
   return Changed;
 }