probinson (Paul Robinson)
User

Projects

User does not belong to any projects.

User Details

User Since
May 9 2013, 11:10 AM (228 w, 1 d)

Recent Activity

Thu, Sep 21

probinson accepted D38088: Fix out-of-order stepping behavior in programs with hoisted constants..

Well then! LGTM.

Thu, Sep 21, 4:56 PM
probinson added a comment to D38088: Fix out-of-order stepping behavior in programs with hoisted constants..

This came out of our sample-PGO work, wasn't sure it would be kosher for me to approve a patch from the guy in the next cube.
It's completely in line with the other similar patches that Andrea and Dehao have been doing. So if Craig is happy, I'm happy.

Thu, Sep 21, 4:39 PM

Wed, Sep 20

probinson added inline comments to D37971: [dwarfdump] Add verbose output for .debug-line section.
Wed, Sep 20, 4:47 PM

Fri, Sep 15

probinson added a comment to D37157: Fix Bug 30978 by emitting cv file checksums..

A few minor commentary points; nothing huge. I've been looking at this mainly with the thought of having to do something similar for DWARF 5 eventually, but it seems likely there won't be a lot to leverage as CodeView and DWARF appear to associate the hashes with files pretty differently. Oh well.

Fri, Sep 15, 10:53 AM

Thu, Sep 14

probinson added inline comments to D37157: Fix Bug 30978 by emitting cv file checksums..
Thu, Sep 14, 4:58 PM
probinson added a comment to D37852: [dwarfdump] Make .eh_frame an alias for .debug_frame.

My impression is that .eh_frame and .debug_frame are interchangeable and in fact you get only one or the other in any given compilation. So, if you ask for .debug_frame and the object has .eh_frame, I think we should still dump it; and vice versa.

Thu, Sep 14, 1:33 PM

Wed, Sep 13

probinson added inline comments to D37771: llvm-dwarfdump: automatically dump both regular and .dwo variant of sections.
Wed, Sep 13, 2:52 PM
probinson added inline comments to D37771: llvm-dwarfdump: automatically dump both regular and .dwo variant of sections.
Wed, Sep 13, 2:36 PM
probinson added inline comments to D37771: llvm-dwarfdump: automatically dump both regular and .dwo variant of sections.
Wed, Sep 13, 11:55 AM
probinson added inline comments to D37771: llvm-dwarfdump: automatically dump both regular and .dwo variant of sections.
Wed, Sep 13, 11:44 AM
probinson added a comment to D37756: [lit] Force site configs to be run before source-tree configs.

If you use cmake to generate Visual Studio projects on Windows, do they use lit to run the tests? If so, do they run with this patch correctly? I have no opinion one way or the other but it's an environment nobody seems to have thought of before (including me).

I just tested running check-llvm from inside Visual Studio using a Visual Studio generated CMake project and it worked fine. I didn't run the rest of them, since if one works they all should.

Wed, Sep 13, 8:18 AM

Tue, Sep 12

probinson added a comment to D37771: llvm-dwarfdump: automatically dump both regular and .dwo variant of sections.

UI is tricky. I think I lean in the direction that if the section doesn't exist, don't print anything, even the "contents:" line.
This is certainly how I want --all to behave, and obviously it's easiest and self-consistent if that's how the tool always behaves. So for example --debug-types when there is no .debug_types section should print nothing.
I could see an argument that if I explicitly asked for a particular section and it doesn't exist, I should get the "contents:" line with nothing after it. But that's really not how I want --all to behave.

Tue, Sep 12, 3:38 PM
probinson added inline comments to D37768: [IR] Add llvm.dbg.addr, a control-dependent version of llvm.dbg.declare.
Tue, Sep 12, 3:26 PM
probinson added a comment to D37756: [lit] Force site configs to be run before source-tree configs.

If you use cmake to generate Visual Studio projects on Windows, do they use lit to run the tests? If so, do they run with this patch correctly? I have no opinion one way or the other but it's an environment nobody seems to have thought of before (including me).

