[PATCH] Assembly/bitcode support for GenericDebugNode

[Frédéric Riss]

> 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.

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>