[POC][DebugInfo] Use entry values within IR

[djtodoro]

Summary

Using entry values on IR will give us better debugging user experience when debugging "optimized" code (swift uses llvm.dbg.values even in non-optimized code, so this refers to that code as well). This patch implements a utility (within Transforms/Utils/Local.cpp) that finds an entry Value for a given Variable.

Consider this (simple) test case:

void f1(int);
void f2(int i) {
  f1(1);
  i = i + 5;
  f1(3);
}

IR ($ clang -g -O2 -S) for the func looks:

define dso_local void @f2(i32 %i) local_unnamed_addr !dbg !7 {
entry:
  call void @llvm.dbg.value(metadata i32 %i, metadata !12, metadata !DIExpression()), !dbg !13
  tail call void @f1(i32 1), !dbg !14
  call void @llvm.dbg.value(metadata i32 %i, metadata !12, metadata !DIExpression(DW_OP_plus_uconst, 5, DW_OP_stack_value)), !dbg !13
  tail call void @f1(i32 3), !dbg !15
  ret void, !dbg !16
}

SelectionDAG for the second dbg.value generates:

DBG_VALUE $noreg, $noreg, !"i", !DIExpression(DW_OP_plus_uconst, 5, DW_OP_stack_value), debug-location !13; test.c:0 line no:2

After this set of patches, we will salvage the value by using the entry values, so the DBG_VALUE looks as following:

DBG_VALUE $edi, $noreg, !12, !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 5, DW_OP_stack_value), debug-location !13

[djtodoro]

This is actually ISel phase, but we still consider SSA, so this should be the latest point we are able to use the findEntryValue() in the pipeline.

[djtodoro]

(test.c was listed within llvm/test/DebugInfo/X86/entry-values-for-isel-invalidated-nodes.ll)
Looks like LLDB has a problem with parsing "complex" expression with DW_OP_entry_value:

$ cat main.c
#include <stdio.h>

void f1(int x) {
  printf("%d\n", x);
}

int main()
{
  f2(4);
  return 0;
}

GCC generated debug_loc (gcc-test.o):

    00000000 0000000000000000 0000000000000009 (DW_OP_reg5 (rdi))
    00000013 0000000000000009 000000000000000e (DW_OP_GNU_entry_value: (DW_OP_reg5 (rdi)); DW_OP_stack_value)
    00000029 000000000000000e 000000000000001c (DW_OP_GNU_entry_value: (DW_OP_reg5 (rdi)); DW_OP_plus_uconst: 5; DW_OP_stack_value)
$ gcc --version
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-39)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ gcc main.c gcc-test.o -o main-gcc
$ lldb ./main-gcc
(lldb) target create "./main-gcc"
Current executable set to 'main-gcc' (x86_64).
(lldb) b test.c:5
Breakpoint 1: where = main-gcc`f2 + 14 at test.c:5, address = 0x000000000040119e
(lldb) r
Process 16815 launched: 'main-gcc' (x86_64)
1
Process 16815 stopped
* thread #1, name = 'main-gcc', stop reason = breakpoint 1.1
    frame #0: 0x000000000040119e main-gcc`f2(i=5) at test.c:5
   2    void f2(int i) {
   3      f1(1);
   4      i = i + 5;
-> 5      f1(3);
   6    }
   7
(lldb) p i
(int) $0 = 5

The same situation is with the test case compiled with clang (patched by using this change).

[dstenb]

Interesting!

I will go through this more deeply, but just an initial comment is that I think it may be worth considering splitting it into two patches, in order to make it easier to review:

1. One which loosens the DIExpression limitations, and adds support to DwarfExpression. With test cases using handcrafted DIExpressions.
2. One which adds the SelectionDAG support.

[dstenb]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

608–609

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

[dstenb]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

456–458

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();

[djtodoro]

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

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

456–458

I should have removed this from the patch, since this is unused indeed (it was part of some testing during the development).

608–609

The same as above.

[djtodoro]

-Split into 2 parts
-Remove unused parts

[dstenb]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

513

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.

llvm/lib/IR/DebugInfoMetadata.cpp

1032

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)

[djtodoro]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

513

Hm... yes, I guess. It might be closer to Implicit location? I think that should be a separate change.

llvm/lib/IR/DebugInfoMetadata.cpp

1032

Yes, thanks!

[djtodoro]

• Test new expression

[djtodoro]

