<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 4, 2016 at 3:03 AM, 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"><br><br><div class="gmail_quote"><span class=""><div dir="ltr">On Thu, Aug 4, 2016 at 1:33 AM 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"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Aug 4, 2016 at 12:45 AM, 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"><span><div dir="ltr">On Thu, Aug 4, 2016 at 12:32 AM 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">silvas added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D21462#505454" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D21462#505454</a>, @chandlerc wrote:<br>
<br>
> > I hear your argument (IIRC we discussed this at a social) about back pointers being somewhat annoying, but the ability to use just a `void*` to represent the information passed to an analysis makes it much easier to have a unified analysis manager.<br>
><br>
><br>
> I don't think I understand the connection you see between up pointers and using a 'void*' to represent the information passed to an analysis. Can you re-explain this? Sorry if it just isn't clicking for me....<br>
><br>
> That said, in general I would like to not sacrifice type safety because of concerns around mapping arguments from one interface to another though.<br>
<br>
<br>
The issue is that for a unified analysis manager, AnalysisPassConcept is not templated (on IRUnitT and couldn't be on the ExtraArgTs). Therefore, we need to pass a type-erased object through its `run` method. Currently, the type-erased IRUnit can be passed as a void*, and then the AnalysisPassModel (which *is* templated on IRUnitT) just casts it back since it knows the right type.<br>
At least with the current registration stuff, all we need right now for type-safety is tracking which IRUnit it is (and there are 4 discrete values which is easy to track).<br>
<br>
Or to put it another way, the ideas you are running with in this patch is very intimately tied to having the analysis manager templated on the signature of the associated `run` method for analyses it manages. A unified analysis manager (which seems like it was the agreed upon design for solving dependency tracking)</blockquote><div><br></div></span><div>There was no agreement to this. I specifically said I don't at this point agree with that approach, and that I think it is too soon to try to design that approach.</div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Then you need to bring this up on llvmdev for discussion. We had a thread about this, and I have clearly been moving forward with this approach after there seemed to be some agreement on it. If you disagree you need to bring that up.</div></div></div></div></blockquote><div><br></div></span><div>I think I already did in my response to your thread here: <a href="http://www.google.com/url?q=http%3A%2F%2Flists.llvm.org%2Fpipermail%2Fllvm-dev%2F2016-July%2F102245.html&sa=D&sntz=1&usg=AFQjCNFALEelNf-foMSoSYHh_roMivDlPA" target="_blank">http://lists.llvm.org/<wbr>pipermail/llvm-dev/2016-July/<wbr>102245.html</a><br><br>The subsequent discussion</div></div></div></blockquote><div><br></div><div><div>The subsequent discussion also contained Hal saying:<br>"I'll go back to my previous position: we need a general dependency graph built as the analysis cache is used"</div><div><a href="http://lists.llvm.org/pipermail/llvm-dev/2016-July/102333.html">http://lists.llvm.org/pipermail/llvm-dev/2016-July/102333.html</a><br></div><div><br></div><div>With Mehdi replying</div><div>"I share the same feeling."</div><div><a href="http://lists.llvm.org/pipermail/llvm-dev/2016-July/102334.html">http://lists.llvm.org/pipermail/llvm-dev/2016-July/102334.html</a><br></div><div><br></div><div>And no objection from you.</div><div><br></div><div><br></div><div>Your only post later on this topic was: <a href="http://lists.llvm.org/pipermail/llvm-dev/2016-July/102339.html">http://lists.llvm.org/pipermail/llvm-dev/2016-July/102339.html</a></div><div>which was largely about terminology, and some hand-waving of solutions which I explained were inadequate: <a href="http://lists.llvm.org/pipermail/llvm-dev/2016-July/102346.html">http://lists.llvm.org/pipermail/llvm-dev/2016-July/102346.html</a></div><div><br></div><div>And again, you did not provide any response.</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> seemed to center around LCSSA and LoopSimplify which pose separate problems, or the retention of Loop object pointers which also pose different problems.<br></div></div></div></blockquote><div><br></div><div>Retention of Loop object pointers is exactly the same problem. If an analysis result A has a pointer to any object whose lifetime ends when another analysis result B is invalidated, then A must be invalidated when B is invalidated. There is no way around this.</div><div><br></div><div>Just because we could hypothetically refactor a large part of the codebase to avoid some specific instances of the issue doesn't mean that we don't need to solve it. And if we need to solve it anyway, then the argument for doing such a refactoring is quite weak, since it isn't needed.</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><br>I continue to think the plan at the bottom of that message is the correct way forward until we do the mentioned refactorings of analyses and some of the infrastructure stabilizes.</div></div></div></blockquote><div><br></div><div>There are a few cases for which it is not sufficient to just pass in through the query path.</div><div>Also, it's not clear if this is feasible e.g. for AA where different AA's have different information needed by their query paths.</div><div><br></div><div>Also, are you suggesting that we need to modify every query of each of the following analyses before the new PM can be used with basic confidence that there aren't going to be use-after-free errors?</div><div><br></div><div>BFI</div><div>SCEV</div><div>BasicAA</div><div>LVI</div><div>DemandedBits</div><div>MemDep</div><div>MemorySSA</div><div>SCEVAA</div><div>LoopAccessAnalysis</div><div>DependenceAnalysis</div><div><br></div><div>-- Sean Silva</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><span style="line-height:1.5"> </span><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"><span class=""><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>-- Sean Silva</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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><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"> must type-erase the information you have baked into the template arguments of AnalysisPassConcept here.<br>
At least with the way that the registration stuff works now, you would need to find a way to type-erase the exact signature of the `run` method in order to get type-safety.<br></blockquote><div><br></div></span><div>Much of this is why I don't agree with the design.</div><div><br></div><div>I'm happy to actually work on a solution to the problem, but right now I'm in large part waiting on review of these patches.</div><div><br></div><div>If in fact we need to go back and type erase things we can always create a new patch to move in that direction.</div></div></div>
</blockquote></div></div></div></span>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>
</blockquote></div><br></div></div>