probinson (Paul Robinson)
User

Projects

User Details

User Since
May 9 2013, 11:10 AM (280 w, 5 d)

Recent Activity

Fri, Sep 21

probinson added a comment to D52296: [Clang] - Add -gsingle-file-split-dwarf option..

Do we generate the .dwo file directly these days? If not, I can imagine wanting to avoid the overhead of the objcopy hack; as long as the linker is smart enough not to bother with the .debug_*.dwo sections this seems like a build-time win.

Fri, Sep 21, 7:21 AM

Wed, Sep 19

probinson accepted D51908: [DebugInfo] Do not generate address info for removed labels..

LGTM

Wed, Sep 19, 11:41 AM

Tue, Sep 18

probinson added inline comments to D51908: [DebugInfo] Do not generate address info for removed labels..
Tue, Sep 18, 10:40 AM
probinson added inline comments to D52223: [dwarfdump] Verify DW_AT_type..
Tue, Sep 18, 10:34 AM · debug-info
probinson added inline comments to D51976: [DebugInfo][Dexter] Speculated BB presents illegal variable value to debugger.
Tue, Sep 18, 6:25 AM · debug-info

Mon, Sep 17

probinson added a comment to D51976: [DebugInfo][Dexter] Speculated BB presents illegal variable value to debugger.
In D51976#1237392, @vsk wrote:

I expect that extending dbg.value in this way will be fairly involved. There are a couple questions worth thinking about, imo:

  • Would the extra SSA values need to be constants (or live in registers)? If not, how will their locations be represented in a TAG_variable entry?
  • What's the expected memory size overhead of adding an extra operand to dbg.value? Does it need to be counterbalanced by introducing dbg.value.simple() [for the case where there is an empty DIExpression/extended SSA stack]?
Mon, Sep 17, 2:30 PM · debug-info
probinson added a comment to D51976: [DebugInfo][Dexter] Speculated BB presents illegal variable value to debugger.

Having read the real testcase now, I understand why you are dropping the debug info now. I would like to point out that we could do better. If we extended llvm.dbg.value to take more than one LLVM SSA Value, then we could produce a DIExpression that selects either %add or %sub depending on the value of %cmp. Let me know if you are interested in implementing this, it should be relatively straight forward to do and would come in useful in all sorts of other situations, too.

That is a very good idea and I am interested in implementing it. A good opportunity to improve my knowledge in that area. I can see the associated benefits.

Mon, Sep 17, 10:34 AM · debug-info

Fri, Sep 14

probinson added a comment to D51908: [DebugInfo] Do not generate address info for removed labels..

Perhaps we could implement this behavior (pr: i.e., omitting the label if it has been optimized away) only if we're tuning for GDB & otherwise produce a label without a low_pc? Not sure how other folks (@aprantl @echristo @probinson @JDevlieghere) feel about this - apply the workaround generally, because it's not super important to have a label for name-lookup porpoises? or is it worth carrying the two codepaths to ensure that slightly more accurate debug info where it's possible (presuming LLDB can cope with it)?

Fri, Sep 14, 3:36 AM

Thu, Sep 13

probinson added inline comments to D51935: [LLDB] - Improved DWARF5 support..
Thu, Sep 13, 4:12 AM

Wed, Sep 12

probinson added inline comments to D51976: [DebugInfo][Dexter] Speculated BB presents illegal variable value to debugger.
Wed, Sep 12, 9:08 AM · debug-info
probinson added a comment to D51908: [DebugInfo] Do not generate address info for removed labels..

Can you preserved the declaration DIE (at 0x0000007c) while omitting the definition DIE (at 0x00000063)? Does that still cause a problem for GDB?

Wed, Sep 12, 2:18 AM

Tue, Sep 4

probinson added inline comments to D51640: [DebugInfo] Normalize common kinds of DWARF sub-expressions..
Tue, Sep 4, 1:50 PM · debug-info
probinson added inline comments to D51640: [DebugInfo] Normalize common kinds of DWARF sub-expressions..
Tue, Sep 4, 12:29 PM · debug-info
probinson updated subscribers of D51340: Add /Zc:DllexportInlines option to clang-cl.

+cfe-commits

Tue, Sep 4, 8:12 AM

Tue, Aug 28

probinson added a comment to D51315: [debuginfo] generate debug info with asm+.file.

Awesome thanks!

Tue, Aug 28, 7:21 AM

Mon, Aug 27

probinson accepted D51315: [debuginfo] generate debug info with asm+.file.

