Page MenuHomePhabricator

debug-infoProject
ActivePublic

Recent Activity

Today

avl added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

I think OffsetToFirstDefinition should not be taken into account at all.
i.e., coverage should be calculated against block scope, not against variable scope(which is unknown).

Fri, Oct 18, 3:04 AM · debug-info, Restricted Project

Yesterday

SouraVX added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

Hi @probinson @dblaikie @aprantl , I've was investigating and working on your inputs regarding the problem with DW_at_defaulted once. I think clang has also some issues. Though I'm not able to precisely point out. I ranned into some problems in CFE while marking out_of_class functions. i.e consider this for instance "debug-info-defaulted-out_of_class.cpp" FIXME:. Causing too much trouble and possibly can introduce some bugs in clang/llvm.
May be we'll reconsider this in future. Thanks for putting time in reviewing and discussing this.

Thu, Oct 17, 10:56 PM · debug-info, Restricted Project, Restricted Project
aprantl added inline comments to D67556: [ARM][AArch64][DebugInfo] Improve call site instruction interpretation.
Thu, Oct 17, 4:40 PM · debug-info
vsk accepted D69109: [DebugInfo] Stop describing imms in TargetInstrInfo's describeLoadedValue() impl.

LGTM! Handling each instruction separately is indeed more safer! Too bad that we can't relay much on MI generic flags such as this one.

Thu, Oct 17, 2:09 PM · Restricted Project, debug-info
dblaikie added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

I'm confused - given the presence of noncontiguous ranges, why would the "first def" be an important property?

The existence of this operation is "iffy" to me - I'd expect a general solution that handles all the noncontiguous range chunks equally, so there wouldn't be a special case for the "first def".

Currently, we calculate the variable's coverage this way (if I understand the things correctly):

  1. Calculate the size of the whole scope - subprogram, lexical block, etc (either as a distance between scope's HighPC or LowPC or a sum of sizes of scope's ranges).
  2. Calculate the number of bytes where the variable is defined (by its location list).
  3. Calculate and substruct OffsetToFirstDefinition from the size of the scope.
  4. Divide the number of bytes where a variable is defined by the adjusted size of the scope. OffsetToFirstDefinition is applied in the case the variable first definition address greater than scope's LowPC. I guess this is done to handle cases where the variable is defined not in the beginning of the scope (e.g. in the middle of a function).
Thu, Oct 17, 1:22 PM · debug-info, Restricted Project
NikolaPrica updated the diff for D67556: [ARM][AArch64][DebugInfo] Improve call site instruction interpretation.

-Rebase
-Move register identity transformation check to DwarfDebug::collectCallSiteParameters()

Thu, Oct 17, 7:39 AM · debug-info
NikolaPrica added a comment to D69109: [DebugInfo] Stop describing imms in TargetInstrInfo's describeLoadedValue() impl.

LGTM! Handling each instruction separately is indeed more safer! Too bad that we can't relay much on MI generic flags such as this one.

Thu, Oct 17, 7:30 AM · Restricted Project, debug-info
dstenb created D69109: [DebugInfo] Stop describing imms in TargetInstrInfo's describeLoadedValue() impl.
Thu, Oct 17, 6:52 AM · Restricted Project, debug-info
djtodoro added a comment to D68209: [LiveDebugValues] Introduce entry values of unmodified params.

Hi Jeremy,

Thu, Oct 17, 12:39 AM · Restricted Project, debug-info

Wed, Oct 16

krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

I'm confused - given the presence of noncontiguous ranges, why would the "first def" be an important property?

The existence of this operation is "iffy" to me - I'd expect a general solution that handles all the noncontiguous range chunks equally, so there wouldn't be a special case for the "first def".

Wed, Oct 16, 4:23 PM · debug-info, Restricted Project
dstenb added inline comments to D68209: [LiveDebugValues] Introduce entry values of unmodified params.
Wed, Oct 16, 3:54 PM · Restricted Project, debug-info
probinson added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

