Page MenuHomePhabricator

[LiveDebugValues] Emit parameter's entry value
Needs ReviewPublic

Authored by djtodoro on Mon, Feb 11, 3:50 AM.

Details

Summary

Job of this pass to broadcast DBG_VALUES to the all successive blocks where its preserved location is valid. As a natural way it should emit replacements for clobbered parameters location.

Also this pass keeps track which variables are visible at some basic block range. We use this information to delete DBG_CALLSITEPARAM instructions that do not have valid primary nor secondary location, meaning that backup DICallSiteParam references variable that is considered to be optimized out at that point.

Authors: @asowda, @NikolaPrica, @djtodoro, @ivanbaev

Diff Detail

Event Timeline

djtodoro created this revision.Mon, Feb 11, 3:50 AM
djtodoro added a project: debug-info.
djtodoro added a subscriber: petarj.
wolfgangp added inline comments.Mon, Feb 11, 6:16 PM
lib/CodeGen/LiveDebugValues.cpp
302

I would prefer either the name "ParameterEntryVals" or "ParamEntyrVals", throughout the file, instead of "ParametrEntryVals".

318

This type is used in several locations. Perhaps we could have a using declaration
for it and use a more compact name?

829

This is invariant in the for-loop, no? Perhaps it should be pulled ahead of it?

1002

I think this comment needs perhaps some rephrasing. Something like:
Enter DEBUG_VALUE instructions that track parameters into ParameterEntryVals. When we encounter a variable that we have already entered, we assume that we have found all parameter
entry locations and stop.

1004

some spelling errors: wheter -> whether, vraible->variable, it->is

Suggested wording:
We check whether MI is inlined in order to deduce whether the variable that it tracks
comes from a different function. If that is the case we can't track its entry value.

@wolfgangp Thanks a lot for your comments! It has been addressed.

djtodoro updated this revision to Diff 186452.Tue, Feb 12, 6:40 AM
djtodoro updated this revision to Diff 186613.Wed, Feb 13, 2:45 AM
  • Rename: VariableNotChanged ===> ArgumentNotModified
  • Refactor a test case
dstenb added a subscriber: dstenb.Wed, Feb 13, 1:19 PM
dstenb added inline comments.
lib/CodeGen/LiveDebugValues.cpp
444–446

Should this use Expr->isEntryValue()? However, the implementation of that function in D58041 only checks if the first element is the operation:

bool isEntryValue() const {
    return getNumElements() > 0 && getElement(0) == dwarf::DW_OP_GNU_entry_value;
}

Is that correct, or should it look through all elements like is done here? Or is there some reason for why they should be disjunct? If so, could you add a comment here?

NikolaPrica added inline comments.Thu, Feb 14, 3:05 AM
lib/CodeGen/LiveDebugValues.cpp
444–446

You are right! It should be isEntryValue()! Thanks!

djtodoro updated this revision to Diff 187190.Sun, Feb 17, 10:41 PM
  • Use isEntryValue() from DIExpression
dstenb added inline comments.Mon, Feb 18, 5:06 AM
lib/CodeGen/LiveDebugValues.cpp
444–446

Okay, thanks for the clarification. So DW_OP_GNU_entry_value will only be located in the first element then?

djtodoro marked an inline comment as done.Tue, Feb 19, 12:14 AM
djtodoro added inline comments.
lib/CodeGen/LiveDebugValues.cpp
444–446

In the current design, yes.
DIExpression will always have entry value DWARF operand as first element.