The indentation in the test looks a little odd. Please verify there are no hard tabs.
Otherwise LGTM.

Mon, Aug 27, 12:55 PM

Aug 22 2018

probinson added inline comments to D51081: [DWARF v5] Refactoring range list dumping to fold DWARF v4 functionality into v5 handling (almost NFC)..
Aug 22 2018, 2:46 PM

Aug 10 2018

probinson added inline comments to D50525: [WIP] Move lldb-mi interpreter tests to LIT.
Aug 10 2018, 8:54 AM

Aug 8 2018

probinson committed rL339302: [DWARF] Verifier now handles .debug_types sections..
[DWARF] Verifier now handles .debug_types sections.
Aug 8 2018, 4:51 PM
probinson closed D50466: [DWARF] Verifier now handles .debug_types sections..
Aug 8 2018, 4:51 PM
probinson added inline comments to D50466: [DWARF] Verifier now handles .debug_types sections..
Aug 8 2018, 4:39 PM
probinson created D50466: [DWARF] Verifier now handles .debug_types sections..
Aug 8 2018, 10:48 AM

Aug 7 2018

probinson abandoned D50197: [DebugInfo/DWARF] Try to make a loop more readable. NFC.

I'm abandoning this minor tweak as I am convinced that there is something more fundamentally wrong here.
llvm-dwarfdump doesn't call the right API sequence to show the bug; it requires a DWP, looking up a specific unit by hash via the DWP index, and then scanning all the units. I can envision how to do a unittest for that, but the unittest infrastructure doesn't currently support building a DWP. I can't take the time right now to address this, but I'll keep it on The List.

Aug 7 2018, 10:47 AM · debug-info

Aug 6 2018

probinson added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 6 2018, 8:13 AM

Aug 3 2018

probinson added a comment to D50213: DebugInfo: Add metadata support for disabling DWARF pub sections.

How does this interact with v5 .debug_names?

Aug 3 2018, 9:04 AM
probinson added a comment to D50220: [DebugInfo] Refactor DbgInfoIntrinsic class hierarchy..

Two totally optional style nits. The new class distinction seems like an improvement.

Aug 3 2018, 8:52 AM
probinson committed rL338892: Release note for DWARF v5 support.
Release note for DWARF v5 support
Aug 3 2018, 7:05 AM
probinson committed rL338891: Release note for DWARF v5 support.
Release note for DWARF v5 support
Aug 3 2018, 7:05 AM

Aug 2 2018

probinson updated subscribers of D50197: [DebugInfo/DWARF] Try to make a loop more readable. NFC.
Aug 2 2018, 2:07 PM · debug-info
probinson created D50197: [DebugInfo/DWARF] Try to make a loop more readable. NFC.
Aug 2 2018, 12:38 PM · debug-info
probinson committed rL338759: [DebugInfo/DWARF] Remove redundant iterator type. NFC.
[DebugInfo/DWARF] Remove redundant iterator type. NFC
Aug 2 2018, 12:30 PM
probinson added inline comments to D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections..
Aug 2 2018, 8:14 AM
probinson added inline comments to D49964: [DWARF] Refactor DWARF classes to use unified error reporting. NFC..
Aug 2 2018, 8:04 AM · debug-info

Aug 1 2018

probinson committed rL338633: [DebugInfo/DWARF] [4/4] Unify handling of compile and type units. NFC.
[DebugInfo/DWARF] [4/4] Unify handling of compile and type units. NFC
Aug 1 2018, 1:54 PM
probinson closed D49744: [DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC.
Aug 1 2018, 1:54 PM · debug-info
probinson committed rL338632: [DebugInfo/DWARF] [3/4] Rename DWARFUnitSection to DWARFUnitVector. NFC.
[DebugInfo/DWARF] [3/4] Rename DWARFUnitSection to DWARFUnitVector. NFC
Aug 1 2018, 1:50 PM
probinson closed D49743: [DebugInfo/DWARF] [3/4] De-segregate type units and compile units. NFC.
Aug 1 2018, 1:50 PM · debug-info
probinson committed rL338629: [DebugInfo/DWARF] [2/4] Type units no longer in a std::deque. NFC.
[DebugInfo/DWARF] [2/4] Type units no longer in a std::deque. NFC
Aug 1 2018, 1:47 PM
probinson closed D49742: [DebugInfo/DWARF] [2/4] De-segregate type units and compile units. NFC.
Aug 1 2018, 1:47 PM · debug-info
probinson committed rL338628: [DebugInfo/DWARF] [1/4] De-templatize DWARFUnitSection. NFC.
[DebugInfo/DWARF] [1/4] De-templatize DWARFUnitSection. NFC
Aug 1 2018, 1:44 PM
probinson closed D49741: [DebugInfo/DWARF] [1/4] De-segregate type units and compile units. NFC.
Aug 1 2018, 1:44 PM · debug-info

Jul 31 2018

probinson added inline comments to D50027: Added initial unit test for LLDB's Stream class..
Jul 31 2018, 7:32 PM
probinson accepted D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs..

Using %t with suffixes instead of %T is the preferred idiom, glad you worked that out.
The more elaborate test depends on a set of Posix-y calls, but that's fine. If there are more environments that can't handle it, the bots will find them for you. :-)
LGTM with the fancier test, once you fix the missing RUN. (The RUN prefixes are how lit decides which lines are the test script.)

