[PATCH] D22025: AMDGPU/SI: Do not insert EndCf in an unreachable block

[Matt Arsenault via llvm-commits]

arsenm added inline comments.

================
Comment at: test/CodeGen/AMDGPU/si-annotate-cf-unreachable.ll:30-34
@@ +29,7 @@
+
+bb4:                                              ; preds = %bb3
+  unreachable
+
+bb5:                                              ; preds = %bb3, %bb1
+  unreachable
+}
----------------
cfang wrote:
> arsenm wrote:
> > cfang wrote:
> > > arsenm wrote:
> > > > There should still be another copy of this function with blocks with code (like a volatile store) before the unreachable
> > > The issue is: for an unreachable block, if we insert EndCf, we will hit "Instruction does not dominate all uses!" assert!
> > > 
> > > The "fix" here is actually just a workaround: if there is no instruction before unreachable, we don't insert EndCf.
> > > 
> > > If as you suggested there are instructions before unreachable, we will have to insert endCF, and the problem is still there.
> > > 
> > > This is a special case in control flow that both "else" and "then" is unreachable. Maybe we need to insert endCF on 
> > > both paths. 
> > > 
> > You check at the insertion point if it is unreachable, not that the block ends in unreachable. That's what I want the other test for, to make sure it doesn't crash in that case. I think what you might really want to be doing is checking for no successors
> Can you use a simple example to show me what do you want to test? I am afraid that the test you request should still fail with this patch.

bb4:
  store volatile i32 0, i32 addrspace(1)* undef
  unreachable

This patch should just be an optimization? It is not correct to do something for correctness assuming there is only one instruction in the block ending in unreachable


https://reviews.llvm.org/D22025