Tue, Sep 12, 3:06 PM
probinson added inline comments to D37771: llvm-dwarfdump: automatically dump both regular and .dwo variant of sections.
Tue, Sep 12, 2:41 PM

Mon, Sep 11

probinson added a comment to D37714: llvm-dwarfdump: Replace -debug-dump=sect option with individual options..

What about supporting both options, but --debug-info also implies --debug-info-dwo?

Mon, Sep 11, 4:16 PM
probinson added a comment to D37714: llvm-dwarfdump: Replace -debug-dump=sect option with individual options..

I would kind of like one option to dump whichever of .debug_info or .debug_info.dwo (etc) there is, so I don't have to know up front which section to be asking for. Would also be more user-friendly to people who kind of know DWARF but not at that level of detail.

Mon, Sep 11, 4:07 PM
probinson accepted D37625: [DWARF] Incorrect prologue end line record..

I see that findPrologueEndLoc() is also checking the FrameSetup flag, so at least this makes the handling consistent, and relying on FrameSetup actually seems cleaner than relying on source locations. If you have time to investigate why the source locations seem wrong, that would be great.
LGTM after you fix the comment Adrian pointed out.

Mon, Sep 11, 10:51 AM

Fri, Sep 8

probinson added a comment to D37625: [DWARF] Incorrect prologue end line record..

I think the argument (and the test) would be stronger if it was based on the instruction sequence rather than offsets. For example if you generate asm and then show prologue_end is immediately before the call to get_arg.
Also we generally like to trim the IR source by removing unnecessary attribute sets.

Fri, Sep 8, 11:27 AM
probinson added a comment to D37604: Disable debuginfo-tests for non-native configurations.

This seems reasonable to me, thanks!
When you commit this, could you please double-check that the tests are still running on the green dragon builders? I'll also keep an eye on them.

Fri, Sep 8, 10:54 AM
probinson closed D37604: Disable debuginfo-tests for non-native configurations.

r312803. Forgot to put the tag in the commit message....

Fri, Sep 8, 10:13 AM
probinson committed rL312803: Restrict debuginfo-tests to native configurations..
Restrict debuginfo-tests to native configurations.
Fri, Sep 8, 10:12 AM

Thu, Sep 7

probinson added inline comments to D37602: Properly hook debuginfo-tests up to lit and CMake.
Thu, Sep 7, 5:08 PM
probinson added a comment to D37604: Disable debuginfo-tests for non-native configurations.

The new file goes in the debuginfo-tests directory. It's a separate project so that's probably not obvious from the diff.

Thu, Sep 7, 4:49 PM
probinson created D37604: Disable debuginfo-tests for non-native configurations.
Thu, Sep 7, 4:48 PM
probinson committed rL312751: [DWARF] Line 0 should not have a discriminator..
[DWARF] Line 0 should not have a discriminator.
Thu, Sep 7, 3:19 PM
probinson closed D37364: [DWARF] Line 0 should not have a discriminator by committing rL312751: [DWARF] Line 0 should not have a discriminator..
Thu, Sep 7, 3:19 PM
probinson added a comment to D37364: [DWARF] Line 0 should not have a discriminator.

Seems OK to me

For a minor perk - would it be possible to test the line table length? Since the benefit (smaller line table) of this change isn't readily apparent in the high level line table printing that dwarfdump does. I know it'd make the test a bit more brittle.

Thu, Sep 7, 3:04 PM
probinson updated the diff for D37364: [DWARF] Line 0 should not have a discriminator.

Seriously simplified test (derived from DebugInfo/X86/discriminator2.ll, as suggested by dblaikie).

Thu, Sep 7, 2:28 PM

Tue, Sep 5

probinson updated subscribers of D35532: [CMake] Update GetSVN.cmake to handle Repo.

The patch seems trivial enough but I do not have a good idea of the subtle differences in the various environments.
I have subscribed a couple of people who probably can tell whether this change makes sense.

