<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 3, 2020 at 6:47 AM Sourabh Singh Tomar <<a href="mailto:sourav0311@gmail.com" target="_blank">sourav0311@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">Hi David,<div><br></div><div>Thanks for sharing your thoughts/suggestions on this.<br><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:13px">4 forms that share the same encoding in DWARFv4 - you mean the define, define_strp, define_strx, and define_sup?</span> </blockquote><div>These are:</div><div> DW_MACINFO_define = DW_MACRO_define</div><div> DW_MACINFO_undef = DW_MACRO_undef</div><div> DW_MACINFO_start_file = DW_MACRO_start_file</div><div> DW_MACINFO_end_file = DW_MACRO_end_file</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:13px">I think that the solution here is probably not to change the IR representation - IR doesn't have the semantics that necessitate strip/strx/sup encodings (there's no indexed string section at the IR level - string sharing is a lower level implementation detail at the IR Level that's not apparent at the IR Level). These are choices the backend would make about how to efficiently encode the strings into the final output.</span></blockquote><div>Agreed. IR doesn't have the semantics that necessitate strp/strx/sup encodings, and don't have too. However considering the most productive macro-string representation form /DW_FORM_define_strx/ and taking the existing infrastructure, emission psuedo code will be :</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> if (Version == 4 && getMacinfoType() == DW_MACINFO_define)<br>emit DW_MACINFO_define<br> if (Version == 5 && getMacinfoType() == DW_MACINFO_define) <br>emit DW_MACRO_define_strx</blockquote><div> Now here the presence of "DW_MACINFO_define" here seems unfair/odd, may confuse the user/developer.</div><div><br></div><div>With new types, we can have much cleaner code:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">if (getGenericMacroType() == MACRO_definition) /* IR types doesn't show/share any false dependence on how back-end DWARF or other will consume this*/<br>if (Version == 5) emit "DW_MACRO_define_strx"<br>else emit "DW_MACRO_define"</blockquote></div></div></blockquote><div><br>I don't think that adding a non-DWARF name can make the code cleaner than reusing the DWARF name for both v4 and v5.<br><br>Either you have to translate from generic to v4 and generic to v5, or you have to translate from v4 to v5, and do nothing for v4 to v4. It seems strictly simpler (you could for symmetry always do a v4 to v4 translation too, but I doubt that'd be necessary).<br><br>I'd probably leave the existing code mostly as-is with just a couple of 'if' statements:<br><br> bool UseMacinfo = getDwarfVersion() >= 5;</div><div> unsigned MacInfoType = M.getMacinfoType();<br> assert(MacinfoType == DW_MACINFO_define || MacinfoType == DW_MACINFO_undef);<br> // alternatively this could use a map lookup or other mechanism for mapping from DWARFv4 to DWARFv5 encodings<br> Asm->EmitULEB128(MacinfoType + (UseMacInfo ? (DW_MACRO_define_strx - 1) : 0);<br> Asm->EmitULEB128(M.getLine());<br> StringRef Name = M.getName();<br> StringRef Value = M.getValue();<br> if (UseMacinfo) {<br> DwarfFile &Holder = useSplitDwarf() ? SkeletonHolder : InfoHolder;<br> Asm->EmitULEB128(DG.getStringPool().getEntry(*DG.getAsmPrinter(), (Name + ' ' + Value).str()));<br> return;<br> }<br> Asm->OutStreamer->EmitBytes(Name);<br> if (!Value.empty()) {<br> // There should be one space between macro name and macro value.<br> Asm->emitInt8(' ');<br> Asm->OutStreamer->EmitBytes(Value);<br> }<br> Asm->emitInt8('\0');<br><br><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><div><br></div><div>Even at the IR level, user will have confusion. When in .ll file we have "DW_MACINFO_define" and subsequent binary contains "DW_MACRO_define_strx". This again, is due to the fact that "DW_MACINFO_define" is defined in the DWARF spec, resulting in a false dependence/representation. At least visible to user familiar with DWARF.</div><div><br></div><div>Yet another motivating example: Exactly same thing happens with "DW_MACINFO_start_file". We'll have "DW_MACINFO_start_file" in .ll file and subsequent binary will contain "DW_MACRO_start_file". Consider this snippet from llvm:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> auto *MF = DIMacroFile::get(VMContext, dwarf::DW_MACINFO_start_file, TMF->getLine(), TMF->getFile(), getOrCreateMacroArray(I.second.getArrayRef()));<br> replaceTemporary(llvm::TempDIMacroNode(TMF), MF);</blockquote><div> </div><div>Now based on this, emission psuedo code will be:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> if (Version == 4 && getMacinfoType() == DW_MACINFO_start_file)<br>emit "DW_MACINFO_start_file"<br> if (Version == 5 && getMacinfoType() == DW_MACINFO_start_file) <br>emit "DW_MACRO_start_file"</blockquote><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:13px">All this is to say that I agree with you about the semantics of your proposal, but I think the /syntax/ of the IR can remain the same, just with the semantics you're describing. (ie: that DW_MACINFO_define in the IR doesn't necessarily mean you'll get a DW_MACINFO_define in the output (you might get a DW_MACINFO_define, maybe a DW_MACINFO_define_strx, etc, etc... )</span> </blockquote><div><br></div><div>To summarize: Adding new Generic macro type to metadata IR, and removing old ones seems a clean and promising solution. This also enforces/depicts that IR metadata is not dependent on the target consumer DWARF or other.</div><div><br></div><div>Thanks, </div><div>Sourabh.</div><div> </div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 3, 2020 at 4:01 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@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"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 2, 2020 at 6:03 AM Sourabh Singh Tomar <<a href="mailto:sourav0311@gmail.com" target="_blank">sourav0311@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"><div class="gmail_quote"><div dir="ltr" class="gmail_attr"><span style="font-family:Arial,sans-serif;font-size:10pt">Hello
Everyone,</span><br></div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><p class="MsoNormal" style="margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt"></span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif"> </span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">I would like to have your thoughts on this.</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif"> </span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">Overview of the problem</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">===================</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">While implementing support for the DWARFv5 debug_macro section
support in LLVM. I came across these holes:</span></p>
<ul><li><span style="font-family:Arial,sans-serif;font-size:10pt"> The
macros infrastructure in CLANG/LLVM is inherently tied to a
particular version(v4 macinfo). For instance, consider this snippet
from CLANG:</span></li></ul></div></div></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div dir="ltr"><div dir="ltr"><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-family:Calibri,sans-serif;font-size:11pt">Gen->getCGDebugInfo()->CreateMacro(getCurrentScope(), llvm::dwarf::DW_MACINFO_define,
location, Name.str(), Value.str()).</span></blockquote></div></div></div></blockquote></blockquote><div dir="ltr"><div dir="ltr"><div dir="ltr"><ul><ul><li>So,
let's say you want to add support for a new form /DW_MACRO_define_strx/,
with existing infrastructure, you can't do this without polluting code
with lot of if-else. This is just to highlight the problem, there are
numerous areas where we will have to add if-else.<br></li></ul><li> DWARF
forms[DW_MACINFO*] are unnecessarily hard-coded in CLANG, resulting in a
false dependence of CLANG/LLVM on entire generation phase of debug info
for macros, even when they don't have to.</li><li> To
worsen the problem even more, we have some DWARF forms sharing same
encoding. Currently, we have 4-forms that share same encoding with
previous DWARFv4.</li></ul></div></div></div></div></div></div></blockquote><div><br>4 forms that share the same encoding in DWARFv4 - you mean the define, define_strp, define_strx, and define_sup? <br><br>I think that the solution here is probably not to change the IR representation - IR doesn't have the semantics that necessitate strip/strx/sup encodings (there's no indexed string section at the IR level - string sharing is a lower level implementation detail at the IR Level that's not apparent at the IR Level). These are choices the backend would make about how to efficiently encode the strings into the final output.<br><br>All this is to say that I agree with you about the semantics of your proposal, but I think the /syntax/ of the IR can remain the same, just with the semantics you're describing. (ie: that DW_MACINFO_define in the IR doesn't necessarily mean you'll get a DW_MACINFO_define in the output (you might get a DW_MACINFO_define, maybe a DW_MACINFO_define_strx, etc, etc... )<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 class="gmail_quote"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><ol start="1" type="1" style="margin-bottom:0in">
</ol>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif"> </span><span style="font-family:Arial,sans-serif;font-size:10pt">High Level Summary of Solution</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">========================</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">To deal with this and to facilitate addition of new future DWARF
forms and other stuff, I propose following changes to be done:</span></p>
<p class="MsoNormal" style="margin:0in 0in 0.0001pt 47.25pt;background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Symbol">·<span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:7pt;line-height:normal;font-family:"Times New Roman"">
</span></span><span style="font-size:10pt;font-family:Arial,sans-serif"> Segregate CLANG
entirely from macro debug info part. This has been done by introducing a new
Generic Macro Types. Since macro information can be broadly classified
into 4 categories:</span><br></p>
<p class="MsoNormal" style="margin:0in 0in 0.0001pt 83.25pt;background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">1.<span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:7pt;line-height:normal;font-family:"Times New Roman"">
</span></span><span style="font-size:10pt;font-family:Arial,sans-serif">Definition</span></p>
<p class="MsoNormal" style="margin:0in 0in 0.0001pt 83.25pt;background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">2.<span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:7pt;line-height:normal;font-family:"Times New Roman"">
</span></span><span style="font-size:10pt;font-family:Arial,sans-serif">Un-definition</span></p>
<p class="MsoNormal" style="margin:0in 0in 0.0001pt 83.25pt;background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">3.<span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:7pt;line-height:normal;font-family:"Times New Roman"">
</span></span><span style="font-size:10pt;font-family:Arial,sans-serif">File inclusion</span></p>
<p class="MsoNormal" style="margin:0in 0in 0.0001pt 83.25pt;background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">4.<span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:7pt;line-height:normal;font-family:"Times New Roman"">
</span></span><span style="font-size:10pt;font-family:Arial,sans-serif">File termination</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">So I propose to introduce 4 new Generic Macro Types to be
used as types, with no dependence to various DWARFv5/v4 forms. </span></p><p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"></p><ol><li><span style="font-size:10pt;font-family:Arial,sans-serif">MACRO_definition</span></li><li><span style="font-size:10pt;font-family:Arial,sans-serif">MACRO_undef</span></li><li><span style="font-size:10pt;font-family:Arial,sans-serif">MACRO_start_file</span></li><li><span style="font-size:10pt;font-family:Arial,sans-serif">MACRO_end_file</span></li></ol><p></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><br></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">- Previous infrastructure uses DWARF forms as types in metadata
--</span></p>
<blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote"><span style="font-size:10pt;font-family:Arial,sans-serif">!DIMacro(type: DW_MACINFO_define, line: 9,
name: "Name", value: "Value")</span></blockquote>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">And the production part will query this and emits the form
present[hardcoded]. </span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif"> </span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">Upgraded infrastructure will be using something like --</span></p>
<blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote"><span style="font-size:10pt;font-family:Arial,sans-serif">!DIMacro(type: MACRO_definition, line: 9,
name: "Name", value: "Value") </span></blockquote>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">Now production part will query the Generic Macro Type present in
metadata and based on the version of DWARF specified, it will emit forms
accordingly.</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif"> </span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">- Now this should cater to all our needs of LLVM and debug-info
production-consumption WRT to macro. </span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif"> </span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">This approach solves the problem highlighted above and
simplifies the code, increases code/infrastructure reuse for building DWARFv5 macro and future additons. </span></p><p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif"><br></span></p><p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">I have a patch[implementing this] ready, currently supporting DWARFv4 macro only. Planning to build rest of DWARFv5 on top of this. Could you guys let me know your thoughts and how to proceed forward with this. Should we need an RFC for it have a better discussion?</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif"> </span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">Thanks in anticipation,</span></p>
<p class="MsoNormal" style="background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial;margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt;font-family:Arial,sans-serif">Sourabh.</span></p>
<p class="MsoNormal" style="margin:0in 0in 0.0001pt;font-size:11pt;font-family:Calibri,sans-serif"><span style="font-size:10pt"> </span></p></div></div></div></div>
</div></div>
</blockquote></div></div>
</blockquote></div></div></div>
</blockquote></div></div>