Page MenuHomePhabricator

[BasicAA] Fix handling of invariant group launders

Authored by amharc on May 15 2018, 1:10 PM.



A recent patch (rL331587) to Capture Tracking taught it that the launder_invariant_group intrinsic captures its argument only by returning it. Unfortunately, BasicAA still considered every call instruction as a possible escape source and hence concluded that the result of a launder_invariant_group call cannot alias any local non-escaping value. This led to bug 37458.

This patch updates the relevant check for escape sources in BasicAA.

Diff Detail


Event Timeline

amharc created this revision.May 15 2018, 1:10 PM

I went throught it and it looks good to me, I didn't expect that fixing CaptureTracking can lead Alias Analisis to return NoAlias instead of MustAlias. It would be nice if the code would check for such assumptions, but I have no ideas how it could be implemented.

We should also have a direct AA test (something like test/Analysis/BasicAA/cs-cs.ll).

Also, let's not repeat this problem in the future. Please add a comment both here and in CaptureTracking to remind people that intrinsics added to once place must be added to the other place.

138 ↗(On Diff #146901)

Don't need the { } here.

amharc updated this revision to Diff 147035.May 16 2018, 3:25 AM

Added a direct AA test and cautionary comments in both BasicAA and CaptureTracking, removed unnecessary { }.

amharc marked an inline comment as done.May 16 2018, 3:26 AM

Removed unnecessary { }.

hfinkel accepted this revision.May 16 2018, 4:39 AM


This revision is now accepted and ready to land.May 16 2018, 4:39 AM
xbolva00 accepted this revision.May 16 2018, 4:44 AM
Closed by commit rL332466: [BasicAA] Fix handling of invariant group launders (authored by kpszeniczny, committed by ). · Explain WhyMay 16 2018, 6:20 AM
This revision was automatically updated to reflect the committed changes.