(@dblaikie Aside regarding noreturn, the original DWARF proposal was for a debugger wanting to know a given function will not return.

I'd still be curious to know which consumer, if they're still interested, what feature they're trying to build with it, if they're using Clang/LLVM's output, etc.

Wed, Oct 16, 2:48 PM · debug-info, Restricted Project, Restricted Project
dblaikie added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

I think the existing feature's perhaps a bit misspecified - because where you put the DEFAULTED_{in_class,out_of_class} would communicate that without needing the two values - if you put it on an out of line function definition, it's defaulted out of line, if you put it on the in-class declaration then it's defaulted inline.

But spec'd the way it is, if we want to implement this at all/if there's some actual user need (admittedly the noreturn attribute has gone in recently without a discussion of usage... so I'm not the only gatekeeper here & other people have other opinions, and that's OK - if someone (@probinson @aprantl etc) want to tell me I'm being unreasonable here & we should just put it in - it's only a bit here or there & not likely to make a huge difference to DWARF size & possibly enable some scenarios we haven't imagined yet and avoid the chicken-and-egg problem for the future implementer of such features, that's OK) - I'd essentially implement it that way. Put DEFAULTED_out_of_class on the member function definition DIE if it's defaulted there, and put DEFAULTED_in_class on the declaration DIE if it's defaulted there.

And I'd probably only spend one bit of flags on this, if possible - "not defaulted (0/assumed for all subprograms) or defaulted (1)" and translate it into one of the two values (in_class or out_of_class) in the backend based on which subprogram DIE it's attached to.

That seems reasonable too. Of course if we're already spending a bit on Deleted, and the same subprogram can't be both Deleted and Defaulted, it costs no extra DISP bits to represent the 4 cases (defaulted-in-class, defaulted-out-of-class, deleted, none-of-the-above).

Wed, Oct 16, 12:40 PM · debug-info, Restricted Project, Restricted Project
dblaikie added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

I'm confused - given the presence of noncontiguous ranges, why would the "first def" be an important property?

Wed, Oct 16, 12:31 PM · debug-info, Restricted Project
aprantl closed D68697: [DWARF5] Added support for DW_AT_noreturn attribute to be emitted for C++ class member functions..
Wed, Oct 16, 9:32 AM · Restricted Project, Restricted Project, debug-info
jmorse added a comment to D68209: [LiveDebugValues] Introduce entry values of unmodified params.

Djordje wrote:

Nevertheless, there are many cases where (due to various optimizations) a parameter value is only moved (copied) around, from one register to another one. We should recognize such situation and keep tracking of entry value of such parameter, since the value actually has not changed.

Wed, Oct 16, 9:22 AM · Restricted Project, debug-info
aprantl accepted D68697: [DWARF5] Added support for DW_AT_noreturn attribute to be emitted for C++ class member functions..
Wed, Oct 16, 9:04 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

I think the existing feature's perhaps a bit misspecified - because where you put the DEFAULTED_{in_class,out_of_class} would communicate that without needing the two values - if you put it on an out of line function definition, it's defaulted out of line, if you put it on the in-class declaration then it's defaulted inline.

But spec'd the way it is, if we want to implement this at all/if there's some actual user need (admittedly the noreturn attribute has gone in recently without a discussion of usage... so I'm not the only gatekeeper here & other people have other opinions, and that's OK - if someone (@probinson @aprantl etc) want to tell me I'm being unreasonable here & we should just put it in - it's only a bit here or there & not likely to make a huge difference to DWARF size & possibly enable some scenarios we haven't imagined yet and avoid the chicken-and-egg problem for the future implementer of such features, that's OK) - I'd essentially implement it that way. Put DEFAULTED_out_of_class on the member function definition DIE if it's defaulted there, and put DEFAULTED_in_class on the declaration DIE if it's defaulted there.

And I'd probably only spend one bit of flags on this, if possible - "not defaulted (0/assumed for all subprograms) or defaulted (1)" and translate it into one of the two values (in_class or out_of_class) in the backend based on which subprogram DIE it's attached to.

Wed, Oct 16, 8:27 AM · debug-info, Restricted Project, Restricted Project
dstenb added inline comments to D67556: [ARM][AArch64][DebugInfo] Improve call site instruction interpretation.
Wed, Oct 16, 5:49 AM · debug-info
djtodoro updated the diff for D68973: WIP: [LiveDebugValues] Support the debug entry values for modified parameters.

-Rebase

Wed, Oct 16, 5:23 AM · Restricted Project, debug-info
djtodoro updated the diff for D68209: [LiveDebugValues] Introduce entry values of unmodified params.

-Addressing comments

Wed, Oct 16, 5:23 AM · Restricted Project, debug-info
djtodoro added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

Thanks for working on this.

Wed, Oct 16, 5:20 AM · debug-info, Restricted Project
djtodoro added inline comments to D68973: WIP: [LiveDebugValues] Support the debug entry values for modified parameters.
Wed, Oct 16, 5:20 AM · Restricted Project, debug-info
djtodoro added a comment to D68209: [LiveDebugValues] Introduce entry values of unmodified params.

@vsk Thanks for the comments! Sure, I will try. :)

