[PATCH] D30667: GVNHoist: handle basic blocks with UnreachableInst

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added a comment.

In https://reviews.llvm.org/D30667#694450, @dberlin wrote:

> In https://reviews.llvm.org/D30667#694347, @hiraditya wrote:
>
> > In https://reviews.llvm.org/D30667#693739, @dberlin wrote:
> >
> > > Uh, won't this already return false on the call to longjmp?
> >
> >
> > GVNHoist only calls isGuaranteedToTransferExecutionToSuccessor for terminators.
>
>
> This is not safe to do.
>  LLVM's basic blocks are really closer to EBB's.
>  You can't assume the only place it will return false is at a terminator. It can and will return false for calls in the middle of the block, for example.


I agree. Still I think there is a bug in isGuaranteedToTransferExecutionToSuccessor. What if there is a basic block like this with only an unreachable instruction.

  exit:
    unreachable

In this case isGuaranteedToTransferExecutionToSuccessor should return false for the unreachable instruction.


https://reviews.llvm.org/D30667