@dstenb Please let me know if this looks OK to you now.

Any other comments?

[Orlando]

llvm/lib/Transforms/Utils/Local.cpp

1596

Hi @djtodoro. IIUC this function assumes that the returned dbg.value - the first one we find - for parameter variable Var uses the corresponding argument value (as opposed to some other value that a mutable parameter could be assigned). If that is correct, why is that assumption safe?

[djtodoro]

llvm/lib/Transforms/Utils/Local.cpp

1596

(Hmm.... I've forgotten to take another look into this, in spite of the fact that I've had an item for it within my TODO list...)

Hi @Orlando.
I had a crazy premise in my mind, that every parameter should have a dbg_value on the beginning of the bb.entry. In some situations, the Value could be undef (due to some optimizations), but it will indicate just that the value has been optimized out (but it should be still there in the code). Is that correct? I've prepared this POC in rush, and have forgotten to check this once again. If the users() aren't sorted out, this should be reimplemented.

[Orlando]

llvm/lib/Transforms/Utils/Local.cpp

1596

Every parameter should have a dbg_value on the beginning of the bb.entry

My understanding is that there should be _at least_ one dbg.value for each param at the top of the function. E.g. in the following source, a parameter test has been split into two argument values and there's a fragment expression for each.

$ cat test.c
struct S {
  long long one;
  long long two;
};
void foo(struct S test) {}

$ clang -O2 -g -S -emit-llvm -o - test.c
...
define dso_local void @foo(i64 %test.coerce0, i64 %test.coerce1) local_unnamed_addr #0 !dbg !7 {
entry:
  call void @llvm.dbg.value(metadata i64 %test.coerce0, metadata !16, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !17
  call void @llvm.dbg.value(metadata i64 %test.coerce1, metadata !16, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !17
  ret void, !dbg !18
}
...

If the users() aren't sorted out, this should be reimplemented

I am not sure if users() is sorted. I had a hunch that it isn't and I can't see any comments indicating that it is. @jmorse - or anyone else - do you know for sure?

Supposing that users() isn't sorted and we take the first assumption as truth (that every parameter should have a dbg.value at the top of the function, ignoring fragmented parameters), then I don't think it should be too hard to go forward from here. I think we could just order the dbg.values on Instruction::comesBefore(const Instruction *).

Additionally, shouldn't we be filtering out dbg.values from inlined uses of Var?

[dblaikie]

llvm/lib/Transforms/Utils/Local.cpp

1596

I am not sure if users() is sorted. I had a hunch that it isn't and I can't see any comments indicating that it is. @jmorse - or anyone else - do you know for sure?

I'm about 98% sure that use list order is not stable. (that's why there's this: https://llvm.org/doxygen/UseListOrder_8h_source.html )

[djtodoro]

llvm/lib/Transforms/Utils/Local.cpp

1596

Thanks!

We can simply iterate throughout the entry-bb and find the first dbg.value for a given parameter.

Additionally, shouldn't we be filtering out dbg.values from inlined uses of Var?

Yes. We should avoid fragmented variables as well, since we do not support such entry values at the moment.

[djtodoro]

• Update the algorithm for finding the entry values
• Improve the tests

[Orlando]

llvm/lib/Transforms/Utils/Local.cpp

1596

This function (and the unit test) LGTM now, thanks.

[dstenb]

Sorry for a piecemeal review from my part!

llvm/lib/Transforms/Utils/Local.cpp

1612

Perhaps this is too verbose?

1613

Maybe make this an early continue?

DbgValueInst *DVI = dyn_cast<DbgValueInst>(&I);
if (!DVI)
  continue;

1622–1623

I might be misunderstanding something here, but shouldn't we want to still search for the debug value even though we have encountered an inline variant of the same variable? I guess this is something that is not really common but still.

If so, this needs to be moved above the getNumElements() check.

[djtodoro]

In D87233#2288809, @dstenb wrote:

Sorry for a piecemeal review from my part!

No problem, the tempo works for me. :) Thanks!

llvm/lib/Transforms/Utils/Local.cpp

1612

No problem, I'll remove this.

1613

Sure.

1622–1623

Nice catch, thanks!

[djtodoro]

• addressing comments

[StephenTozer]

LGTM. I think the Implicit flag idea might be able to be folded into this patch without much issue, but it's not urgent and can definitely be shifted to another patch (or even just left as a TODO). Should probably be looked at by others with experience in the area, but I think most of the comments are probably going to be for the later patches in this stack.

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

