<div dir="ltr"><div dir="ltr"><div>Hi David,</div><div><br></div><div>Thanks for the feedback! I'm happy to see interest in improving instrumentation size in upstream LLVM.<br></div><div><br></div><div>As for detecting CFG mismatches with <span style="font-family:monospace">__llvm_prf_data</span>, we can easily store the cfg hash in Dwarf using the <span style="font-family:monospace">DW_TAG_LLVM_annotation</span> attribute which can store any link-time constant. This value would be included final <span style="font-family:monospace">default.profdata</span> profile without any format change, so the compile would consume it in the same way.</div><div><br></div><div><font color="#888888">Ellis</font><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 18, 2021 at 12:30 PM Xinliang David Li <<a href="mailto:davidxl@google.com">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 style="font-family:monospace;font-size:small;color:rgb(0,0,0)">Hi Ellis, thanks for the proposal. Improving the usability of Instrumentation PGO is indeed very important.</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)">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).</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)">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.</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)">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).  </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)">thanks,</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></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
</blockquote></div>
</blockquote></div></div>