<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div style='font-family: arial,helvetica,sans-serif; font-size: 10pt; color: #000000'><br><br><hr id="zwchr"><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" <davidxl@google.com><br><b>To: </b>"Mehdi Amini" <mehdi.amini@apple.com><br><b>Cc: </b>"Chandler Carruth" <chandlerc@gmail.com>, "Hal Finkel" <hfinkel@anl.gov>, "via llvm-dev" <llvm-dev@lists.llvm.org><br><b>Sent: </b>Monday, April 18, 2016 4:38:12 PM<br><b>Subject: </b>Re: [llvm-dev] Move InlineCost.cpp out of Analysis?<br><br><div dir="ltr"><br><div class="gmail_extra"><br><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:<br><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;"><br><div><span class=""><blockquote><div>On Apr 18, 2016, at 2:28 PM, Xinliang David Li <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>> wrote:</div><br><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 18, 2016 at 2:18 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: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr">The difference between Analysis and Transforms is *not* about passes, but about what the code *does*.<div><br></div><div>Code for mutating the IR should be in Transforms, and code that analyzes the IR without mutating it should be in Analysis. This is why, for example, InstructionSimplify is in Analysis -- it does not mutate the IR in any way.</div><div><br></div><div>So I think InlineCost and similar things should stay in the Analysis library regardless of whether they are passes or not.</div></div></blockquote><div><br></div><div>Is that the only criteria (IR mod or not) ? Most of the transformations have pass specific analysis (that are not shared with other clients) -- those code stay with the transformation -- it does not make sense to move those code in to Analysis. For the same reason, we need to ask first if InlineCost is intended to be a shared utility?</div></div></div></div></div></blockquote><div><br></div></span><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 id="DWT12024">Yes -- this is a very good point.</div></div></div></div></blockquote>Independent of anything else, +1.<br><br> -Hal<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_extra"><div class="gmail_quote"><div></div><div><br></div><div>David</div><div><br></div><div> </div><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><br></div><div>Moving something from Analysis to TransformUtils just because it needs to access ProfileData is a slippery slope...</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-- </div><div>Mehdi</div></font></span><div><div class="h5"><div><br></div><br><blockquote><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>David</div><div><br></div><div> </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><div><div><br><div class="gmail_quote"><div dir="ltr">On Mon, Apr 18, 2016 at 2:14 PM Mehdi Amini via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</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;"><br>
> On Apr 18, 2016, at 2:07 PM, Hal Finkel via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
> <hr id="zwchr"><br>
>> From: "Easwaran Raman" <<a href="mailto:eraman@google.com" target="_blank">eraman@google.com</a>><br>
>> To: "via llvm-dev" <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
>> Cc: "Chandler Carruth" <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, "Philip Reames"<br>
>> <<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>>, "David Li" <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>><br>
>> Sent: Monday, April 18, 2016 2:39:49 PM<br>
>> Subject: Move InlineCost.cpp out of Analysis?<br>
>><br>
>><br>
>> Hi,<br>
>><br>
>><br>
>> After r256521 - which removes InlineCostAnalysis class - I think<br>
>> there is no strong reason for InlineCost.cpp to be part of the<br>
>> Analysis library. Is it fine to make it part of TransformUtils?<br>
>><br>
><br>
> Given that InlineCost is not really an analysis any longer, I think this is fine.<br>
<br>
Isn't it? It is not a pass, but I see it as an analysis utils.<br>
<br>
><br>
>><br>
>> I submitted r266477 (which has now been reverted) that made Analysis<br>
>> depend on ProfileData in order to obtain ProfileSummary for the<br>
>> module, but there is an existing dependency of ProfileData on<br>
>> Analysis (through Object and BitCode).<br>
<br>
The real issue is that BitCode depends on Analysis I think.<br>
I'm not sure about ProfileData that depends on Bitcode, do you know why?<br>
<br>
--<br>
Mehdi<br>
<br>
<br>
>> Moving InlineCost.cpp under<br>
>> Transforms/Utils will fix this issue. There are other ways to fix<br>
>> this (make Inliner.cpp get the ProfileSummary and pass it to<br>
>> InlineCost, for example), but I think it makes sense to move<br>
>> InlineCost.<br>
>><br>
>><br>
>> Thoughts?<br>
>><br>
>><br>
>> Thanks,<br>
>> Easwaran<br>
>><br>
>><br>
><br>
> --<br>
> Hal Finkel<br>
> Assistant Computational Scientist<br>
> Leadership Computing Facility<br>
> Argonne National Laboratory<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>
_______________________________________________<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>
</blockquote></div></div></div></div></div>
</blockquote></div><br></div></div>
</div></blockquote></div></div></div><br></div></blockquote></div><br></div></div>
</blockquote><br><br><br>-- <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></div></body></html>