303

With regards to the comment below about using an Implicit location, this would be a good place to do it I think. If you set LocationKind = isIndirect() ? Memory : Implicit, then DwarfExpression::addExpression will add the required stack value automatically at the end (in addition to removing the need to add || isEntryValue() to the clauses below).

[dstenb]

I'm equally fine with doing the Implicit flag change here, or for someone to do it in a separate patch.

[djtodoro]

@StephenTozer @dstenb Thanks for your comments! I'll try to change the approach (by putting some extra context), so we might end up changing this one (a bit).

I'll let you know once I am ready. :)

[djtodoro]

I am about to post new set of patches addressing this problem. We are back to the initial idea that refers to extending the llvm.dbg.value() to hold info about the entry Value. We need to do that, since the current approach (by relying on the method that uses the DILocalVariable*) couldn't detect variable modifications (in an efficient way). Another advantage we are getting with this new approach is that we can handle local variables as well, if it can be expressed in terms of an entry value such as:

void fn (int param) {
 ...
 int local = param + 2;
 ...

The idea is to extend the debug intrinsic with two extra operands:

1. 4th operand points to the Value * that holds the entry value the variable depends on
2. 5th the expression that applies to the entry Value * expressing a modification in terms of the entry value

In this initial stage, I am not handling the AutoUpgrade and bitcode backward compatibility. I will add that as well in following days, as well as some unit tests for the new functionality.

[djtodoro]

• Extend the llvm.dbg.value() with 2 operands by describing Entry Value

TODOs:

1. Handle auto-upgrade
2. Handle bitcode backward compatibility
3. Add tests (unit tests)
4. Add documentation

For now, the usage of this can be seen at: D87357

[djtodoro]

• Avoid null metadata within the new llvm.dbg.value()

[dstenb]

Some minor code style comments while I look into this.

llvm/lib/Transforms/Utils/Local.cpp

1595–1637

Perhaps make the return type Optional<EntryValue>?

1629–1633

I think this should be able to be simplified into:

return DIExpression::prepend(DIExpr, DIExpression::StackValue, Offset);

(And perhaps the lambda is not necessary then.)

[djtodoro]

llvm/lib/Transforms/Utils/Local.cpp

1595–1637

Makes sense, I'll refactor it :)

1629–1633

I've used "copy-paste" technique to make the tests working. This piece of code was copied from salvageDebugInfoImpl(), and I think this can be refactored into a static function or so.

[djtodoro]

• Use VMContext instead of InsertBB->getParent()->getContext()

• Additional TODOs:
• handle findDbgUsers() and dropDebugUsers()
• refactoring

[djtodoro]

TODO: Handle Function Inlining

[dstenb]

If I understand this correctly, the new {EntryValue, EntryExpr} operands do, if EntryExpr is not undef, specify a location that is identical to the dbg.value's current {Value, Expr} operands, but with DW_OP_LLVM_entry_value implicitly being applied to EntryValue before EntryExpr. Is that correct?

If so, just throwing out some questions (sorry in case I have overlooked the answers for this in the implementation):

• I assume that we are only interested in EntryValue operands that are Argument values? Should the verifier catch that? Or is there some case where it could anything else than an Argument?

• What should happen in cases where the dbg.values Value operand already is an Argument? Do we need to specify that value in the EntryValue operand also to ensure that entry values can be emitted?

llvm/lib/Transforms/Utils/Local.cpp

1615–1618

Nit: Can use BI here instead.

[djtodoro]

In D87233#2316204, @dstenb wrote:

If I understand this correctly, the new {EntryValue, EntryExpr} operands do, if EntryExpr is not undef, specify a location that is identical to the dbg.value's current {Value, Expr} operands, but with DW_OP_LLVM_entry_value implicitly being applied to EntryValue before EntryExpr. Is that correct?

Yes, that is correct.

If so, just throwing out some questions (sorry in case I have overlooked the answers for this in the implementation):

• I assume that we are only interested in EntryValue operands that are Argument values? Should the verifier catch that? Or is there some case where it could anything else than an Argument?

I think we should extend the Verifier with some kind of support for this. Not sure at the moment how we are going to handle this:

fn(%param) {
...
}

isa<Argument>(%param) ==>true
isa<Argument>(%param.something) ==> false (something could be `addr`)

Actually, I extended the Verifier locally, and I am catching some cases during the replaceAllUsesWith() where the entry value needs to be handled differently, e.g.:

llvm.dbg.value(%p1, !var_p1, !DIExpr(), %p1, !DIExpr()) becomes llvm.dbg.value(i32 0, !var_p1, !DIExpr(), i32 0, !DIExpr())

but we want the entry value to be an undef in this situation, so I am wondering whether we need a new representation of the entry-value, something like MetadataAsEntryValue, so we can invalidate such nodes in an efficent way.

• What should happen in cases where the dbg.values Value operand already is an Argument? Do we need to specify that value in the EntryValue operand also to ensure that entry values can be emitted?

We should, since the Argument (from some point) can start depending on different entry value, please consider this case:

void fn (int p1, int p2) {
   ...
   p1 = p2;
}

And the intrinsics would be:

llvm.dbg.value(%p1, !var_p1, !DIExpr(), %p1, !DIExpr())
llvm.dbg.value(%p2, !var_p1, !DIExpr(), %p2, !DIExpr())
....
llvm.dbg.value(%p1, !var_p1, !DIExpr(), %p2, !DIExpr()) <=== p1 is an argument, but it depends on different entry value

[djtodoro]

In D87233#2321073, @djtodoro wrote:

In D87233#2316204, @dstenb wrote:

If I understand this correctly, the new {EntryValue, EntryExpr} operands do, if EntryExpr is not undef, specify a location that is identical to the dbg.value's current {Value, Expr} operands, but with DW_OP_LLVM_entry_value implicitly being applied to EntryValue before EntryExpr. Is that correct?

Yes, that is correct.

If so, just throwing out some questions (sorry in case I have overlooked the answers for this in the implementation):

• I assume that we are only interested in EntryValue operands that are Argument values? Should the verifier catch that? Or is there some case where it could anything else than an Argument?

I think we should extend the Verifier with some kind of support for this. Not sure at the moment how we are going to handle this:

fn(%param) {
...
}

isa<Argument>(%param) ==>true
isa<Argument>(%param.something) ==> false (something could be `addr`)

Actually, I extended the Verifier locally, and I am catching some cases during the replaceAllUsesWith() where the entry value needs to be handled differently, e.g.:

llvm.dbg.value(%p1, !var_p1, !DIExpr(), %p1, !DIExpr()) becomes llvm.dbg.value(i32 0, !var_p1, !DIExpr(), i32 0, !DIExpr())

but we want the entry value to be an undef in this situation, so I am wondering whether we need a new representation of the entry-value, something like MetadataAsEntryValue, so we can invalidate such nodes in an efficent way.

• What should happen in cases where the dbg.values Value operand already is an Argument? Do we need to specify that value in the EntryValue operand also to ensure that entry values can be emitted?

We should, since the Argument (from some point) can start depending on different entry value, please consider this case:

void fn (int p1, int p2) {
   ...
   p1 = p2;
}

And the intrinsics would be:

llvm.dbg.value(%p1, !var_p1, !DIExpr(), %p1, !DIExpr())
llvm.dbg.value(%p2, !var_p1, !DIExpr(), %p2, !DIExpr())

!var_p2, sorry for my mistake

....
llvm.dbg.value(%p1, !var_p1, !DIExpr(), %p2, !DIExpr()) <=== p1 is an argument, but it depends on different entry value

[StephenTozer]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

290–292

Should be fixed up before merging (relatively simple logic reassociation).

llvm/lib/Transforms/Utils/Local.cpp

1625

Nit: I believe you don't need to end the final statement in LLVM_DEBUG with a semicolon.

1627–1628

Would it not be possible to just use salvageDebugInfoImpl directly here? As far as I understand, there's no reason we shouldn't be able to support all of the salvage options for an entry value. Also, it should be possible to perform this salvage repeatedly; i.e. if we have:

void foo(int param) {
  int i = param + 3;
  int j = i + 6;
}

When creating an entry value for j, we could attempt to salvage it, find that the first operand %i is not an argument but can itself be salvaged, then attempt to salvage %i and find that its first operand is an argument, resulting in an entry value %param, DIExpression(DW_OP_plus_uconst, 3, DW_OP_plus_uconst, 6).

[djtodoro]

• Support old metadata
• Fix testcases

llvm/include/llvm/Transforms/Utils/Local.h

 /// 'V' points to. This may include a mix of dbg.declare and
 /// dbg.addr intrinsics.
 TinyPtrVector<DbgVariableIntrinsic *> FindDbgAddrUses(Value *V);
 
 /// Like \c FindDbgAddrUses, but only returns dbg.declare intrinsics, not
 /// dbg.addr.
 TinyPtrVector<DbgDeclareInst *> FindDbgDeclareUses(Value *V);
 
+/// Finds an entry value for the variable.
+Value *findEntryValue(DILocalVariable *Var, Function &F);
+
 /// Finds the llvm.dbg.value intrinsics describing a value.
 void findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V);
 
 /// Finds the debug info intrinsics describing a value.
 void findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgInsts, Value *V);
 
 /// Replaces llvm.dbg.declare instruction when the address it
 /// describes is replaced with a new value. If Deref is true, an

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

