Page MenuHomePhabricator

[AA] Move earliest escape tracking from DSE to AA
ClosedPublic

Authored by nikic on Thu, Sep 23, 2:28 PM.

Details

Summary

This is a followup to D109844 (and alternative to D109907), which integrates the new "earliest escape" tracking into AliasAnalysis. This is done by replacing the pre-existing context-free capture cache in AAQueryInfo with a replaceable (virtual) object with two implementations: The SimpleCaptureInfo implements the previous behavior (check whether object is captured at all), while EarliestEscapeInfo implements the new behavior from DSE.

This combines the "earliest escape" analysis with the full power of BasicAA: It subsumes the call handling from D109907, considers a wider range of escape sources, a wider context, and works with AA recursion.

It does come at a slightly higher compile-time cost: http://llvm-compile-time-tracker.com/compare.php?from=1e3c6fc7cb9d2ee6a5328881f95d6643afeadbff&to=23296d5394689e40a0b4376f080cfab18c43442c&stat=instructions (The comparison point would be https://llvm-compile-time-tracker.com/compare.php?from=7faf1285f2c416494aacf7ee13e70e330b2f0540&to=a4964cf36bb0f777d82d6a1025435bbd9f67bc4f&stat=instructions for D109907).

Diff Detail

Event Timeline

nikic created this revision.Thu, Sep 23, 2:28 PM
nikic requested review of this revision.Thu, Sep 23, 2:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Sep 23, 2:28 PM
fhahn added a comment.Fri, Sep 24, 1:58 AM

This combines the "earliest escape" analysis with the full power of BasicAA:

Are there any test cases that show the benefit of that, other than the improvements for the load-load case discussed in D109844 ?

nikic updated this revision to Diff 374927.Fri, Sep 24, 12:27 PM

Rebase over additional tests.

nikic updated this revision to Diff 374933.Fri, Sep 24, 12:47 PM

Fix unit test build.

This combines the "earliest escape" analysis with the full power of BasicAA:

Are there any test cases that show the benefit of that, other than the improvements for the load-load case discussed in D109844 ?

See the last three tests in captures-before-load.ll. Only the last one truly requires BasicAA integration though.

Ultimately my main interest here is to integrate this functionality in a way that does not require reimplementing parts of AA in DSE. Better analysis quality is just a nice side effect.

fhahn accepted this revision.Sat, Sep 25, 12:18 PM

This combines the "earliest escape" analysis with the full power of BasicAA:

Are there any test cases that show the benefit of that, other than the improvements for the load-load case discussed in D109844 ?

See the last three tests in captures-before-load.ll. Only the last one truly requires BasicAA integration though.

Great thanks!

Ultimately my main interest here is to integrate this functionality in a way that does not require reimplementing parts of AA in DSE. Better analysis quality is just a nice side effect.

Agreed, just wanted to double check w.r.t. the description.

LGTM, I am just wondering if the change in isEscapeSource is required or can be split off?

llvm/include/llvm/Analysis/AliasAnalysis.h
383

nit: could just be a struct?

407

I should have added doc-comments in the original DSE version but unfortunately did not. Might be good to add some here.

llvm/lib/Analysis/BasicAliasAnalysis.cpp
122–123

this seems unrelated or could at least be split off?

This revision is now accepted and ready to land.Sat, Sep 25, 12:18 PM
This revision was landed with ongoing or failed builds.Sat, Sep 25, 1:47 PM
This revision was automatically updated to reflect the committed changes.
nikic marked 3 inline comments as done.