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

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:513
     case dwarf::DW_OP_plus_uconst:
-      assert(!isRegisterLocation());
+      assert(!isRegisterLocation() || isEntryValue());
       emitOp(dwarf::DW_OP_plus_uconst);
----------------
dstenb wrote:
> This might be outside of the scope of this patch, but I wonder if should change the location kind from Register to Unknown/Implicit after finalizing the entry value? The entry value operation's operand is a register location description, but the operation itself just pushes a value on the stack.
Hm... yes, I guess. It might be closer to `Implicit` location? I think that should be a separate change.


================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:1032
       // other expressions.
-      return I->get() == expr_op_begin()->get() && I->getArg(0) == 1 &&
-             getNumElements() == 2;
+      return I->get() == expr_op_begin()->get() && I->getArg(0) == 1;
     }
----------------
dstenb wrote:
> Ah, I it seems that we did not have a small Verifier case for the last condition. Perhaps we now should add a small permissive one. Something like this:
> 
> ```
> --- a/llvm/test/Verifier/diexpression-valid-entry-value.ll
> +++ b/llvm/test/Verifier/diexpression-valid-entry-value.ll
> @@ -1,5 +1,6 @@
>  ; RUN: opt -S < %s 2>&1 | FileCheck %s
>  
> -!named = !{!0}
> +!named = !{!0, !1}
>  ; CHECK-NOT: invalid expression
>  !0 = !DIExpression(DW_OP_LLVM_entry_value, 1)
> +!1 = !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 3, DW_OP_stack_value)
> ```
Yes, thanks!


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

https://reviews.llvm.org/D87233