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

Nicolai Hähnle via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 9 10:09:44 PDT 2016

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?


More information about the llvm-commits mailing list