<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 14, 2017 at 8:56 AM, Vedant Kumar via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">vsk added a comment.<br>
<span class=""><br>
In <a href="https://reviews.llvm.org/D32073#727016" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32073#727016</a>, @davidxl wrote:<br>
<br>
> The main client (the compiler) should not need to know the details so a higher level interface taking 'Module' should be better.<br>
<br>
<br>
</span>I see your point, but taking a 'Module' doesn't seem to be nicer, because it forces us to expose more APIs which are very slightly different from each other. This leaks details because in order to pick the right API, you need to know the low level details (e.g whether or not segment prefixes are needed).<br>
<span class=""><br></span></blockquote><div><br></div><div>The only distinction between two sets of APIs is compiler vs other host tools.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>> Wdyt of removing all of the get*SectionName helpers, and just exposing: getInstrProfSectionName(<wbr>InstrProfSectKind IPSK, ObjectFileKind OFK, bool AddSegmentPrefix). It will make client code more verbose but very clear / unambiguous.<br>
><br>
</span><span class="">> It is the host tool that need to use the lower level interface. For  the later use, there was never a need for segment prefix, so adding the flexibility is not necessary.<br>
<br>
</span>We could set 'bool AddSegmentPrefix = true' by default. This still provides some extra flexibility, but the right option is picked by default, so there should be less confusion.<br></blockquote><div><br></div><div>If you prefer strongly about unifying the APIs, we should probably do this after this feature settles. We (including you ;) ) can do this as a followup patch and we can go from there. For now, I think it is better to reduce churns.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
If you'd prefer to extend the set of get*SectionName() functions, I just ask that the names of the "getXSectionName(bool)" and "getXSectionName(Module *)" functions be made different.<br></blockquote><div><br></div><div>Ok, I will try to come up with a different name.</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">
<br>
<br>
<a href="https://reviews.llvm.org/D32073" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32073</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>