[PATCH] D68209: WIP: [LiveDebugValues] Introduce entry values of unmodified/modified params

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro marked an inline comment as done.
djtodoro added a comment.

@aprantl

> So we've come full circle :-)

Unfortunately, yes. :)

> I'm not a fan of lumping arbitrary funcitonality into LiveDebugValues, but if we can keep it readable this is probably the best place for it.

+1

@jmorse

Hi Jeremy,

I think as well the set of patches you applied could be useful here, I see 2/3 are already committed and I will rebase this on top of that. :)

> If I understand you correctly (75% confidence), with those patches you would be able to identify whether a location is the entry value by examining the VarLoc's MI field, even if the location has been transferred in the meantime.

Yes, that is the goal.



================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:966
 
+bool DIExpression::isOpWithOffset() const {
+  if (getNumElements() == 0)
----------------
aprantl wrote:
> This appears to be of limited use since many DIExpressions are more complex. Is this meant to detect spill locations? If yes, could we mark them differently when we create them instead of trying to detect them here?
I am aware of that, we should definitely make it more useful somehow.

The point is that this is meant to be used to detect modification of a parameter that could be expressed in terms of its entry value. Up to this point we supported only unmodified parameters, but at this point we want to extend it and try to salvage entry values even for modified parameters (as GCC does). The problem is that we are analyzing only `DBG_VALUE`s in order to distinguish whether a parameter's value has changed and if so, the idea is only to look at the `DIExpression` to distinguish whether to track the modification (e.g. expressions like plus_const, minus_const, etc.) or not, by not analyzing the actual `MI` that performs the modification that the `DBG_VALUE` describes. In addition, the problem is in many cases the instruction performing the parameter's modification gets optimized even at `IR` level, and we have just the `DBG_VALUE` describing that modification. Consider the test case attached:

...
`b = b + 7;`
...

the `add` intrinsic will be removed after the `Combine redundant instructions` pass and we will have just the `dbg.value()` at the place describing actual modification, but we want to generate the expression like `DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 7, DW_OP_stack_value` in `LiveDebugValues`.

I think that I should split this patch into support for unmodified and modified parameters, and that will be easier to review.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68209/new/

https://reviews.llvm.org/D68209