[BasicAA] Fix handling of invariant group launders
ClosedPublic

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

Details

Summary

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

Repository
rL LLVM
amharc created this revision.Tue, May 15, 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.

llvm/lib/Analysis/BasicAliasAnalysis.cpp
138 ↗(On Diff #146901)

Don't need the { } here.

amharc updated this revision to Diff 147035.Wed, May 16, 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.Wed, May 16, 3:26 AM

Removed unnecessary { }.

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

LGTM

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