[PATCH] D87233: [POC][DebugInfo] Use entry values within IR

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added a comment.

@dstenb Thanks a lot for your comments! I will split this into two patches.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:456-458
-  // Entry values can currently only cover the initial register location,
-  // and not any other parts of the following DWARF expression.
-  assert(!IsEmittingEntryValue && "Can't emit entry value around expression");
----------------
dstenb wrote:
> Is there test case where we land in this situation? As I understand it we still have the limitation inside DIExpression that it can only cover one operand (the register operand), and if I am not overlooking something we should have finalized the entry value on line 299:
> 
> ```
>     if (isEntryValue())
>       finalizeEntryValue();
> ```
I should have removed this from the patch, since this is unused indeed (it was part of some testing during the development).


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:604-605
+
+  if (IsEmittingEntryValue)
+    finalizeEntryValue();
 }
----------------
dstenb wrote:
> This means that the entry value will cover the whole expression, regardless of the number of operands that are specified in DW_OP_LLVM_entry_value? I think we want to do this conditionally in the ExprCursor loop?
> 
> Doing this after `addStackValue()` makes it so that the entry value contains an implicit location description, but as mentioned in DWARF5 2.5.1.7 the second operand in a `DW_OP_entry_value` is "a block containing a DWARF expression or a register location description", and as was discussed in D75270 and [0], location descriptions does not seem to be allowed to be used where "DWARF expressions" are specified.
> 
> [0] http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2020-March/004610.html
The same as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87233