[llvm] r277981 - [PM] BasicAA needs to be invalidated since it holds pointers to other stuff.

[Sean Silva via llvm-commits]

On Sun, Aug 7, 2016 at 11:42 PM, Chandler Carruth <chandlerc at google.com>
wrote:

> On Sun, Aug 7, 2016 at 10:45 PM Sean Silva via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: silvas
>> Date: Mon Aug  8 00:38:03 2016
>> New Revision: 277981
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=277981&view=rev
>> Log:
>> [PM] BasicAA needs to be invalidated since it holds pointers to other
>> stuff.
>>
>
> While this may be true, I don't think that is a sufficient explanation for
> the patch.
>
> I would much rather you actually send this kind of patch out for review
> and discussion before landing it.
>
> For example, the alternative approach of passing analysis into the query
> path has not been discussed.
>

We have discussed that in the thread "[PM] I think that the new PM needs to
learn about inter-analysis dependencies..." and there isn't a clear
solution for how to pass into the query path for alias analyses.

Also, at least in the current state of the tree, it is incorrect for
BasicAA to say that it is "immutable"/"stateless" since it does in fact
hold state. Of course, if we ever figure out how to pass query-dependencies
into the AA query path then we can get rid of the state and can change this
back.


> Nor has a test case been added that specifically checks this situation.
>

If you think that would be useful I would be happy to add one. However,
AFAIK we don't have tests for any other analyses specifically checking that
they are not "immutable". For example, we don't have such a test for SCEVAA
even though it is in a completely analogous situation AFAICT.

-- Sean Silva



>
> I'm not saying that this change is necessarily wrong, just trying to point
> out what I think would be a better process to follow.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160808/3ef6b486/attachment.html>