[PATCH] D153511: [BasicAA] Don't short-circuit non-capturing arguments

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D153511#4442266 <https://reviews.llvm.org/D153511#4442266>, @fhahn wrote:

> In D153511#4442180 <https://reviews.llvm.org/D153511#4442180>, @nikic wrote:
>
>> You don't really need an assume to show the issue, this simpler variant exhibits the same problem: https://llvm.godbolt.org/z/M3K5fG97G The store is removed even though the call reads it.
>>
>> This just happens to mostly work out fine right now because such calls get DCEd, so we don't observe end-to-end miscompiles. If capture tracking were slightly smarter and followed the return value (as we effectively do in the assume case) we would get observable miscompiles even without ephemeral value interactions.
>
> Yeah, one issue seems to be that AA uses CaptureTracking as proxy for escaping (e.g. EarliestEscapeInfo by DSE), but in some cases escaping doesn't imply capturing and things go wrong. I *think* tracking escapes (instead of captures) would make sense in general, as this should help to avoid most code-size changes seen by this patch I suspect.

CaptureTracking handles both captures and escapes. There is no escape in this case. No escape does //not// imply that memory is not accessed, just that provenance is not //retained// after the call.

BasicAA is making assumptions about how the CaptureTracking implementation works here, which goes beyond the actual capture/escape properties. Those assumptions are close, but not quite correct.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153511/new/

https://reviews.llvm.org/D153511