if ((HasComplexExpression || IsEmittingEntryValue) && DwarfRegs.size() > 1) {
     LocationKind = Unknown;
     return false;
   }
 
   // Handle simple register locations. If we are supposed to emit
   // a call site parameter expression and if that expression is just a register
   // location, emit it with addBReg and offset 0, because we should emit a DWARF
   // expression representing a value, rather than a location.
-  if (!isMemoryLocation() && !HasComplexExpression &&
+  // FIXME: Simplify the statements regarding entry values.
+  if (!isMemoryLocation() && (!HasComplexExpression || isEntryValue()) &&
       (!isParameterValue() || isEntryValue())) {
     for (auto &Reg : DwarfRegs) {
       if (Reg.DwarfRegNo >= 0)
         addReg(Reg.DwarfRegNo, Reg.Comment);
       addOpPiece(Reg.SubRegSize);
     }
 
     if (isEntryValue())
       finalizeEntryValue();
 
     if (isEntryValue() && !isIndirect() && !isParameterValue() &&
-        DwarfVersion >= 4)
+        DwarfVersion >= 4 && !HasComplexExpression)
       emitOp(dwarf::DW_OP_stack_value);
 
     DwarfRegs.clear();
     return true;
   }
 
   // Don't emit locations that cannot be expressed without DW_OP_stack_value.
   if (DwarfVersion < 4)
