<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Aug 7, 2016 at 11:42 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><div dir="ltr">On Sun, Aug 7, 2016 at 10:45 PM Sean Silva via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Author: silvas<br>
Date: Mon Aug  8 00:38:03 2016<br>
New Revision: 277981<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=277981&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=277981&view=rev</a><br>
Log:<br>
[PM] BasicAA needs to be invalidated since it holds pointers to other stuff.<br></blockquote><div><br></div></span><div>While this may be true, I don't think that is a sufficient explanation for the patch.</div><div><br></div><div>I would much rather you actually send this kind of patch out for review and discussion before landing it.</div><div><br></div><div>For example, the alternative approach of passing analysis into the query path has not been discussed.</div></div></div></blockquote><div><br></div><div><div>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.</div><div><br></div><div>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.</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> Nor has a test case been added that specifically checks this situation.</div></div></div></blockquote><div><br></div><div>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.</div><div><br class="">-- Sean Silva<br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>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.</div></div></div>
</blockquote></div><br></div></div>