Jul 31 2018, 1:54 PM
probinson added a comment to D50055: Update the coding standard about NFC changes and whitespace.

I think you forgot to subscribe llvm-commits to this review?

Jul 31 2018, 1:29 PM
probinson added inline comments to D49744: [DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC.
Jul 31 2018, 10:43 AM · debug-info
probinson accepted D46850: [DebugInfo] Generate fixups as emitting DWARF .debug_line..

LGTM.

Jul 31 2018, 10:25 AM
probinson added inline comments to D49744: [DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC.
Jul 31 2018, 9:33 AM · debug-info
probinson updated the diff for D49744: [DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC.

Rebase. Add to and improve commentary.

Jul 31 2018, 9:31 AM · debug-info
probinson updated the diff for D49743: [DebugInfo/DWARF] [3/4] De-segregate type units and compile units. NFC.

Rebase.

Jul 31 2018, 9:30 AM · debug-info
probinson updated the diff for D49742: [DebugInfo/DWARF] [2/4] De-segregate type units and compile units. NFC.

Rebase. Renamed 'parse' methods to start with 'addUnits' to reflect their cumulative nature.

Jul 31 2018, 9:28 AM · debug-info
probinson added inline comments to D49741: [DebugInfo/DWARF] [1/4] De-segregate type units and compile units. NFC.
Jul 31 2018, 9:26 AM · debug-info
probinson updated the diff for D49741: [DebugInfo/DWARF] [1/4] De-segregate type units and compile units. NFC.

Rebase. Update some commentary.

Jul 31 2018, 9:25 AM · debug-info
probinson added inline comments to D50055: Update the coding standard about NFC changes and whitespace.
Jul 31 2018, 8:13 AM
probinson added inline comments to D49744: [DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC.
Jul 31 2018, 6:32 AM · debug-info
probinson added inline comments to D49742: [DebugInfo/DWARF] [2/4] De-segregate type units and compile units. NFC.
Jul 31 2018, 6:25 AM · debug-info

Jul 30 2018

probinson added inline comments to D49744: [DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC.
Jul 30 2018, 11:00 AM · debug-info
probinson added inline comments to D49742: [DebugInfo/DWARF] [2/4] De-segregate type units and compile units. NFC.
Jul 30 2018, 10:58 AM · debug-info
probinson added inline comments to D49741: [DebugInfo/DWARF] [1/4] De-segregate type units and compile units. NFC.
Jul 30 2018, 10:40 AM · debug-info
probinson added a comment to D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs..

Is it possible/reasonable for the test to call __llvm_profile_recursive_mkdir and then verify the mode of the created directory?
The test would have to be flagged as unsupported on Windows but that seems fine.

Jul 30 2018, 10:17 AM

Jul 27 2018

probinson added a comment to D49887: [DebugInfo] Add support for DWARF5 call site-related attributes.

As a partial and somewhat experimental feature, this should be controlled by a command-line option and off by default.
Even when it's complete it should probably still be under at least a tuning option, unless you can demonstrate the size cost is small.

Jul 27 2018, 10:17 AM · debug-info

Jul 26 2018

probinson accepted D46190: For a used declaration, mark any associated usings as referenced..

Although I am far from expert in how Clang manages declarations, AFAICT this does what @rsmith requested, so LGTM.

Jul 26 2018, 1:26 PM · Restricted Project
probinson accepted D49467: Fix inconsistency with/without debug information (-g).

Reassembled to reset the debug id's

Jul 26 2018, 10:43 AM
probinson added inline comments to D49676: [DWARF] support for .debug_addr (consumer).
Jul 26 2018, 7:07 AM · debug-info

Jul 25 2018

probinson added inline comments to D49676: [DWARF] support for .debug_addr (consumer).
Jul 25 2018, 7:35 AM · debug-info

Jul 24 2018

probinson created D49744: [DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC.
Jul 24 2018, 10:17 AM · debug-info
probinson created D49743: [DebugInfo/DWARF] [3/4] De-segregate type units and compile units. NFC.
Jul 24 2018, 10:16 AM · debug-info
probinson created D49742: [DebugInfo/DWARF] [2/4] De-segregate type units and compile units. NFC.
Jul 24 2018, 10:14 AM · debug-info
probinson created D49741: [DebugInfo/DWARF] [1/4] De-segregate type units and compile units. NFC.
Jul 24 2018, 10:12 AM · debug-info
probinson added inline comments to D49676: [DWARF] support for .debug_addr (consumer).
Jul 24 2018, 9:35 AM · debug-info
probinson added a comment to D49652: Apply -fdebug-prefix-map in reverse of command line order.

my general theory is that it's better to have tests that test everything at once if possible, but I guess it's unlikely enough that the debug info path will be correct in the IR and then not correct in the generated file and that that won't be caught by LLVM tests.

Jul 24 2018, 8:14 AM

Jul 23 2018

probinson added a comment to D49652: Apply -fdebug-prefix-map in reverse of command line order.

I was just going to run llvm-dwarfdump.

Jul 23 2018, 2:19 PM
probinson added a comment to D49652: Apply -fdebug-prefix-map in reverse of command line order.

Re. SmallVector versus std::vector, they are functionally similar but have different memory-allocation behaviors. SmallVector includes a vector of N elements (where N is the template parameter) so does no dynamic allocation until you have more than N elements; but it takes up that much more space as a member of a class. It's mainly intended for stack variables. Given that this particular option is used rarely, I would probably go with std::vector. The code change otherwise is pretty simple and looks fine.

Jul 23 2018, 1:21 PM
probinson updated subscribers of D49676: [DWARF] support for .debug_addr (consumer).

cc llvm-commits. Please do this when first creating the review.

Jul 23 2018, 10:58 AM · debug-info
probinson updated subscribers of D49670: dwarfgen: Add support for generating the debug_str_offsets section.

+ @clayborg who IIRC did the original generator

Jul 23 2018, 9:19 AM
probinson added a comment to D49652: Apply -fdebug-prefix-map in reverse of command line order.

Also, please document the option in clang/docs/UsersManual.rst. It should have been added when the option was first added, but certainly with right-to-left behavior it needs a mention. Nearly all options work left-to-right, so even though it's the same as gcc, not everybody comes to clang from gcc.

Jul 23 2018, 7:16 AM
probinson added a reviewer for D49652: Apply -fdebug-prefix-map in reverse of command line order: probinson.

Needs a test.

Jul 23 2018, 7:11 AM

Jul 20 2018

probinson accepted D48986: [FileCheck] Fix search ranges for DAG-NOT-DAG.

Sorry, lost track of this one. One comment suggestion and LGTM.

Jul 20 2018, 10:44 AM
probinson accepted D49594: [DebugInfo] Emit diagnostics when enabling -fdebug-types-section on non-linux target..

If the plan is for this to be relatively temporary then LGTM.

Jul 20 2018, 10:17 AM · debug-info
probinson added a comment to D49467: Fix inconsistency with/without debug information (-g).

It seems like the metadata could be reduced further. In particular, do you really need all those different types? The dbg.value needs only !11, I think you should be able to remove !12 through !18 (and references to them).

Jul 20 2018, 9:21 AM
probinson added a project to D49594: [DebugInfo] Emit diagnostics when enabling -fdebug-types-section on non-linux target.: debug-info.
Jul 20 2018, 8:34 AM · debug-info
probinson added a comment to D49594: [DebugInfo] Emit diagnostics when enabling -fdebug-types-section on non-linux target..

Is this because type units depend on COMDAT support? I had a vague idea that COFF also supports COMDAT.

Jul 20 2018, 8:33 AM · debug-info
probinson added a comment to D49467: Fix inconsistency with/without debug information (-g).

Please rename the new test to simply 'codegenprepare-and-debug.ll` (the convention of a date on the test name is obsolete, we don't do that for new tests anymore).

Jul 20 2018, 8:14 AM

Jul 19 2018

probinson added a comment to D49493: [DebugInfo] Reduce debug_str_offsets section size.

LGTM but I'll let @JDevlieghere have the last word. It looks like he still has one open question in DwarfDebug.cpp.

Jul 19 2018, 9:21 AM
probinson added a comment to D49493: [DebugInfo] Reduce debug_str_offsets section size.

I agree the ordering test looks fragile. The only other option that comes to mind is trying to exercise the APIs directly from a unittest, but then you need enough scaffolding around it to capture at least the assembler output and verify it. Probably best to leave the test as it is.

Jul 19 2018, 9:14 AM
probinson added a comment to D46190: For a used declaration, mark any associated usings as referenced..

@rsmith any further thoughts? We would really like to get this in before the LLVM 7.0 branch, currently scheduled for 1 August.

Jul 19 2018, 7:38 AM · Restricted Project

Jul 18 2018

probinson added a comment to D49493: [DebugInfo] Reduce debug_str_offsets section size.

A few typos.

Jul 18 2018, 10:56 AM
probinson added a comment to D46190: For a used declaration, mark any associated usings as referenced..

A bunch of style comments. Maybe try clang-format-diff.

Jul 18 2018, 10:05 AM · Restricted Project
probinson added a comment to D49426: [DEBUG_INFO] fix .loc directives emitted for missing prologues.

I wonder if we should experiment with making the line-table builder complain about a .loc that doesn't increment the address. That would tell us whether these zero-length cases show up anywhere else. Or, if we decide the assembler should handle it correctly, whether anything slips through.

Jul 18 2018, 9:20 AM
probinson added a comment to D49426: [DEBUG_INFO] fix .loc directives emitted for missing prologues.

However, this particular piece of work addresses what I perceived to be an artefact, a .loc directive for a missing prologue.

I'm not sure it's really a bug here, though - if the assembler addressed the sub-optimality of producing zero-length regions. It seems to me like a natural implementation to change the location, emit the prologue, then change the location again - and if the prologue happens to be empty/zero-length, the correct output falls out naturally. Avoiding special casing the empty prologue keeps the code simple/easier to read/understand, I think.

Jul 18 2018, 8:54 AM

Jul 17 2018

probinson added a comment to D49297: expose debugify as an LLVM option in clang.

I will try to look at this tomorrow if @vsk doesn't. Passes are not really my thing but as it's based on the existing opt code it should not be too hard to review.

Jul 17 2018, 3:26 PM
probinson added inline comments to D49214: [DWARF v5] emit DWARF v5 range lists (no support for fission yet).
Jul 17 2018, 12:19 PM
probinson added a comment to D49426: [DEBUG_INFO] fix .loc directives emitted for missing prologues.

The repeated address is syntactically valid per the rules for the line-number program, but semantically undefined per the rationale in the non-normative description of the table that the line-number program is supposed to be encoding (i.e., a table indexed by instruction address).

Jul 17 2018, 10:56 AM
probinson added inline comments to D49214: [DWARF v5] emit DWARF v5 range lists (no support for fission yet).
Jul 17 2018, 10:53 AM
probinson added inline comments to D49420: [DebugInfo] Generate .debug_names section when it makes sense.
Jul 17 2018, 10:38 AM
probinson added inline comments to D49214: [DWARF v5] emit DWARF v5 range lists (no support for fission yet).
Jul 17 2018, 10:25 AM
probinson added inline comments to D49420: [DebugInfo] Generate .debug_names section when it makes sense.
Jul 17 2018, 8:14 AM
probinson added a comment to D49420: [DebugInfo] Generate .debug_names section when it makes sense.

This is slightly complicated by the fact that the debug_names tables are
incompatible with the DWARF v4 type units (they assume that the type
units are in the debug_info section), and unfortunately, right now we
generate DWARF v4-style type units even for -gdwarf-5.

Jul 17 2018, 8:07 AM
probinson updated subscribers of D49426: [DEBUG_INFO] fix .loc directives emitted for missing prologues.

+llvm-commits.

Jul 17 2018, 8:00 AM
probinson updated subscribers of D49328: [FileCheck] Provide an option for FileCheck to dump original input to stderr on failure.

+llvm-commits

Jul 17 2018, 5:40 AM

Jul 16 2018

probinson added inline comments to D49214: [DWARF v5] emit DWARF v5 range lists (no support for fission yet).
Jul 16 2018, 12:37 PM