Page MenuHomePhabricator

CFL AA Tests + Minor Fix
ClosedPublic

Authored by george.burgess.iv on Sep 8 2014, 7:28 AM.

Details

Reviewers
hfinkel
Summary

Added some functionality tests for CFL AA, and made a minor fix to CFL AA

  • CFL AA fix: We used to return PartialAlias if *either* variable being queried interacted with arguments or globals. AFAICT, we can change this to only returning PartialAlias iff *both* variables being queried interacted with arguments or globals.
  • Functionality tests: some basic IPA tests, checking that we give conservative responses with arguments/globals thrown in the mix, and ensuring that we trace values through stores and loads.

x interacted with arguments or globals = the Attributes of the StratifiedSet that x belongs to has any bits set.

Diff Detail

Event Timeline

george.burgess.iv retitled this revision from to CFL AA Tests + Minor Fix.
george.burgess.iv updated this object.
george.burgess.iv edited the test plan for this revision. (Show Details)
george.burgess.iv added a reviewer: hfinkel.
george.burgess.iv added a subscriber: Unknown Object (MLST).
hfinkel edited edge metadata.Sep 11 2014, 11:22 PM

Why does PartialAlias have anything to do with arguments or globals? We should return PartialAlias when we know that the two pointers partially overlap.

george.burgess.iv edited edge metadata.

We now return MayAlias instead of PartialAlias if the only thing that could make variables alias is the aliasing of arguments/globals.

hfinkel added inline comments.Sep 18 2014, 2:55 PM
lib/Analysis/CFLAliasAnalysis.cpp
989–999

Can you please add some comments here explaining what is going on? It seems like this is a significant change: Previously, we would return NoAlias only if none of the bits in either AttrsA or AttrsB were set. Now we'll return NoAlias if either of AttrsA or AttrsB have no bits set. So this seems to be doing much more than changing a PartialAlias to a MayAlias in some circumstances.

Added comments as requested. I'm happy to swap the condition back if you don't agree with the change.

hfinkel accepted this revision.Oct 6 2014, 6:35 AM
hfinkel edited edge metadata.

Okay, thanks! LGTM.

P.S. Have you had a change to look at: http://llvm.org/bugs/show_bug.cgi?id=20954

This revision is now accepted and ready to land.Oct 6 2014, 6:35 AM
hfinkel closed this revision.Oct 6 2014, 7:54 AM

r219122.