Tue, Sep 5, 5:02 PM
probinson added a comment to D37364: [DWARF] Line 0 should not have a discriminator.

An MIR test that set up the situation more directly might be better, although I am not super confident in my ability to hand-reduce an MIR input.

Tue, Sep 5, 10:35 AM
probinson added a comment to D37364: [DWARF] Line 0 should not have a discriminator.

The change looks good to me. But the test is quite large and hard to understand. I don't think it's necessary to have the test generated from a source file, just forge a test with discriminator attached to line 0, and then checks if discriminators is omitted in the generated code. I guess it should suffice with < 20 LOC

Tue, Sep 5, 10:22 AM

Thu, Aug 31

probinson created D37364: [DWARF] Line 0 should not have a discriminator.
Thu, Aug 31, 5:46 PM
probinson added a comment to D37038: Replace temp MD nodes with unique/distinct before cloning.

I now think the tactic to pursue is making sure the containing DICompositeType(s) for the method have complete descriptions, and RAUW the temporary nodes, prior to calling CloneFunction. Actually wading around in the MDNodes, Types, and Decls to accomplish that result promises to be "interesting" and "educational" (advice/suggestions welcome).

Thu, Aug 31, 11:34 AM

Tue, Aug 29

probinson added a comment to D37038: Replace temp MD nodes with unique/distinct before cloning.

This may have gotten lost earlier: Would it be possible to instruct CloneFunction to not clone any temporary MDNodes via one of the flags that are passed to the ValueMapper?

Tue, Aug 29, 5:34 PM
probinson added a comment to D37038: Replace temp MD nodes with unique/distinct before cloning.

This may have gotten lost earlier: Would it be possible to instruct CloneFunction to not clone any temporary MDNodes via one of the flags that are passed to the ValueMapper?

Tue, Aug 29, 11:04 AM

Mon, Aug 28

probinson added a comment to D37038: Replace temp MD nodes with unique/distinct before cloning.

Dumping the whole module, the temporary DICompositeType node for CBdVfsImpl is used both as the scope node for the DISubprogram, and also as the baseType of a DIDerivedType which is a pointer type in the type list for the subprogram.

Mon, Aug 28, 5:43 PM
probinson added a comment to D37038: Replace temp MD nodes with unique/distinct before cloning.

finalizeSubprogram() retrieves the variables from the subprogram and handles them; what is it missing?

Mon, Aug 28, 4:56 PM
probinson added a comment to D37038: Replace temp MD nodes with unique/distinct before cloning.

Trying to understand the broader context here, I looked back through the list of revisions mentioned in PR33930 to see if that helped.

Mon, Aug 28, 3:50 PM

Fri, Aug 25

probinson added a comment to D36501: Add flag to request Clang is ABI-compatible with older versions of itself.

Sorry it took so long to wrap my head around this, but it has been about 20 years since I've had to accommodate an intentional ABI breakage, and that's actually what you want to do. Remembering that case, I have a model for this one, and I understand what you are doing now.

Fri, Aug 25, 5:44 PM
probinson added inline comments to D37038: Replace temp MD nodes with unique/distinct before cloning.
Fri, Aug 25, 4:59 PM
probinson added inline comments to D36501: Add flag to request Clang is ABI-compatible with older versions of itself.
Fri, Aug 25, 4:19 PM
probinson added a comment to D36993: [llvm-dwarfdump] Print type names in DW_AT_type DIEs.

I'm happy, but Adrian should have a chance to comment too.

Fri, Aug 25, 2:35 PM
probinson added a comment to D6379: Only warn about DWARF2 supporting one section per compilation unit for code sections.
In D6379#852974, @dim wrote:

Actually I see that we also get this warning on our .section ".note.GNU-stack","",@progbits directives, which are in most of our hand-written .S files. Since the type of that section is progbits, this fix won't work for it.

However, interestingly, clang also outputs these same .note.GNU-stack directives in .s output for compiled files, but it does *not* warn on those, even when using -gdwarf-2. So how is it avoiding them in that case?

