<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 18, 2016 at 4:36 PM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"><span class="">
<br>
<br>
<div>On 04/18/2016 04:05 PM, Easwaran Raman
via llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Mon, Apr 18, 2016 at 3:25 PM,
Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank"></a><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 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: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 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: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 Mon, Apr 18, 2016
at 2:48 PM Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank"></a><a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>
wrote:<br>
</div>
</span>
<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>
<div>
<div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)"><br>
<br>
<hr>
<blockquote style="border-left-width:2px;border-left-style:solid;border-left-color: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"></a><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-width:2px;border-left-style:solid;border-left-color: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"></a><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-width:1px;border-left-style:solid;border-left-color: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)">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>
</span>
<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>
</div>
</blockquote>
<div>Sorry, I'm lost here. There is an IR annotation (module
flag) called ProfileSummary and this data is represented
in memory by the ProfileSummary class. . This class
provides methods to serialize/de-serialize this data
into/from metadata. This class has methods to compute this
summary, but this is used only by the profile readers and
writers. I am not sure what you mean by "it isn't just* an
annotation on the IR". <br>
</div>
</div>
</div>
</div>
</blockquote></span>
If this is true, why is this class currently part of ProfileData?
Shouldn't this live in IR? Or to phrase this differently, why is an
"analysis" over IR mixed in with the parsing code?</div></blockquote><div><br></div><div>ProfileSummary does not have any analysis over IR, so I don't understand that part of your question. As to why this is part of ProfileData, it is partly because initially summary was something that was just displayed by the llvm-profdata tool and partly because it felt natural to keep all profile related code together. Having said that, I have nothing against moving it to IR if that is the appropriate place.</div><div><br></div><div>- Easwaran</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<div>- Easwaran</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 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>
<span>
<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_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>
</span>
<div>Well, this will also re-highlight the fundamental
pass manager problem as you won't have easy access
to this analysis pass.</div>
<span>
<div><span style="line-height:1.5"><br>
</span></div>
<div><span style="line-height:1.5"> </span></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">
<div><br>
</div>
<div>- Easwaran</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><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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color: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>
</span></div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
<br>
<fieldset></fieldset>
<br>
<pre>_______________________________________________
LLVM Developers mailing list
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
<br>
</span></div>
</blockquote></div><br></div></div>