[llvm] 57a371d - Remove overzealous verifier check on DW_OP_LLVM_entry_value and improve the documentation

[Adrian Prantl via llvm-commits]

Author: Adrian Prantl
Date: 2021-02-04T10:58:35-08:00
New Revision: 57a371d7010802804343d17b85ac5e0d28d0f309

URL: https://github.com/llvm/llvm-project/commit/57a371d7010802804343d17b85ac5e0d28d0f309
DIFF: https://github.com/llvm/llvm-project/commit/57a371d7010802804343d17b85ac5e0d28d0f309.diff

LOG: Remove overzealous verifier check on DW_OP_LLVM_entry_value and improve the documentation

Based on the comments in the code, the idea is that AsmPrinter is
unable to produce entry value blocks of arbitrary length, such as
DW_OP_entry_value [DW_OP_reg5 DW_OP_lit1 DW_OP_plus]. But the way the
Verifier check is written it also disallows DW_OP_entry_value
[DW_OP_reg5] DW_OP_lit1 DW_OP_plus which seems to overshoot the
target.

Note that this patch does not change any of the safety guards in
LiveDebugValues — there is zero behavior change for clang. It just
allows us to legalize more complex expressions in future patches.

rdar://73907559

Differential Revision: https://reviews.llvm.org/D95990

Added: 
    

Modified: 
    llvm/docs/LangRef.rst
    llvm/lib/IR/DebugInfoMetadata.cpp
    llvm/test/Verifier/diexpression-valid-entry-value.ll

Removed: 
    


################################################################################
diff  --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 2e735e1d634f..94951ae2dd3e 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -5259,26 +5259,28 @@ The current supported opcode vocabulary is limited:
   of the stack is treated as an address. The second stack entry is treated as an
   address space identifier.
 - ``DW_OP_stack_value`` marks a constant value.
-- ``DW_OP_LLVM_entry_value, N`` can only appear at the beginning of a
-  ``DIExpression``, and it specifies that all register and memory read
-  operations for the debug value instruction's value/address operand and for
-  the ``(N - 1)`` operations immediately following the
-  ``DW_OP_LLVM_entry_value`` refer to their respective values at function
-  entry. For example, ``!DIExpression(DW_OP_LLVM_entry_value, 1,
-  DW_OP_plus_uconst, 123, DW_OP_stack_value)`` specifies an expression where
-  the entry value of the debug value instruction's value/address operand is
-  pushed to the stack, and is added with 123. Due to framework limitations
-  ``N`` can currently only be 1.
-
-  ``DW_OP_LLVM_entry_value`` is only legal in MIR. The operation is introduced
-  by the ``LiveDebugValues`` pass; currently only for function parameters that
-  are unmodified throughout a function. Support is limited to function
-  parameter that are described as simple register location descriptions, or as
-  indirect locations (e.g. when a struct is passed-by-value to a callee via a
-  pointer to a temporary copy made in the caller). The entry value op is also
-  introduced by the ``AsmPrinter`` pass when a call site parameter value
-  (``DW_AT_call_site_parameter_value``) is represented as entry value of the
-  parameter.
+- ``DW_OP_LLVM_entry_value, N`` may only appear in MIR and at the
+  beginning of a ``DIExpression``. In DWARF a ``DBG_VALUE``
+  instruction binding a ``DIExpression(DW_OP_LLVM_entry_value`` to a
+  register is lowered to a ``DW_OP_entry_value [reg]``, pushing the
+  value the register had upon function entry onto the stack.  The next
+  ``(N - 1)`` operations will be part of the ``DW_OP_entry_value``
+  block argument. For example, ``!DIExpression(DW_OP_LLVM_entry_value,
+  1, DW_OP_plus_uconst, 123, DW_OP_stack_value)`` specifies an
+  expression where the entry value of the debug value instruction's
+  value/address operand is pushed to the stack, and is added
+  with 123. Due to framework limitations ``N`` can currently only
+  be 1.
+
+  The operation is introduced by the ``LiveDebugValues`` pass, which
+  applies it only to function parameters that are unmodified
+  throughout the function. Support is limited to simple register
+  location descriptions, or as indirect locations (e.g., when a struct
+  is passed-by-value to a callee via a pointer to a temporary copy
+  made in the caller). The entry value op is also introduced by the
+  ``AsmPrinter`` pass when a call site parameter value
+  (``DW_AT_call_site_parameter_value``) is represented as entry value
+  of the parameter.
 - ``DW_OP_breg`` (or ``DW_OP_bregx``) represents a content on the provided
   signed offset of the specified register. The opcode is only generated by the
   ``AsmPrinter`` pass to describe call site parameter value which requires an

diff  --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 77bba9f7ed0e..55eff38d3fa0 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1111,8 +1111,7 @@ bool DIExpression::isValid() const {
       // entry values of a simple register location. One reason for this is that
       // we currently can't calculate the size of the resulting DWARF block for
       // 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;
     }
     case dwarf::DW_OP_LLVM_implicit_pointer:
     case dwarf::DW_OP_LLVM_convert:
@@ -1279,9 +1278,10 @@ DIExpression *DIExpression::prependOpcodes(const DIExpression *Expr,
 
   if (EntryValue) {
     Ops.push_back(dwarf::DW_OP_LLVM_entry_value);
-    // Add size info needed for entry value expression.
-    // Add plus one for target register operand.
-    Ops.push_back(Expr->getNumElements() + 1);
+    // Use a block size of 1 for the target register operand.  The
+    // DWARF backend currently cannot emit entry values with a block
+    // size > 1.
+    Ops.push_back(1);
   }
 
   // If there are no ops to prepend, do not even add the DW_OP_stack_value.

diff  --git a/llvm/test/Verifier/diexpression-valid-entry-value.ll b/llvm/test/Verifier/diexpression-valid-entry-value.ll
index 9a4b64ddb92e..7ba1c4698f6f 100644
--- 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_lit0, DW_OP_plus)