<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></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 Feb 2, 2015, at 1:51 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com" class="">aprantl@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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=""><br class="Apple-interchange-newline">On Feb 2, 2015, at 1:34 PM, Frédéric Riss <<a href="mailto:friss@apple.com" class="">friss@apple.com</a>> wrote:<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Feb 2, 2015, at 12:55 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class=""><br class="">Add assembly/bitcode support to `GenericDebugNode`. There are two<br class="">things I need reviewed here (although other comments are obviously<br class="">welcome):<br class=""><br class="">- The first three patches add a `dwarf::getTag()` to complement<br class=""> `dwarf::TagString()`. There's a small functionality change in the<br class=""> first patch I want confirmation on: `dwarf::TagString()` will return<br class=""> `nullptr` for `DW_TAG_lo_user` (and related non-tags). This<br class=""> constant is not really a DWARF tag; it's just a marker for the first<br class=""> recommended user-defined tag). Stringifying it seems wrong (and<br class=""> would conflict with stringifying a user-defined tag of the same<br class=""> number).<br class=""></blockquote><br class="">The 3 patches SGTM, a couple of questions/remarks though:<br class=""><br class="">I agree that stringifying lo_user and friends makes no real sense as we<br class="">are using the TagString function to pretty print actual dwarf data. Not that<br class="">it matters today as we have no such extension registered.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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=""><br class="">Why would you want to ever accept them as input? I really don’t see a<br class="">reason, but given you’ve introduced a special case for them, you really<br class="">must need them.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">Derived tools like llvm-dwarfdump should be able to handle DWARF generated by other compilers and should probably print something like DW_TAG_user(0x1234) when it encounters an unknown tag.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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></blockquote><div><br class=""></div><div>Yes, and that’s what we do. I was talking about accepting the textual input ‘DW_TAG_lo_user’ as a valid assembly token.</div><div><br class=""></div><div>Fred</div><br class=""><blockquote type="cite" class=""><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">-- adrian</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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=""><br class="">Is there a way to cleanly generate the enum in the Dwarf.h file from the<br class="">.def file also? This way there would be only one source of tag definitions.<br class="">(This would me having Dwarf.def as an exported header, which is quite<br class="">ugly). At least add a comment to the header that anything added to the<br class="">enum should be added to the def file.<br class=""><br class="">Fred<br class=""><br class=""><blockquote type="cite" class="">- The fourth patch adds assembly/bitcode support, and the fifth<br class=""> improves it by using `dwarf::getTag()`. Here's the resulting<br class=""> syntax:<br class=""><br class=""> !0 = !GenericDebugNode(tag: DW_TAG_entry_point,<br class=""> header: "some\00header",<br class=""> operands: {!1, !2, !7})<br class=""><br class=""> This shouldn't be surprising (or a departure from the plan of<br class=""> record), but officially I need every assembly change reviewed<br class=""> pre-commit.<br class=""><br class="">Note that this commit doesn't actually move `GenericDebugNode` into<br class="">place (and that's not the next step; more below for those interested).<br class=""><br class="">(An extended side-note:<br class=""><br class="">I've been sitting on these patches for a while, kind of stumped on the<br class="">best way to stage the rest of the work here.<br class=""><br class="">Originally, I'd planned to move `GenericDebugNode` in underneath the<br class="">`DIDescriptor` classes right after this patch series. This would have<br class="">required updating all the testcases to use it, only to subsequently<br class="">update every line *again* as I implemented the more specialized nodes<br class="">(making schema changes along the way).<br class=""><br class="">Instead I'll be proceeding as follows: add specialized nodes that match<br class="">the current schema, add assembly/bitcode support for them (I have<br class="">out-of-tree patches that get to here), move them all underneath the<br class="">`DIDescriptor` hierarchy (only requiring each line to be updated<br class="">*once*), and then start hacking the schema once they're in place.<br class=""><br class="">The new approach minimizes testcase churn, and (perhaps more<br class="">importantly) better separates the infrastructure changes (which are<br class="">rather mechanical) from the schema changes (which need more careful<br class="">consideration).)<br class=""><br class=""><0001-Support-Stop-stringifying-DW_TAG_-lo-hi-_user.patch><0002-Support-Re-implement-dwarf-TagString-using-a-.def-fi.patch><0003-Support-Add-string-unsigned-mapping-for-DW_TAG.patch><0004-IR-Assembly-and-bitcode-for-GenericDebugNode.patch><0005-AsmParser-Recognize-DW_TAG_-constants.patch></blockquote></blockquote></div></blockquote></div><br class=""></body></html>