[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?
http://reviews.llvm.org/D22055
More information about the llvm-commits
mailing list