[PATCH] D29092: PR31729: [GVNHoist] Don't hoist unsafe scalars at -Oz

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added a comment.

In https://reviews.llvm.org/D29092#685323, @efriedma wrote:

> > Thanks for the feedback. GVNHoist does that. Please see the GVNHoist:298 hoistingFromAllPaths(). It will hoist instructions only when they are very busy.
>
> Oh, okay.  That's... the right sort of thing, but I think it's missing some checks.  The most obvious hole is that you need to call isGuaranteedToTransferExecutionToSuccessor on each instruction in the block, not just the terminator.  It also looks like it doesn't handle infinite loops correctly.


It checks for Branches with isGuaranteedToTransferExecutionToSuccessor, other than that there are memory instructions and calls in the definition of isGuaranteedToTransferExecutionToSuccessor, which are handled separately (with some help from MemorySSA) here. So there is no need to call this function for each instruction.


https://reviews.llvm.org/D29092