Page MenuHomePhabricator

[JumpThreading] PRE unordered loads
ClosedPublic

Authored by sanjoy on Jul 13 2016, 5:20 PM.

Details

Summary

Extend JumpThreading's PRE to unordered atomic loads.

Diff Detail

Repository
rL LLVM

Event Timeline

sanjoy updated this revision to Diff 63888.Jul 13 2016, 5:20 PM
sanjoy retitled this revision from to [JumpThreading] PRE unordered loads.
sanjoy updated this object.
sanjoy added reviewers: hfinkel, reames.
sanjoy added a subscriber: llvm-commits.
majnemer added inline comments.
lib/Transforms/Scalar/JumpThreading.cpp
927 ↗(On Diff #63888)

This comment seems wrong now.

sanjoy updated this revision to Diff 63892.Jul 13 2016, 5:48 PM
  • address review comments
sanjoy added a reviewer: jfb.Jul 14 2016, 10:53 AM
jfb edited edge metadata.Jul 14 2016, 11:11 AM

Could you test that:

  • non-atomic loads aren't forwarded to atomic loads
  • non-atomic stores aren't forwarded to atomic loads (this seems useless, maybe it's valid?)

lgtm otherwise.

lib/Transforms/Scalar/JumpThreading.cpp
1059 ↗(On Diff #63892)

Li->isVolatile() seems simpler than having the comment.

Will add the negative tests as you suggest.

lib/Transforms/Scalar/JumpThreading.cpp
1059 ↗(On Diff #63892)

That sounds like an unrelated (to this patch) change. I've removed the /*isVolatile*/ comment for now (so that bit stays the same), let me know if you'd like me to s/false/LI->isVolatile()/ separately.

This revision was automatically updated to reflect the committed changes.