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).
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.
-Move register identity transformation check to DwarfDebug::collectCallSiteParameters()
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.
Wed, Oct 16
I'm confused - given the presence of noncontiguous ranges, why would the "first def" be an important property?
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.
Thanks for working on this.
@vsk Thanks for the comments! Sure, I will try. :)
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.
Tue, Oct 15
Added test case in CFE for checking expected LLVM IR emission.
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?
Apologies for missing this until now. Our email system keeps dropping stuff sent by Phabricator.
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?
Address comment about bad decrementing iterator.
Thanks Adrian for review!
Addressing your review comments.
LGTM with updated testcase, thanks!
@dstenb Thanks a lot for your comments!
Thanks for the reviews! I'll merge this shortly.