Fri, Aug 25, 2:25 PM
probinson added inline comments to D36501: Add flag to request Clang is ABI-compatible with older versions of itself.
Fri, Aug 25, 2:18 PM
probinson added a comment to D6379: Only warn about DWARF2 supporting one section per compilation unit for code sections.

The DWARF2 restriction looks like it is related to non-contiguous code ranges, which were added in DWARF 3. So the warning should apply to code sections only. In that respect, functionally I think it's okay.
The diff needs to be refreshed as the surrounding code has changed in the interim, and it still needs a test.

Fri, Aug 25, 12:33 PM
probinson added a comment to D37123: [dwarfdump] Pretty print location expressions and location lists.

There are two copies of DWARFExpression.h?

Fri, Aug 25, 12:10 PM
probinson added a comment to D36501: Add flag to request Clang is ABI-compatible with older versions of itself.

I would prefer a diagnostic if PS4 && >3.2. Whether you map "latest" to 3.2 or diagnose that also, up to you, I'm okay either way.

Fri, Aug 25, 11:32 AM

Thu, Aug 24

probinson added a comment to D36501: Add flag to request Clang is ABI-compatible with older versions of itself.

Locally we have a couple different tactics for dealing with changes that we can't support. A more coherent approach would be great.
For example we defined a new TargetCXXABI::Kind value that is mostly GenericItaniumABI except where it isn't.
I personally did not do most of the various ABI-related tweaks so I don't claim to have a good handle on them, and we have been slow to get these things upstream; but I'd love to make that happen.

Thu, Aug 24, 12:24 PM
probinson added inline comments to D36993: [llvm-dwarfdump] Print type names in DW_AT_type DIEs.
Thu, Aug 24, 11:18 AM

Aug 22 2017

probinson added a comment to D37038: Replace temp MD nodes with unique/distinct before cloning.

I mentioned this in the PR but I should have restated it here, sorry...
Wolfgang authored this patch. He is away for a month so I volunteered to post this, in case it was okay for resolving the PR as the crash is present in the 5.0 branch.
I do know Wolfgang was not really happy with it, and your questions might well be part of why that is.
The patch has been in our local version of 5.0 for a couple of weeks without obvious problems, but that is not proof that it is always okay.

Aug 22 2017, 4:45 PM
probinson created D37038: Replace temp MD nodes with unique/distinct before cloning.
Aug 22 2017, 4:26 PM
probinson accepted D36993: [llvm-dwarfdump] Print type names in DW_AT_type DIEs.

LGTM

Aug 22 2017, 2:03 PM
probinson requested changes to D36993: [llvm-dwarfdump] Print type names in DW_AT_type DIEs.