default:
       return false;
     }
   }
   return true;
 }
 
 void DwarfExpression::addExpression(DIExpressionCursor &&ExprCursor,
                                     unsigned FragmentOffsetInBits) {
   // 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");
 
   // If we need to mask out a subregister, do it now, unless the next
   // operation would emit an OpPiece anyway.
   auto N = ExprCursor.peek();
   if (SubRegisterSizeInBits && N && (N->getOp() != dwarf::DW_OP_LLVM_fragment))
     maskSubRegister();
 
   Optional<DIExpression::ExprOperand> PrevConvertOp = None;
case dwarf::DW_OP_LLVM_fragment: {
       // Emit the DW_OP_piece.
       addOpPiece(SizeInBits, SubRegisterOffsetInBits);
       setSubRegisterPiece(0, 0);
       // Reset the location description kind.
       LocationKind = Unknown;
       return;
     }
     case dwarf::DW_OP_plus_uconst:
-      assert(!isRegisterLocation());
+      assert(!isRegisterLocation() || isEntryValue());
       emitOp(dwarf::DW_OP_plus_uconst);
       emitUnsigned(Op->getArg(0));
       break;
     case dwarf::DW_OP_plus:
     case dwarf::DW_OP_minus:
     case dwarf::DW_OP_mul:
     case dwarf::DW_OP_div:
     case dwarf::DW_OP_mod:
case dwarf::DW_OP_deref:
       if (!isMemoryLocation() && ::isMemoryLocation(ExprCursor))
         // Turning this into a memory location description makes the deref
         // implicit.
         LocationKind = Memory;
       else
         emitOp(dwarf::DW_OP_deref);
       break;
     case dwarf::DW_OP_constu:
-      assert(!isRegisterLocation());
+      assert(!isRegisterLocation() || isEntryValue());
       emitConstu(Op->getArg(0));
       break;
     case dwarf::DW_OP_LLVM_convert: {
       unsigned BitSize = Op->getArg(0);
       dwarf::TypeKind Encoding = static_cast<dwarf::TypeKind>(Op->getArg(1));
       if (DwarfVersion >= 5) {
         emitOp(dwarf::DW_OP_convert);
         // If targeting a location-list; simply emit the index into the raw
default:
       llvm_unreachable("unhandled opcode found in expression");
     }
   }
 
   if (isImplicitLocation() && !isParameterValue())
     // Turn this into an implicit location description.
     addStackValue();
 }
 
 /// add masking operations to stencil out a subregister.
 void DwarfExpression::maskSubRegister() {
   assert(SubRegisterSizeInBits && "no subregister was registered");
   if (SubRegisterOffsetInBits > 0)
     addShr(SubRegisterOffsetInBits);
   uint64_t Mask = (1ULL << (uint64_t)SubRegisterSizeInBits) - 1ULL;
   addAnd(Mask);
 }
 

