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

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

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

> There are two ways you can prove hoisting an instruction doesn't introduce undefined behavior.
>
> One is isSafeToSpeculativelyExecute.  This is obvious and uncontroversial.
>
> The other involves proving that if the hoisted instruction has undefined behavior, the program's behavior was undefined anyway.  For example, consider "if (x) { f(a/b); } else { g(a/b); }".  You can hoist a/b out of the if statement because if b is zero, you hit undefined behavior on either path.  Proving safety here is tricky; there are a lot of edge cases you need to deal with to get this right.  For example, you need to check individual instructions with isGuaranteedToTransferExecutionToSuccessor, and you need to handle potentially infinite loops.  (It's possible we could improve the IR to make this easier; see the discussion on https://reviews.llvm.org/D21041 .)
>
> GVNHoist on trunk just blindly hoists anything, without trying to prove either condition.


Well, i don't believe this is quite true.
I believe instead, the checks are hidden and a complete mess, such that it's not obvious that it's either correct or complete.


https://reviews.llvm.org/D29092