This is an archive of the discontinued LLVM Phabricator instance.

[BasicAA] Guard intrinsics don't write to memory
ClosedPublic

Authored by sanjoy on Apr 26 2016, 6:28 PM.

Details

Summary

The idea is very close to what we do for assume intrinsics: we mark the
guard intrinsics as writing to arbitrary memory to maintain control
dependence, but under the covers we teach AA that they do not alias any
particular memory location.

Diff Detail

Event Timeline

sanjoy updated this revision to Diff 55150.Apr 26 2016, 6:28 PM
sanjoy retitled this revision from to [BasicAA] Learn about guard intrinsics don't write memory.
sanjoy updated this object.
sanjoy added reviewers: chandlerc, hfinkel.
sanjoy added a subscriber: llvm-commits.
sanjoy retitled this revision from [BasicAA] Learn about guard intrinsics don't write memory to [BasicAA] Guard intrinsics don't write to memory.Apr 28 2016, 12:57 PM
sanjoy updated this object.
sanjoy added a reviewer: gbiv.
chandlerc added inline comments.Apr 28 2016, 10:08 PM
lib/Analysis/BasicAliasAnalysis.cpp
775–779

Why MRI_Ref instead of NoModRef? And in general, why anything different from assume?

798–809

Again, why different behavior from assume?

sanjoy added inline comments.Apr 28 2016, 10:18 PM
lib/Analysis/BasicAliasAnalysis.cpp
775–779

Guards have deoptimization state (via "deopt" operand bundles) and are thus allowed to read all memory. If a guard fails then, at the time of failure, the heap better look like it was supposed to look like in the original program. E.g. see @test6 in D19578.

george.burgess.iv added inline comments.
lib/Analysis/BasicAliasAnalysis.cpp
775–779

Can you add a comment in the code noting this, please? :)

reames added inline comments.Apr 29 2016, 2:12 PM
lib/Analysis/BasicAliasAnalysis.cpp
777

As others pointed out, this comment is really confusing.

reames requested changes to this revision.May 5 2016, 6:07 PM
reames edited edge metadata.
This revision now requires changes to proceed.May 5 2016, 6:07 PM
sanjoy updated this revision to Diff 56376.May 5 2016, 6:34 PM
sanjoy edited edge metadata.
  • Add comments, as asked for in the review
sanjoy updated this revision to Diff 56617.May 9 2016, 1:13 PM
sanjoy edited edge metadata.
  • Clarify comments a bit more (s/aliases/mods/), and also ping
reames accepted this revision.May 9 2016, 6:25 PM
reames edited edge metadata.

LGTM w/minor comment.

test/Analysis/BasicAA/guards.ll
26

please rename to reflect this is an readonly call.

This revision is now accepted and ready to land.May 9 2016, 6:25 PM
This revision was automatically updated to reflect the committed changes.
sanjoy marked an inline comment as done.