[PATCH] D22055: AMDGPU: Fix verifier error with kill intrinsic

[Nicolai Hähnle via llvm-commits]

nhaehnle accepted this revision.
nhaehnle added a comment.
This revision is now accepted and ready to land.

Some small remarks, apart from that LGTM.


================
Comment at: lib/Target/AMDGPU/SILowerControlFlow.cpp:212
@@ -211,3 +211,3 @@
 
 bool SILowerControlFlow::skipIfDead(MachineInstr &MI) {
   MachineBasicBlock &MBB = *MI.getParent();
----------------
The function now logically operates on a block level, so I'd change it to take MBB as a parameter instead of MI.

================
Comment at: lib/Target/AMDGPU/SILowerControlFlow.cpp:217
@@ -216,2 +216,3 @@
+  if (MF->getFunction()->getCallingConv() != CallingConv::AMDGPU_PS ||
       !shouldSkip(&MBB, &MBB.getParent()->back()))
     return false;
----------------
I think the first argument to shouldSkip should be the successor MBB.

That said, shouldSkip itself is a bit broken as a heuristic since it doesn't seem to follow successors itself, so perhaps it makes sense to fix this separately.

================
Comment at: lib/Target/AMDGPU/SILowerControlFlow.cpp:235
@@ -231,2 +234,3 @@
 
-  MBB.addSuccessor(RemainderBB);
+  SkipBB->addSuccessor(&*BBInsertPt);
+  MBB.addSuccessor(SkipBB);
----------------
The SkipBB shouldn't have any successors, should it?


http://reviews.llvm.org/D22055