llvm/lib/IR/DebugInfoMetadata.cpp

case dwarf::DW_OP_swap: {
       break;
     }
     case dwarf::DW_OP_LLVM_entry_value: {
       // An entry value operator must appear at the beginning and the number of
       // operations it cover can currently only be 1, because we support only
       // 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_convert:
     case dwarf::DW_OP_LLVM_tag_offset:
     case dwarf::DW_OP_constu:
     case dwarf::DW_OP_plus_uconst:
     case dwarf::DW_OP_plus:
     case dwarf::DW_OP_minus:
     case dwarf::DW_OP_mul:
DIExpression *DIExpression::prependOpcodes(const DIExpression *Expr,
                                            SmallVectorImpl<uint64_t> &Ops,
                                            bool StackValue,
                                            bool EntryValue) {
   assert(Expr && "Can't prepend ops to this expression");
 
   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);
+    // It is always of size 1 at the moment, since we support
+    // simple register locations only.
+    Ops.push_back(1);
   }
 
   // If there are no ops to prepend, do not even add the DW_OP_stack_value.
   if (Ops.empty())
     StackValue = false;
   for (auto Op : Expr->expr_ops()) {
     // A DW_OP_stack_value comes at the end, but before a DW_OP_LLVM_fragment.
     if (StackValue) {

llvm/lib/Transforms/Utils/Local.cpp

 
 TinyPtrVector<DbgDeclareInst *> llvm::FindDbgDeclareUses(Value *V) {
   TinyPtrVector<DbgDeclareInst *> DDIs;
   for (DbgVariableIntrinsic *DVI : FindDbgAddrUses(V))
     if (auto *DDI = dyn_cast<DbgDeclareInst>(DVI))
       DDIs.push_back(DDI);
   return DDIs;
 }
 
+Value *llvm::findEntryValue(DILocalVariable *Var, Function &F) {
+  // Skip empty functions.
+  if (F.empty())
+    return nullptr;
+
+  // TODO: Find an entry value for local variables,
+  // if the local variable value can be expressed
+  // in terms of an entry value (of a parameter).
+  if (!cast<DILocalVariable>(Var)->isParameter())
+    return nullptr;
+
+  LLVM_DEBUG(llvm::dbgs() << "Finding entry value for variable: ";
+             Var->dump(););
+
+  BasicBlock &EntryBB = *(F.begin());
+  for (auto &I : EntryBB) {
+    LLVM_DEBUG(llvm::dbgs() << "  Investigating: "; I.dump(););
+    if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(&I)) {
+      if (DVI->getVariable() != Var)
+        continue;
+
+      // TODO: Add support for variables with pre-existing elements.
+      if (DVI->getExpression()->getNumElements() > 0)
+        return nullptr;
+
+      // Do not consider variables from inlined functions.
+      if (DVI->getDebugLoc()->getInlinedAt())
+        return nullptr;
+
+      auto V = DVI->getValue();
+      if (isa<UndefValue>(V))
+        return nullptr;
+
+      LLVM_DEBUG(llvm::dbgs() << "    Found: "; V->dump(););
+      return V;
+    }
+  }
+
+  return nullptr;
+}
+
 void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V) {
   // This function is hot. Check whether the value has any metadata to avoid a
   // DenseMap lookup.
   if (!V->isUsedByMetadata())
     return;
   if (auto *L = LocalAsMetadata::getIfExists(V))
     if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L))
       for (User *U : MDV->users())
         if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(U))

llvm/test/Verifier/diexpression-valid-entry-value.ll

 ; RUN: opt -S < %s 2>&1 | FileCheck %s
 
-!named = !{!0}
+!named = !{!0, !1, !2}
 ; 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)
+!2 = !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_constu, 9, DW_OP_minus, DW_OP_stack_value)

