<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 18, 2016, at 2:24 PM, Easwaran Raman <<a href="mailto:eraman@google.com" class="">eraman@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">Thanks for the comments.<br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Mon, Apr 18, 2016 at 2:13 PM, Mehdi Amini<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:mehdi.amini@apple.com" target="_blank" class="">mehdi.amini@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><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 class="">> On Apr 18, 2016, at 2:07 PM, Hal Finkel via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a>> wrote:<br class=""><span class="">><br class="">> ----- Original Message -----<br class="">>> From: "Easwaran Raman" <<a href="mailto:eraman@google.com" target="_blank" class="">eraman@google.com</a>><br class="">>> To: "via llvm-dev" <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a>><br class="">>> Cc: "Chandler Carruth" <<a href="mailto:chandlerc@gmail.com" target="_blank" class="">chandlerc@gmail.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank" class="">hfinkel@anl.gov</a>>, "Philip Reames"<br class="">>> <<a href="mailto:listmail@philipreames.com" target="_blank" class="">listmail@philipreames.com</a>>, "David Li" <<a href="mailto:davidxl@google.com" target="_blank" class="">davidxl@google.com</a>><br class="">>> Sent: Monday, April 18, 2016 2:39:49 PM<br class="">>> Subject: Move InlineCost.cpp out of Analysis?<br class="">>><br class="">>><br class="">>> Hi,<br class="">>><br class="">>><br class="">>> After r256521 - which removes InlineCostAnalysis class - I think<br class="">>> there is no strong reason for InlineCost.cpp to be part of the<br class="">>> Analysis library. Is it fine to make it part of TransformUtils?<br class="">>><br class="">><br class=""></span>> Given that InlineCost is not really an analysis any longer, I think this is fine.<br class=""><br class="">Isn't it? It is not a pass, but I see it as an analysis utils.<br class=""></blockquote><div class="">Yes, I meant that it is not an analysis pass. It does perform analysis. I see one such example of something that performs analysis being added under Transforms/Utils: CmpInstAnalysis.cpp.</div></div></div></div></div></blockquote><div><br class=""></div><div>If CmpInstAnalysis.cpp is not mutating the IR, it shouldn't sit here in my opinion.</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><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;"><span class=""><br class="">><br class="">>><br class="">>> I submitted r266477 (which has now been reverted) that made Analysis<br class="">>> depend on ProfileData in order to obtain ProfileSummary for the<br class="">>> module, but there is an existing dependency of ProfileData on<br class="">>> Analysis (through Object and BitCode).<br class=""><br class=""></span>The real issue is that BitCode depends on Analysis I think.<br class=""></blockquote><div class="">I think that is due to ThinLTO's use of getBlockProfileCount.</div></div></div></div></div></blockquote><div><br class=""></div><div>Yeah, I know but we should find a way to break this...</div><div><br class=""></div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><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;">I'm not sure about ProfileData that depends on Bitcode, do you know why?<br class=""></blockquote><div class="">ProfileData (specifically CoverageMapping{Reader|Writer}) depends on Object which depends on Bitcode.</div></div></div></div></div></blockquote><div><br class=""></div><div>Some refactoring or split could help here, this does not seem desirable to me.</div><div><br class=""></div><div>-- </div><div>Mehdi</div><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><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;"><div class=""><div class=""><br class=""></div><div class=""><br class=""><br class="">>> Moving InlineCost.cpp under<br class="">>> Transforms/Utils will fix this issue. There are other ways to fix<br class="">>> this (make Inliner.cpp get the ProfileSummary and pass it to<br class="">>> InlineCost, for example), but I think it makes sense to move<br class="">>> InlineCost.<br class="">>><br class="">>><br class="">>> Thoughts?<br class="">>><br class="">>><br class="">>> Thanks,<br class="">>> Easwaran<br class="">>><br class="">>><br class="">><br class=""></div></div><span class=""><font color="#888888" class="">> --<br class="">> Hal Finkel<br class="">> Assistant Computational Scientist<br class="">> Leadership Computing Facility<br class="">> Argonne National Laboratory<br class="">> _______________________________________________<br class="">> LLVM Developers mailing list<br class="">><span class="Apple-converted-space"> </span><a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a><br class="">><span class="Apple-converted-space"> </span><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a></font></span></blockquote></div></div></div></div></blockquote></div><br class=""></body></html>