Wed, Oct 16, 4:52 AM · Restricted Project, debug-info
krisb updated the summary of D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.
Wed, Oct 16, 4:09 AM · debug-info, Restricted Project
Orlando added a comment to D68816: [NFC] Replace a linked list in LiveDebugVariables pass with a DenseMap.

After timing some more self-hosts it looks like my original numbers which looked
too good to be true sadly are just that. The self-host asan build time reduction
seems correct (-3.5%) but the non-asan builds are much less impacted then first
thought (about -1%). These numbers are more in line with my original expectation.

Wed, Oct 16, 1:45 AM · Restricted Project, debug-info
Orlando updated the summary of D68816: [NFC] Replace a linked list in LiveDebugVariables pass with a DenseMap.
Wed, Oct 16, 1:45 AM · Restricted Project, debug-info
Orlando closed D68816: [NFC] Replace a linked list in LiveDebugVariables pass with a DenseMap.
Wed, Oct 16, 1:36 AM · Restricted Project, debug-info

Tue, Oct 15

SouraVX updated the diff for D68697: [DWARF5] Added support for DW_AT_noreturn attribute to be emitted for C++ class member functions..

Added test case in CFE for checking expected LLVM IR emission.

Tue, Oct 15, 10:13 PM · Restricted Project, Restricted Project, debug-info
dblaikie added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

Their's not much information available behind the suggestion or intention for adding this feature to Spec. http://dwarfstd.org/ShowIssue.php?issue=141215.3 -- I think Paul can better provide remarks on this one.

"It affects overload resolution" according to my record of the DWARF meeting where we discussed this. Although "overload" resolution might not be the technically correct term. Deleted is different from omitted, when trying to determine what to do with a particular source-language expression.

Tue, Oct 15, 5:26 PM · debug-info, Restricted Project, Restricted Project
vsk added a comment to D68209: [LiveDebugValues] Introduce entry values of unmodified params.

Hi @djtodoro, thanks for the patch as always. I'm curious about the DbgEntryValueMovement field. Could you explain it in a little more detail? At a high level, IIUC, it's used to mark copies of entry values for the purpose of simplifying location lists (i.e. keep an original AT_entry_value location description even if there are copies). Is that correct? Does the approach handle there being multiple copies of an entry value within a block?

Tue, Oct 15, 4:13 PM · Restricted Project, debug-info
probinson added a comment to D67723: [DebugInfo] Add option to disable inline line tables..

Apologies for missing this until now. Our email system keeps dropping stuff sent by Phabricator.

Tue, Oct 15, 2:14 PM · debug-info, Restricted Project, Restricted Project
vsk accepted D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag.

Sgtm.

Tue, Oct 15, 1:37 PM · debug-info
vsk accepted D68207: [IR] Remove the DIFlagArgumentNotModified debug info flag.

+ 1

Tue, Oct 15, 1:37 PM · Restricted Project, debug-info
aprantl requested changes to D68697: [DWARF5] Added support for DW_AT_noreturn attribute to be emitted for C++ class member functions..

Ah sorry. While preparing this for commit I realized that the CGDebugInfo change is untested. Can you add a Clang test that checks that CFE is emitting the expected LLVM IR?

Tue, Oct 15, 1:37 PM · Restricted Project, Restricted Project, debug-info
aprantl added inline comments to D68973: WIP: [LiveDebugValues] Support the debug entry values for modified parameters.
Tue, Oct 15, 1:28 PM · Restricted Project, debug-info
aprantl added inline comments to D68973: WIP: [LiveDebugValues] Support the debug entry values for modified parameters.
Tue, Oct 15, 1:28 PM · Restricted Project, debug-info
akhuang updated the diff for D67723: [DebugInfo] Add option to disable inline line tables..

Address comment about bad decrementing iterator.

Tue, Oct 15, 1:19 PM · debug-info, Restricted Project, Restricted Project
aprantl accepted D68207: [IR] Remove the DIFlagArgumentNotModified debug info flag.
Tue, Oct 15, 1:19 PM · Restricted Project, debug-info
aprantl accepted D68206: [clang] Remove the DIFlagArgumentNotModified debug info flag.
Tue, Oct 15, 1:19 PM · debug-info
probinson added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

Hi David,
I did some digging about DW_AT_defaulted and stuff, not much of a success but. Here's what I found -- http://dwarfstd.org/Issues.php?type=closed4 -- here it;s still marked as open, not sure what that means. Abbreviations on this page doesn't describe what "open" meant. But since, it's already in DWARF5 Spec -- it must be accepted.

