This is an archive of the discontinued LLVM Phabricator instance.

[Sink] Fix predicate in legality check
ClosedPublic

Authored by loladiro on May 14 2017, 9:28 PM.

Details

Summary

isSafeToSpeculativelyExecute is the wrong predicate to use here.
All that checks for is whether it is safe to hoist a value due to
unaligned/un-dereferencable accesses. However, not only are we doing
sinking rather than hoisting, our concern is that the location
we're loading from may have been modified. Instead forbid sinking
any load across a critical edge.

Event Timeline

loladiro created this revision.May 14 2017, 9:28 PM
davide added a subscriber: davide.Jun 1 2017, 4:06 PM

Bump. Would be great if somebody could review this. I know Sink it's not really actively developed, but I think this is a pretty straightforward fix.

I'm gonna go ahead and commit this anyway, since this fixes a miscompile. If there are any comments I'll take them in post-commit review.

This revision was automatically updated to reflect the committed changes.