<div dir="ltr"><div class="gmail_default" style="font-family:monospace;font-size:small;color:#000000">ok thanks. SGTM.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 25, 2021 at 11:44 AM Ellis Hoag <<a href="mailto:ellis.sparky.hoag@gmail.com">ellis.sparky.hoag@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">While I am not familiar with the PDB format, most of the changes are independent of the debug info format type. For the instrumentation counters, we emit a `!DIGlobalVariable()` in the LLVM IR which is emitted as Dwarf or CodeView automatically. That being said, I was planning on first extending the `llvm-profdata` tool to handle Dwarf and then I can add support for CodeView if needed.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 25, 2021 at 12:58 PM Petr Hosek <<a href="mailto:phosek@google.com" target="_blank">phosek@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>From our perspective, using debug info wouldn't be an issue since we always include it in every build.</div><div><br></div><div>The issue that hasn't been brought up yet is that the proposed solution only covers DWARF, but the IR instrumentation also supports Windows which uses PDB. More generally, the existing implementation is independent of the debugging format being used. I'm not familiar with PDB to say whether the proposed solution could be extended to also support Windows, but if not then we'll have to support both the use of __llvm_prf_data and __llvm_prf_names sections as well as debug info which seems like an extra complexity.</div><div><br></div><div>I agree with David that making __llvm_prf_names strippable and cutting down the size of __llvm_prf_data would be a worthwhile effort independently of the proposed changes.</div><div><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 20, 2021 at 10:00 AM Xinliang David Li <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div style="font-family:monospace;font-size:small;color:rgb(0,0,0)"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 19, 2021 at 5:12 PM Wenlei He <<a href="mailto:wenlei@fb.com" target="_blank">wenlei@fb.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div lang="EN-US">
<div>
<p class="MsoNormal">> <span style="font-size:12pt;font-family:"Courier New";color:black">
While dwarf is a standard way of program annotation, using it for instrumentation PGO does mean an additional dependency (instead of being self contained).<u></u><u></u></span></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">I think your point on having debug info as dependency is fair. But given most release builds need to generate debug info anyways, this dependency seems to be a minor downside. Hence the tradeoff between extra dependency and minimum binary
size can be worthwhile. In the end, this does not “regress” an existing feature by adding extra dependency – i.e. it doesn’t affect today’s IRPGO; and the actual dependency, the debug info is already commonly used. Hope this is an acceptable trade-off.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">> <span style="font-size:12pt;font-family:"Courier New";color:black">
This proposal requires debug_type info to be emitted, right? What is the object size and compile time overhead? If this can be trimmed, it is a reasonable way to emit the profile data mapping information at compile time.<u></u><u></u></span></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">We don’t really require debug_type, so yes it can be trimmed as a dependency. However, in practice whether we should trim it also depends on whether it makes sense from dwarf perspective, and if it’s worth a new mode/variant for dwarf info.
I tend it view the trimming of debug_type as orthogonal to this proposal – as long as we keep the lightweight PGO technically independent of debug types, the trimming can be done later if it’s appropriate from debug info perspective and when the actual use
case arises.</p></div></div></blockquote><div><br></div><div><br></div><div><div style="font-family:monospace;font-size:small;color:rgb(0,0,0)">Thanks. +Vedant and +Petr for their opinion on using debug info for matching purposes. </div><div style="font-family:monospace;font-size:small;color:rgb(0,0,0)"><br></div><div style="font-family:monospace;font-size:small;color:rgb(0,0,0)">David</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="EN-US"><div><p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Thanks,<u></u><u></u></p>
<p class="MsoNormal">Wenlei<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div style="border-color:rgb(181,196,223) currentcolor currentcolor;border-style:solid none none;border-width:1pt medium medium;padding:3pt 0in 0in">
<p class="MsoNormal" style="margin-bottom:12pt"><b><span style="font-size:12pt;color:black">From:
</span></b><span style="font-size:12pt;color:black">Xinliang David Li <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>><br>
<b>Date: </b>Monday, October 18, 2021 at 2:20 PM<br>
<b>To: </b>Wenlei He <<a href="mailto:wenlei@fb.com" target="_blank">wenlei@fb.com</a>><br>
<b>Cc: </b>Ellis Hoag <<a href="mailto:ellis.sparky.hoag@gmail.com" target="_blank">ellis.sparky.hoag@gmail.com</a>>, Kyungwoo Lee <<a href="mailto:kyulee@fb.com" target="_blank">kyulee@fb.com</a>>, llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
<b>Subject: </b>Re: [llvm-dev] [InstrProfiling] Lightweight Instrumentation<u></u><u></u></span></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"><u></u> <u></u></span></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Mon, Oct 18, 2021 at 1:38 PM Wenlei He <<a href="mailto:wenlei@fb.com" target="_blank">wenlei@fb.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-color:currentcolor currentcolor currentcolor rgb(204,204,204);border-style:none none none solid;border-width:medium medium medium 1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal">Thanks for the feedback, David.
<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">You’re right that most of the savings comes from coarse-grained instrumentation. However, the situation we’re facing for mobile (and also embedded systems) comes with very tight
size constraints. Some components are already built with -Oz, and we’re constantly on the lookout for extra MiB to save so more “features” can get into the components.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">For clang self-build example, 7M overhead is much better than 50M+, and 50M->7M indeed look close to 50M->4M as improvements. But comparing to non-PGO, this is still +7M vs +4M.
The extra 3M is considered quite significant, and could potentially be a deal breaker for some cases.
<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">In short, this is “close” as you mentioned, but not good enough still. Using dwarf as metadata also has a few benefits over tweaking existing metadata to be extractable: it’s less
intrusive, and it’s also a more standardized metadata comparing to PGO’s own metadata.<u></u><u></u></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">While dwarf is a standard way of program annotation, using it for instrumentation PGO does mean an additional dependency (instead of being self contained).<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">This proposal requires debug_type info to be emitted, right? What is the object size and compile time overhead? If this can be trimmed, it is a reasonable way to emit
the profile data mapping information at compile time.<u></u><u></u></span></p>
</div>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">David<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-color:currentcolor currentcolor currentcolor rgb(204,204,204);border-style:none none none solid;border-width:medium medium medium 1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">Thanks,<u></u><u></u></p>
<p class="MsoNormal">Wenlei<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<div style="border-color:rgb(181,196,223) currentcolor currentcolor;border-style:solid none none;border-width:1pt medium medium;padding:3pt 0in 0in">
<p class="MsoNormal" style="margin-bottom:12pt"><b><span style="font-size:12pt;color:black">From:
</span></b><span style="font-size:12pt;color:black">llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>> on behalf of Xinliang David Li via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
<b>Date: </b>Monday, October 18, 2021 at 1:14 PM<br>
<b>To: </b>Ellis Hoag <<a href="mailto:ellis.sparky.hoag@gmail.com" target="_blank">ellis.sparky.hoag@gmail.com</a>><br>
<b>Cc: </b>llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
<b>Subject: </b>Re: [llvm-dev] [InstrProfiling] Lightweight Instrumentation</span><u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">Hi Ellis, thanks for the proposal. Improving the usability of Instrumentation PGO is indeed very important.</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">From the results data below, if I understand correctly, the main savings are from supporting the coverage mode
(using boolean counters), right? If we only enable that, the meta data based IRPGO clang size will be 10 MB larger (__llvm_prf_names are strippible or easily doable).</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">About __llvm_prof_data -- it also serves the purpose of detecting CFG mismatch (with cfg hashing). On the other
hand, about half of the size is used for value profiling purposes, so for coverage mode when value profile is not needed, its size can be cut in half -- leaving the total overhead to be roughly 7 MiB, very close to the debug info based matching scheme.</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">I support the proposal related to different profiling modes (entry only, boolean counter). I suggest having
those features upstreamed. In addition, changes that can reduce existing IRPGO size (e.g, strippable name section, reduced __llvm_prof_data) are also very welcome. After those are done, we will have a better idea if the size is still an issue (with a better
comparison with the debug info based method). </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">thanks,</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black">David</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:12pt;font-family:"Courier New";color:black"> </span><u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<blockquote style="border-color:currentcolor currentcolor currentcolor rgb(204,204,204);border-style:none none none solid;border-width:medium medium medium 1pt;padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div>
</blockquote></div>