llvm/unittests/Transforms/Utils/LocalTest.cpp

 TEST_F(SalvageDebugInfoTest, RecursiveBlockSimplification) {
   BasicBlock *BB = &F->front();
   ASSERT_TRUE(BB);
   bool Deleted = SimplifyInstructionsInBlock(BB);
   ASSERT_TRUE(Deleted);
   verifyDebugValuesAreSalvaged();
 }
 
+TEST(Local, FindEntryValue) {
+  LLVMContext Ctx;
+
+  std::unique_ptr<Module> M = parseIR(Ctx,
+                                      R"(
+    define internal void @foo(i32 %i, i64 %p.0, i64 %p.1, i32 %j, i32 %k) !dbg !4 {
+    entry:
+      call void @llvm.dbg.value(metadata i32 %i, metadata !8, metadata !DIExpression()), !dbg !6
+      call void @llvm.dbg.value(metadata i64 %p.0, metadata !9, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !6
+      call void @llvm.dbg.value(metadata i64 %p.1, metadata !9, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !6
+      call void @llvm.dbg.value(metadata i32 %j, metadata !15, metadata !DIExpression()), !dbg !16
+      call void @llvm.dbg.value(metadata i32 undef, metadata !17, metadata !DIExpression()), !dbg !6
+      ret void, !dbg !6
+    }
+
+    ; Function Attrs: nounwind readnone speculatable willreturn
+    declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!3}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "test.ll", directory: "/")
+    !2 = !{}
+    !3 = !{i32 2, !"Debug Info Version", i32 3}
+    !4 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !5, isLocal: true, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, retainedNodes: !7)
+    !5 = !DISubroutineType(types: !2)
+    !6 = !DILocation(line: 1, column: 1, scope: !4)
+    !7 = !{!8, !9, !15, !17}
+    !8 = !DILocalVariable(name: "i", arg: 1, scope: !4, file: !1, line: 2)
+    !9 = !DILocalVariable(name: "p", arg: 2, scope: !4, file: !1, line: 5, type: !10)
+    !10 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "S", file: !1, line: 1, size: 128, elements: !11)
+    !11 = !{!12, !14}
+    !12 = !DIDerivedType(tag: DW_TAG_member, name: "one", scope: !10, file: !1, line: 2, baseType: !13, size: 64)
+    !13 = !DIBasicType(name: "long long int", size: 64, encoding: DW_ATE_signed)
+    !14 = !DIDerivedType(tag: DW_TAG_member, name: "two", scope: !10, file: !1, line: 3, baseType: !13, size: 64, offset: 64)
+    !15 = !DILocalVariable(name: "j", arg: 3, scope: !4, file: !1, line: 2)
+    !16 = !DILocation(line: 1, column: 1, scope: !4, inlinedAt: !6)
+    !17 = !DILocalVariable(name: "k", arg: 4, scope: !4, file: !1, line: 2)
+
+  )");
+
+  auto &F = *cast<Function>(M->getNamedValue("foo"));
+  auto SP = F.getSubprogram();
+  auto ParamI = SP->getRetainedNodes()[0];
+  auto EntryValue = findEntryValue(cast<DILocalVariable>(ParamI), F);
+  ASSERT_TRUE(EntryValue != nullptr);
+
+  auto ParamP = SP->getRetainedNodes()[1];
+  auto EntryValue2 = findEntryValue(cast<DILocalVariable>(ParamP), F);
+  // Avoid fragmented entry values.
+  ASSERT_TRUE(EntryValue2 == nullptr);
+
+  auto ParamJ = SP->getRetainedNodes()[2];
+  auto EntryValue3 = findEntryValue(cast<DILocalVariable>(ParamJ), F);
+  // Avoid inlined parameters.
+  ASSERT_TRUE(EntryValue3 == nullptr);
+
+  auto ParamK = SP->getRetainedNodes()[3];
+  auto EntryValue4 = findEntryValue(cast<DILocalVariable>(ParamK), F);
+  // Return nullptr for undefs.
+  ASSERT_TRUE(EntryValue4 == nullptr);
+}
+
 TEST(Local, ChangeToUnreachable) {
   LLVMContext Ctx;
 
   std::unique_ptr<Module> M = parseIR(Ctx,
                                       R"(
     define internal void @foo() !dbg !6 {
     entry:
       ret void, !dbg !8