<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 12, 2016 at 10:57 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.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"><div>Yea, this is a nasty problem.</div><div><br></div><div>One important thing to understand is that this is specific to analyses which hold references to other analyses. While this isn't unheard of, it isn't as common as it could be. Still, definitely something we need to address.</div><div><br></div><div>Some ideas about mitigating and fixing it below.</div><span class=""><div dir="ltr"><br></div><div dir="ltr">On Tue, Jul 12, 2016 at 6:15 PM Sean Silva <<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</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"><div dir="ltr"><div><span style="line-height:1.5">How should we solve this? I see two potential solutions:</span><br></div><div><div>1. Analyses must somehow list the analyses they depend on (either by overriding "invalidate" to make sure that they invalidate them, or something "declarative" that would allow the AnalysisManager to walk the transitive dependencies).</div></div></div></blockquote><div><br></div></span><div>I think this is the right approach. I would personally start by overriding the invalidate callback everywhere that it is necessary, and see how bad that becomes.</div><div><br></div><div>If it becomes common and burdensome, then we can change the way invalidation works such that the analysis manager is aware of the preserved analysis set in more detail, and have it build up the necessary data structures to know in-advance whether it must make an explicit invalidate call.</div><div><br></div><div>However, I suspect this may not be *too* bad for two reasons:</div><div><br></div><div>a) As I mentioned above, I'm hoping there aren't *too* many handles between different analyses. But I've not done a careful examination, so we can check this.</div></div></div></blockquote><div><br></div><div>I've replied to one of the posts downthread with some info about this.</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>b) For many analyses that might trigger this, I think we have a simpler option. If the analysis is *immutable* for any reason -- that is, it overrides its invalidate routine to always return "false" the way TargetLibraryInfo should (although I'm not sure it does currently),</div></div></div></blockquote><div><br></div><div>TargetLibraryAnalysis has `run` overloaded for both Module and Function, but TargetLibraryInfo only has the "return false" invalidate override for Module. Probably just an oversight.</div><div><br></div><div><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"><div dir="ltr"><div class="gmail_quote"><div> we shouldn't need to do this as it shouldn't be getting cleared out. Does this make sense? Do others see anything I'm missing with that approach?</div></div></div></blockquote><div><br></div><div>That makes sense, but I think this only applies to a relatively small subset of analyses based on my run-through of PassRegistry.def. <span style="color:rgb(80,0,80);font-size:12.8px">TargetLibraryAnalysis</span> and TargetIRAnalysis I think are the only ones.</div><div><br class="">-- Sean Silva<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"><span class=""><div><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"><div dir="ltr"><div><div>2. The AnalysisManager must do a somewhat complicated dance to track when analyses call back into it in order to get other analyses.</div></div></div></blockquote><div><br></div></span><div>I would really rather avoid this, as currently the analysis manager's logic here is very simple, and in many cases we only need the analyses to *compute* our result, not to embed it. I'm tihnking of stuff like Dominators is used to build LoopInfo, but there isn't a stale handle there.</div><div><br></div><div><br></div><div><br></div><div>There is another aspect of course in that if something is preserving LoopInfo, it really should be preserving Dominators too...</div></div></div></blockquote></div></div><div class="gmail_extra"><div> </div></div></div>