<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<br>
<div class="moz-cite-prefix">On 04/19/2016 10:21 AM, Easwaran Raman
wrote:<br>
</div>
<blockquote
cite="mid:CAPK5YPbTVGAqnPo6Lgzdm082VGa6dhmYB-p0tG=AKgdAOGwiVg@mail.gmail.com"
type="cite">
<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 moz-do-not-send="true"
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
moz-do-not-send="true"
href="mailto:chandlerc@gmail.com"
target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a></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
moz-do-not-send="true"
href="mailto:eraman@google.com"
target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:eraman@google.com">eraman@google.com</a></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
moz-do-not-send="true"
href="mailto:llvm-dev@lists.llvm.org"
target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a></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
moz-do-not-send="true" href="mailto:hfinkel@anl.gov" target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a></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
moz-do-not-send="true" href="mailto:davidxl@google.com" target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:davidxl@google.com">davidxl@google.com</a></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
moz-do-not-send="true" href="mailto:mehdi.amini@apple.com"
target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a></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>
</div>
</div>
</blockquote>
Looking at the declaration of ProfileSummary in ProfileCommon.h, it
definitely looks like it contains some information which should
simply be associated with the Module/Function. As a side note, the
use of inheritance to represent each profiling "kind" seems highly
suspicious. Outside of the parsing, nothing should need to know the
"kind" of profiling used. That's the key point of the profiling
metadata abstraction.<br>
<br>
Actually, can you give a pointer to where this metadata is defined?
Looking at the definition of getMD, I'm worried we have a far bigger
problem here. It looks like this expects to have different
information stored in metadata based on the kind of profiling. If
that's true, I see that as a major problem. This is something which
would need to be very well justified and I haven't seen that
discussion take place. <br>
<br>
More minor: isFunctionHot and isFunctionUnlikely should be part of
an analysis file.<br>
<br>
<br>
<blockquote
cite="mid:CAPK5YPbTVGAqnPo6Lgzdm082VGa6dhmYB-p0tG=AKgdAOGwiVg@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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 moz-do-not-send="true"
href="mailto:llvm-dev@lists.llvm.org"
target="_blank">llvm-dev@lists.llvm.org</a><br>
<a moz-do-not-send="true"
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 moz-do-not-send="true" href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a moz-do-not-send="true" 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>
</blockquote>
<br>
</body>
</html>