[PATCH] Assembly/bitcode support for GenericDebugNode
Duncan P. N. Exon Smith
dexonsmith at apple.com
Wed Feb 4 13:26:52 PST 2015
> On 2015-Feb-02, at 16:48, Eric Christopher <echristo at gmail.com> wrote:
> 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
> 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.
Thanks for the review! Committed as r228029, r228030, r228031, r228041
and r228042 (with a clang fixup in the middle at r228032).
More information about the llvm-commits