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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 23 17:03:13 PST 2017


efriedma added a comment.

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.  This isn't causing real-world issues outside of OptForMinSize mode because, in an unrelated check, GVNHoist doesn't hoist anything past calls which write to memory.


https://reviews.llvm.org/D29092





More information about the llvm-commits mailing list