<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 18, 2016 at 3:31 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:#000000"><br><br><hr><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><b>From: </b>"Chandler Carruth" <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>><br><b>To: </b>"Easwaran Raman" <<a href="mailto:eraman@google.com" target="_blank">eraman@google.com</a>><br><b>Cc: </b>"Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, "Xinliang David Li" <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>>, "via llvm-dev" <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br><b>Sent: </b>Monday, April 18, 2016 5:25:03 PM<span class=""><br><b>Subject: </b>Re: [llvm-dev] Move InlineCost.cpp out of Analysis?<br><br></span><span class=""><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Apr 18, 2016 at 3:20 PM Easwaran Raman <<a href="mailto:eraman@google.com" target="_blank">eraman@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 18, 2016 at 3:00 PM, Chandler Carruth via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><div dir="ltr">On Mon, Apr 18, 2016 at 2:48 PM Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>> wrote:<br></div></span><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)"><br><br><hr><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><b>From: </b>"Xinliang David Li" <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>><br></blockquote></div></div></span><span><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)"><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 18, 2016 at 2:33 PM, Mehdi Amini <span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div><div>In the current case at stake: the issue is that we can't make the Analysis library using anything from the ProfileData library. Conceptually there is a problem IMO.</div></div></div></blockquote><div><br></div><div><br></div><div>Yes -- this is a very good point.</div></div></div></div></blockquote></div></div><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)"><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"></blockquote>Independent of anything else, +1.<br></div></div></span></blockquote><div><br></div><div>The design of ProfileData and reading profile information in the entire middle end had a really fundamental invariant that folks seem to have lost track of:</div><div><br></div><div>a) There is exactly *one* way to get at profile information from general analyses and transforms: a dedicated analysis pass that manages access to the profile info.</div><div><br></div><div>b) There is exactly *one* way for this analysis to compute this information from an *external* profile source: profile metadata attached to the IR.</div><div><br></div><div>c) There could be many external profile sources, but all of them should be read and then translated into metadata annotations on the IR so that serialization / deserialization preserve them in a common format and we can reason about how they work.</div><div><br></div><div><br></div><div>This layering is why it is only a transform that accesses ProfileData -- it is responsible for annotating the IR and nothing else. <span style="line-height:1.5">Then the analysis uses these annotations and never reads the data directly.</span></div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5">I think this is a really important separation of concerns as it ensures that we don't get an explosion of different analyses supporting various different subsets of profile sources.</span></div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5">Now, the original design only accounted for profile information *within* a function body, clearly it needs to be extended to support intraprocedural information. But I would still expect that to follow a similar layering where we first read the data into IR annotations, then have an analysis pass (this time a module analysis pass in all likelihood) that brokers access to these annotations through an API that can do intelligent things like synthesizing it from the "cold" attribute or whatever when missing.</span></div><span><font color="#888888"><div><span style="line-height:1.5"><br></span></div></font></span></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Invariants b) and c) above are still true, but a) is not since InlineCost directly calls ProfileSummary instead of through an analysis pass.</div></div></div></div></blockquote><div><br></div><div>Not quite -- ProfileSummary seems to only exist in the profile *reading* code, so it isn't *just* an annotation on the IR.</div><div><br></div><div>I think the problem here is that we have failed to build a proper separate abstraction around the interprocedural profile data that is extracted from IR annotations. That abstraction should have nothing to do with reading profile data and so shouldn't live in the ProifileData library, it should (IMO) live in the Analysis library.</div><div><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>  I'll add a module level pass as you suggest, but that still needs breaking the ProfileData->Analysis dependence chain. </div></div></div></div></blockquote><div><br></div><div>Well, this will also re-highlight the fundamental pass manager problem as you won't have easy access to this analysis pass.</div><div><span style="line-height:1.5"><br></span></div></div></div></span></blockquote>Could this be an immutable pass?</div></div></blockquote><div><br></div><div>I think yes, but this is independent of the issue at hand -- removing the circular dependency introduced (due to dep of cov mapping on bitcode writer ....). Easwaran is working on that without requiring moving InlineCostAnalysis.</div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:#000000"><span class="HOEnZb"><font color="#888888"><br><br> -Hal</font></span><span class=""><br><br><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><div dir="ltr"><div class="gmail_quote"><div><span style="line-height:1.5"></span></div><div><span style="line-height:1.5"> </span></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>- Easwaran</div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><font color="#888888"><div><span style="line-height:1.5"></span></div><div><span style="line-height:1.5">-Chandler</span></div></font></span></div></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div></div></div></blockquote></div></div>
</blockquote><br><br><br></span><span class="">-- <br><div><span name="x"></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></span></div></div></blockquote></div><br></div></div>