We like to have the original source available for tests like this, ideally with commands to regenerate it. That practice helps people to understand the test and makes it easier to accommodate any syntactic changes that might come along (although that's more the case with IR tests than assembler tests).

Aug 22 2017, 10:09 AM

Aug 8 2017

probinson added a comment to D36411: Restore previous structure ABI for bitfields with 'packed' attribute for PS4 targets.

FTR, from a PS4 perspective this is all good, but we'd like somebody from outside our team to take a look.

Aug 8 2017, 6:50 AM

Aug 2 2017

probinson accepted D36114: [AsmPrinterDwarf] Add support for .cfi_restore directive.

I'm happy with the test changes and the code looks straightforward enough. LGTM.

Aug 2 2017, 7:38 AM

Aug 1 2017

probinson added inline comments to D35994: Debug info for variables whose type is shrinked to bool.
Aug 1 2017, 7:33 AM
probinson added inline comments to D36114: [AsmPrinterDwarf] Add support for .cfi_restore directive.
Aug 1 2017, 7:03 AM

Jul 27 2017

probinson added inline comments to D35953: [LiveDebugVariables] Use lexical scope to trim debug value live intervals.
Jul 27 2017, 3:59 PM

Jul 24 2017

probinson added a comment to D35734: Don't allow LLDB to try and parse .debug_types.

This section have been already removed from Dwarf5 so I agree that we shouldn't spend too much time adding support for it.

Jul 24 2017, 11:45 AM

Jul 20 2017

probinson created D35715: Preserve typedef names in debug info for template type parameters.
Jul 20 2017, 6:36 PM
probinson added a comment to D35698: [DWARF] Generalized verification of .debug_abbrev to be applicable to both .debug_abbrev and .debug_abbrev.dwo sections..

Thanks!

Jul 20 2017, 5:09 PM
probinson added a comment to D35643: [DWARF] Added check that verifies that no abbreviation declaration has more than one attribute with the same name..

I am pretty sure that any verification you want to do on .debug_abbrev would be equally applicable to .debug_abbrev.dwo; I suspect we could also verify that .debug_abbrev.dwo did not use certain forms, e.g. DW_FORM_addr, but that would be a bonus.

Jul 20 2017, 10:43 AM

Jul 19 2017

probinson added a comment to D35643: [DWARF] Added check that verifies that no abbreviation declaration has more than one attribute with the same name..

Wouldn't you also want to verify .debug_abbrev.dwo?

Jul 19 2017, 5:28 PM

Jul 18 2017

probinson added a comment to D35583: Debug Info: Add a file: field to DIImportedEntity.

Sweet. I don't have the chops to understand the Objective-C test but otherwise LGTM.

Jul 18 2017, 3:42 PM

Jul 14 2017

probinson added inline comments to D35437: Don't break bundles when adding DBG_VALUE.
Jul 14 2017, 2:36 PM

Jul 13 2017

probinson committed rL307964: [PS4] Disable LTO unit features under ThinLTO, like for Darwin..
[PS4] Disable LTO unit features under ThinLTO, like for Darwin.
Jul 13 2017, 2:26 PM

Jul 11 2017

probinson updated the diff for D14358: DWARF's forward decl of a template should have template parameters..

Refresh to current TOT, and ping. Funny what you can find in a year-old to-do list....

Jul 11 2017, 10:34 AM

Jul 10 2017

probinson added inline comments to D35166: [DWARF] Introduce verification for the unit header chain in .debug_info section to llvm-dwarfdump..
Jul 10 2017, 1:15 PM

Jul 6 2017

probinson added a comment to D34685: Mark a number of x86 only tests to require x86.
In D34685#796778, @ruiu wrote:

I'm still waiting for you to update this patch to address Rafael's comment.

I don't see any comment by Rafael. What are you referring to?

Jul 6 2017, 9:51 AM

Jun 30 2017

probinson added a comment to D34685: Mark a number of x86 only tests to require x86.
In D34685#795512, @ruiu wrote:

Yes, that's what I meant. It's not ideal, but if you want to write tests that don't depend on target, what target would you choose?

Jun 30 2017, 7:37 AM

Jun 29 2017

probinson added a comment to D34765: [DWARF] [NFC] Move a couple of member functions to DWARFUnit (baseclass) from DWARFCompileUnit (derived class).

No you're right, my bad. Units in the .dwo sections (both type and CU) don't have a str_offsets_base, which implies that the .debug_str_offsets.dwo section has to consist of a monolithic table of string offsets (without the 8 or 16-byte header that's specified in section 7.26 of the DWARF 5 standard). Section 7.26 seems to say the opposite, though. It seems I'll have to clarify this with the DWARF5 folks.

Worth clarifying on the dwarf-discuss list but I believe the idea is that the .dwo would have a single .debug_str_offsets.dwo contribution (complete with header), corresponding to the .debug_str.dwo contribution, and all units in the compilation would share it just like they would ordinarily share the single .debug_str section in a non-split compilation.

So then given the lack of a str_offsets_base attribute in the unit DIEs the string indices in the .dwo units would have to start with 2 instead of 0 in order to ignore the header. Or else the (absent) attribute would have an implicit value of 8 (or 16 for DWARF64). Seems a bit odd.

Jun 29 2017, 12:17 PM
probinson added a comment to D34765: [DWARF] [NFC] Move a couple of member functions to DWARFUnit (baseclass) from DWARFCompileUnit (derived class).

No you're right, my bad. Units in the .dwo sections (both type and CU) don't have a str_offsets_base, which implies that the .debug_str_offsets.dwo section has to consist of a monolithic table of string offsets (without the 8 or 16-byte header that's specified in section 7.26 of the DWARF 5 standard). Section 7.26 seems to say the opposite, though. It seems I'll have to clarify this with the DWARF5 folks.

Jun 29 2017, 11:48 AM
probinson added a comment to D34685: Mark a number of x86 only tests to require x86.
In D34685#795468, @ruiu wrote:
(Also, the fact that we are using "x86" to test generic feature means that you usually want to enable the LLVM x86 target to run these tests.)
Jun 29 2017, 10:46 AM
probinson committed rL306700: Tweak to match change in LLVM API, in r306699.
Tweak to match change in LLVM API, in r306699
Jun 29 2017, 9:53 AM
probinson committed rL306699: [DWARF] NFC: DWARFDataExtractor combines relocs with DataExtractor..
[DWARF] NFC: DWARFDataExtractor combines relocs with DataExtractor.
Jun 29 2017, 9:52 AM
probinson closed D34704: [DWARF] NFC: Combine relocs with DataExtractor by committing rL306699: [DWARF] NFC: DWARFDataExtractor combines relocs with DataExtractor..
Jun 29 2017, 9:52 AM
probinson added inline comments to D34704: [DWARF] NFC: Combine relocs with DataExtractor.
Jun 29 2017, 8:36 AM
probinson added a comment to D34704: [DWARF] NFC: Combine relocs with DataExtractor.

Thanks for taking time with this!

Jun 29 2017, 8:28 AM

Jun 28 2017

probinson updated the diff for D34704: [DWARF] NFC: Combine relocs with DataExtractor.

Removed RelocAddrMap members from DWARFDebugLine and DWARFDebugLoc.
DWARFDataExtractor ctors accept either a DWARFSection (can have relocations) or StringRef (no relocations).
Replace more DataExtractor parameters with DWARFDataExtractor.
Replace free function getRelocatedValue() with equivalent methods on DWARFDataExtractor.
DWARFUnit's LineSection changed from StringRef to DWARFSection, to make it harder to mess up the relocations to use with it.

Jun 28 2017, 5:03 PM
probinson added inline comments to D34704: [DWARF] NFC: Combine relocs with DataExtractor.
Jun 28 2017, 8:31 AM

Jun 27 2017

probinson created D34704: [DWARF] NFC: Combine relocs with DataExtractor.
Jun 27 2017, 1:27 PM
probinson committed rL306418: [DWARF] NFC: Make string-offset handling more like address-table handling; .
[DWARF] NFC: Make string-offset handling more like address-table handling;
Jun 27 2017, 8:40 AM
probinson updated subscribers of D34685: Mark a number of x86 only tests to require x86.

According to the top-level CODE_OWNERS.TXT the overall LLD code owner is @ruiu so please add him as a reviewer.
Also all LLD patches should have llvm-commits as a subscriber.
Note that when you add the list as a subscriber after-the-fact, you should repeat the original patch description as a comment, otherwise it won't appear on the mailing list.

Jun 27 2017, 7:32 AM

Jun 26 2017

probinson committed rL306324: [DWARF] NFC: Give DwarfFormat a 1-byte base type..
[DWARF] NFC: Give DwarfFormat a 1-byte base type.
Jun 26 2017, 12:52 PM
probinson committed rL306316: Tweak to match change in LLVM API, in r306315..
Tweak to match change in LLVM API, in r306315.
Jun 26 2017, 11:43 AM
probinson committed rL306315: [DWARF] NFC: Collect info used by DWARFFormValue into a helper..
[DWARF] NFC: Collect info used by DWARFFormValue into a helper.
Jun 26 2017, 11:43 AM
probinson closed D34570: [DWARF] NFC: Collect info needed by DWARFFormValue into a helper by committing rL306315: [DWARF] NFC: Collect info used by DWARFFormValue into a helper..
Jun 26 2017, 11:43 AM

Jun 23 2017

probinson updated subscribers of D34570: [DWARF] NFC: Collect info needed by DWARFFormValue into a helper.
Jun 23 2017, 2:34 PM
probinson added a comment to D34521: Remove /nologo from windows build.

I am mildly curious how much noise this adds to a build log.

Jun 23 2017, 2:07 PM
probinson added a comment to D34570: [DWARF] NFC: Collect info needed by DWARFFormValue into a helper.

As a follow-up, I was thinking that the DwarfFormat enum could be given a 1-byte base type (given that it has only two values), which would make DWARFFormParams only 4 bytes, which would make it reasonable to pass around by value instead of by reference.

Jun 23 2017, 1:40 PM
probinson added a comment to D33155: [DWARFv5] Support FORM_strp in the line table header.

See D34570, which packages the version/address-size/format and puts the query methods in one place. DWARFUnit and DWARFDebugLine use the new class as a data member, as suggested by @dblaikie. I'll update this patch again after that one's resolved.

Jun 23 2017, 1:35 PM
probinson created D34570: [DWARF] NFC: Collect info needed by DWARFFormValue into a helper.
Jun 23 2017, 1:33 PM

Jun 19 2017

probinson added a comment to D33155: [DWARFv5] Support FORM_strp in the line table header.

I'm not super excited about the hybrid form, but always unpacking a Unit inside the DWARFFormValue methods felt kind of expensive.

Jun 19 2017, 4:04 PM
probinson added a comment to D33155: [DWARFv5] Support FORM_strp in the line table header.

Ping.

Jun 19 2017, 11:57 AM

Jun 13 2017

probinson added inline comments to D34177: [DWARF] Partial verification for .apple_names accelerator table in llvm-dwarfdump output..
Jun 13 2017, 5:18 PM
probinson added a comment to D34177: [DWARF] Partial verification for .apple_names accelerator table in llvm-dwarfdump output..

A couple of minor comments inline. I'm okay with this, assuming Adrian is.

Jun 13 2017, 4:42 PM
probinson added a comment to D34177: [DWARF] Partial verification for .apple_names accelerator table in llvm-dwarfdump output..

I understand that DWARFAcceleratorTable already exists and you are building on that, but I have to wonder how much (if any) of this can be leveraged for the actual DWARF v5 standard's .debug_names section. (I haven't looked at any of this stuff yet; the accelerator table is pretty far down my list of v5 features to look at.) If the answer is "not much" then maybe it ought to be renamed DWARFAppleNamesTable instead.

The DWARF v5 accelerator table is a direct evolution of the Apple accelerator table, so it is quite possible that we can share some of the implementation. Form a cursory glance it looks like at least the accessors for the hash table (bucket count, ...) are similar enough that they can share a common interface. We probably can't make that call until someone actually sits down and comes up with a design for how to implement the DWARF 5 accelerator tables, though. How about we revisit this question then? Hopefully this won't be too far out into the future.

Jun 13 2017, 4:18 PM
probinson added a comment to D34177: [DWARF] Partial verification for .apple_names accelerator table in llvm-dwarfdump output..

Please subscribe 'llvm-commits' when you create a review, so the whole community can see the proposed patch.
(If you add llvm-commits later, we recommend that you paste the original description into a comment, because otherwise the mailing list never sees the original description.)

Jun 13 2017, 3:39 PM

Jun 12 2017

probinson updated the diff for D33155: [DWARFv5] Support FORM_strp in the line table header.

Provide DWARFDebugLine parsers with the DWARFContext, not just a relocation map.
Add a second overload of DWARFFormValue::extractValue() for use by the line-table parser; both overloads call a common private implementation.
This allows the v5 line-table header to use DW_FORM_strp.

Jun 12 2017, 11:45 AM