[PATCH] Assembly/bitcode support for GenericDebugNode

[Adrian Prantl]

> On Feb 2, 2015, at 1:34 PM, Frédéric Riss <friss at apple.com> wrote:
> 
> 
>> On Feb 2, 2015, at 12:55 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> Add assembly/bitcode support to `GenericDebugNode`.  There are two
>> things I need reviewed here (although other comments are obviously
>> welcome):
>> 
>> - The first three patches add a `dwarf::getTag()` to complement
>>   `dwarf::TagString()`.  There's a small functionality change in the
>>   first patch I want confirmation on: `dwarf::TagString()` will return
>>   `nullptr` for `DW_TAG_lo_user` (and related non-tags).  This
>>   constant is not really a DWARF tag; it's just a marker for the first
>>   recommended user-defined tag).  Stringifying it seems wrong (and
>>   would conflict with stringifying a user-defined tag of the same
>>   number).
> 
> The 3 patches SGTM, a couple of questions/remarks though:
> 
> I agree that stringifying lo_user and friends makes no real sense as we
> are using the TagString function to pretty print actual dwarf data. Not that
> it matters today as we have no such extension registered.

> 
> Why would you want to ever accept them as input? I really don’t see a
> reason, but given you’ve introduced a special case for them, you really
> must need them.

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.

-- adrian
> 
> Is there a way to cleanly generate the enum in the Dwarf.h file from the
> .def file also? This way there would be only one source of tag definitions.
> (This would me having Dwarf.def as an exported header, which is quite
> ugly). At least add a comment to the header that anything added to the
> enum should be added to the def file.
> 
> Fred
> 
>> - The fourth patch adds assembly/bitcode support, and the fifth
>>   improves it by using `dwarf::getTag()`.  Here's the resulting
>>   syntax:
>> 
>>       !0 = !GenericDebugNode(tag: DW_TAG_entry_point,
>>                              header: "some\00header",
>>                              operands: {!1, !2, !7})
>> 
>>   This shouldn't be surprising (or a departure from the plan of
>>   record), but officially I need every assembly change reviewed
>>   pre-commit.
>> 
>> Note that this commit doesn't actually move `GenericDebugNode` into
>> place (and that's not the next step; more below for those interested).
>> 
>> (An extended side-note:
>> 
>> I've been sitting on these patches for a while, kind of stumped on the
>> best way to stage the rest of the work here.
>> 
>> Originally, I'd planned to move `GenericDebugNode` in underneath the
>> `DIDescriptor` classes right after this patch series.  This would have
>> required updating all the testcases to use it, only to subsequently
>> update every line *again* as I implemented the more specialized nodes
>> (making schema changes along the way).
>> 
>> Instead I'll be proceeding as follows: add specialized nodes that match
>> the current schema, add assembly/bitcode support for them (I have
>> out-of-tree patches that get to here), move them all underneath the
>> `DIDescriptor` hierarchy (only requiring each line to be updated
>> *once*), and then start hacking the schema once they're in place.
>> 
>> The new approach minimizes testcase churn, and (perhaps more
>> importantly) better separates the infrastructure changes (which are
>> rather mechanical) from the schema changes (which need more careful
>> consideration).)
>> 
>> <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>
>