Tue, Oct 15, 9:52 AM · debug-info, Restricted Project, Restricted Project
SouraVX added inline comments to D68697: [DWARF5] Added support for DW_AT_noreturn attribute to be emitted for C++ class member functions..
Tue, Oct 15, 8:56 AM · Restricted Project, Restricted Project, debug-info
SouraVX added a comment to D68697: [DWARF5] Added support for DW_AT_noreturn attribute to be emitted for C++ class member functions..

Thanks Adrian for review!
Addressing your review comments.

Tue, Oct 15, 8:56 AM · Restricted Project, Restricted Project, debug-info
SouraVX updated the diff for D68697: [DWARF5] Added support for DW_AT_noreturn attribute to be emitted for C++ class member functions..

Thanks Adrian for review!
Addressing your review comments.

Tue, Oct 15, 8:56 AM · Restricted Project, Restricted Project, debug-info
aprantl accepted D68697: [DWARF5] Added support for DW_AT_noreturn attribute to be emitted for C++ class member functions..

LGTM with updated testcase, thanks!

Tue, Oct 15, 8:28 AM · Restricted Project, Restricted Project, debug-info
SouraVX added a comment to D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions..

We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the textual IR because it uses a zero value in the defaulted/deleted subfield of SPFlags.

Thanks Paul, for suggesting this. Your approach works fine. But as I was working on some lvm-dwarfdump test cases. We seems to miss one corner case --
Consider this test case;
class foo{

foo() = default;
~foo() = default;
 void not_special() {}

};
void not_a_member_of_foo(){}

Now I'm getting DW_AT_defaulted getting emitted with value DW_DEFAULTED_no, for functions "not_special" and "not_a_member_of_foo". This behavior is undesirable since, DW_AT_defaulted attributes is only valid for C++ special member functions{Constructors/Destructors, ...}.

Please correct me if I'm wrong -- Now This attributes to- implicitly defined "0" NotDefaulted bit. which is getting checked{that's fine as long as we have a dedicated bits for distinguishing} and true for every subprogram or function in a CU.
void DwarfUnit::applySubprogramAttributes( ...
...
else if (SP->isNotDefaulted())

addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
        dwarf::DW_DEFAULTED_no);

...

Perhaps we should only emit DEFAULTED_yes, and assume anything that's not DEFAULTED_yes, is... not defaulted?

Also: What features is anyone planning to build with this information? I'm sort of inclined not to implement features without some use-case in mind/planned.

Hi David, thanks for your suggestion. But, if we do that way, we may not be able to capture it's location and, whether that function was defaulted in or out of class.

Not sure I follow - for an out-of-class defaulting, I'd expect the non-defining (declaration) DW_TAG_subprogram inside the class to not have the DW_AT_defaulted attribute - and then the out of line definition would have DW_AT_defaulted = DEFAULTED_yes. For an inline defaulted definition, the non-defining DW_TAG_subprogram would have DW_AT_defaulted= DEFAULTED_yes, and the defining DW_TAG_subprogram would have no DW_AT_defaulted, it would inherit it from the declaration via DW_AT_specification.

Regarding the intent behind doing this, we have an initial internal requirement for 100% compliance towards DWARF-5 from producer{Clang} side.

I'd like to discuss that requirement a bit further - obviously I'm not your management/customers/etc, so I may not be able to sway you, but I don't believe absence of DW_AT_defaulted would classify as DWARFv5 non-conformance to me.

Producing, say, debug_ranges instead of debug_rnglists (both in terms of teh section used, and the format of the bytes in that section) would be non-conformant. But DWARF only suggests what some forms/attributes/etc might be useful for, it doesn't require them by any means.

Any idea what the particular motivation for compliance is? So you/we could evaluate whether this feature is meeting a need or not?

Tue, Oct 15, 7:49 AM · debug-info, Restricted Project, Restricted Project
djtodoro updated the diff for D68209: [LiveDebugValues] Introduce entry values of unmodified params.

@dstenb Thanks a lot for your comments!

Tue, Oct 15, 5:26 AM · Restricted Project, debug-info
dstenb closed D67492: [DebugInfo] Add a DW_OP_LLVM_entry_value operation.
Tue, Oct 15, 4:33 AM · Restricted Project, debug-info
dstenb closed D67768: [DebugInfo] Add interface for pre-calculating the size of emitted DWARF.
Tue, Oct 15, 4:21 AM · Restricted Project, debug-info
dstenb added a comment to D67492: [DebugInfo] Add a DW_OP_LLVM_entry_value operation.

Thanks for the reviews! I'll merge this shortly.

Tue, Oct 15, 4:21 AM · Restricted Project, debug-info