[PATCH] Assembly/bitcode support for GenericDebugNode

[Eric Christopher]

On Mon Feb 02 2015 at 2:21:03 PM Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015-Feb-02, at 13:51, Adrian Prantl <aprantl at apple.com> wrote:
> >
> >
> >> 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.
>
> That makes sense, but it doesn't affect `getTag()`, since that's
>

Agreed.


> unstringifying things.  And it doesn't seem to fit into `TagString()` too
> well either.  I think that logic should probably live in llvm-dwarfdump.
>
>
Don't agree :)

All of the internal debugging interfaces use TagString as well.

Mostly a side note though, LGTM.

-eric


> >
> > -- 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>
> >>
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150203/560c7f43/attachment.html>