[SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue

[bjope]

Summary

This patch fixes PR40587.

When a dbg.value instrinsic is emitted to the DAG
by using EmitFuncArgumentDbgValue the resulting
DBG_VALUE is hoisted to the beginning of the entry
block. I think the idea is to be able to locate
a formal argument already from the start of the
function.
However, EmitFuncArgumentDbgValue only checked that
the value that was used to describe a variable was
originating from a function parameter, not that the
variable itself actually was an argument to the
function. So when for example assigning a local
variable "local" the value from an argument "a",
the DBG_VALUE instruction describing that "local"
had the value of "a" would be hoisted to the
beginning of the function, even if the scope for
"local" started somewhere else (or if "local"
had other values earlier in the function).

This patch adds some logic to EmitFuncArgumentDbgValue
to check that the variable being described actually
is an argument to the function. And that the dbg.value
being lowered already is in the entry block. Otherwise
we bail out, and the dbg.value will be handled as an
ordinary dbg.value (not as a "FuncArgumentDbgValue").

A tricky situation is when both the variable and
the value is related to function arguments, but not
the same argument. This is kind of hard to check.
Instead we verify that we do not describe the same
argument more than once as a "FuncArgumentDbgValue".
This solution works as long as opt has injected a
"first" dbg.value that corresponds to the formal
argument.

[bjope]

There is some problem with this making the test/DebugInfo/AArch64/inlined-argument.ll fail. The inlined "resources" parameter will appear optimized out. I haven't figured out why yet.

[aprantl]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

4989

This condition looks very much like the condition in https://reviews.llvm.org/D57584. Could it be factored out into a helper function with a reasonable name and documented behavior?

[bjope]

Added some code to make test/DebugInfo/AArch64/inlined-argument.ll pass.
I now allow emitting using ArgDbgValue if the dbg.value intrinsic is
in the beginning of the function entry (before any non-dbg instruction).
This is a little bit hacky, but the best solution I could find.

[bjope]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

1159

I'll investigate this a little bit more. I have a reproducer where this results would result in incorrect hosting (but that is on trunk). With this patch EmitFuncArgumentDbgValue returns false for that reproducer.
But adding an assert in the else clause (after applying this patch), and then running some test suites might give us some new reproducers (or indicate that the EmitFuncArgumentDbgValue()" call is redundant here).

4989

Yes, I'm kind of monitoring the progress of D57584 to see where we end up with these checks (I'm now using the getInlinedAt kind of check here as well). Either we can wait and refactor after these patches has landed, or we can perhaps create a common solution (in one of the patches) based on which patch that lands first.
I got a feeling that somehow they might overlap (both patches are reducing amount of hoisted DBG_VALUE nodes to the entry).

[bjope]

Revert the no longer needed modification in test/DebugInfo/Sparc/subreg.ll

[jmorse]

Looks great,

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

4997

Typo dgg -> dbg

5025–5032

Will this work with Arguments that are referred to by multiple fragments? Perhaps it doesn't need to, however the two-i32s-packed-in-i64-argument in PR40188 sprung to mind as that produces two DBG_VALUEs based on one Argument.

[bjope]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5025–5032

At one point in time I tried using ArgNo = Variable->getArg(), looking at the C-level argument numbers, but then I had a test case that failed due to receiving that single struct variable in multiple registers (multiple fragments).
Now I use the argument numbers from the function on IR level. So at least that test case passed, since we get one fragment per input register. I guess it will be the same if the value is divided in multiple stack slots.

So I actually had problem with fragments in my first attempts to fix this, but I have no (real, non handwritten IR) test cases failing with the current solution.
Maybe there can be scenarios when we have multiple fragments mapping to one argument on IR level? (probably easy to do when doing a handwritten test case, but does it happen in reality)

So we might need to "complicate" this code even more in the future, or even find a totally different solution (the special case handling for function arguments seems a little bit messy IMHO). Maybe the frontend shouldn't use dbg.declare/dbg.value to describe input arguments, maybe we need dbg.argument or some other way to model this (if we really care about debug info for unused arguments, and if we really care about debug info inside the prologue)?

For now, as far as I've seen (mainly when looking at C-programs), this patch seems to improve things. Avoiding faulty placement of DBG_VALUE nodes due to incorrectly treating them as describing function arguments. There is a small risk that it is too defensive in some situations. But instead of getting weird values (like with the faulty hoisting), being defensive in this case means that we would get <optimized out> in the debugger.

[bjope]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

1159

I've run a bunch of tests (including random programs generated by csmith) and with the changes in EmitFuncArgumentDbgValue I haven't seen any situation when EmitFuncArgumentDbgValue would return true here any longer (and in the past that seems to have been incorrect). So I really think we shold remove the call to EmitFuncArgumentDbgValue here. But I rather do it in a separate patch after this one.

4997

Thanks, I'll fix that!

[jmorse]

LGTM although I'll wait for someone else to approve, plus it'd be good to hear Adrians view on the fragment scenario.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5025–5032

[...] a test case that failed due to receiving that single struct variable in multiple registers (multiple fragments)

Ah, my use of "fragments" was too vague, it was intended to mean:

Maybe there can be scenarios when we have multiple fragments mapping to one argument on IR level?

Which I tried to produce an example for, but I hit some extra bugs along the way: [0]. In the example code there [0] the struct argument comes in as one i64, and SROA splits the dbg.declare into two dbg.values that refer to different variable fragments in their DIExpressions, but both ultimately refer to the same input Argument. If it weren't for [0] I believe (95%) they'd be salvaged back to two dbg.values operating on the input Argument.

For now, as far as I've seen (mainly when looking at C-programs), this patch seems to improve things.

I agree, the argument-handling-weirdness has bugged me for a while, and this patch reduces the amount false debug information we produce. If multiple fragments of one Argument can be an issue we'll definitely hit it when solving [0].

[0] https://bugs.llvm.org/show_bug.cgi?id=40645

[aprantl]

plus it'd be good to hear Adrians view on the fragment scenario.

Do you have any specific questions in mind?

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5011

I don't understand why this is necessary?

5023

That is not generally possible. For example:

$ cat /tmp/s.c 
struct s { long long int i, j; };

int f(struct s s) {
  return s.i+s.j;
}

$ clang -g -S -emit-llvm /tmp/s.c -o -
; ModuleID = '/tmp/s.c'
source_filename = "/tmp/s.c"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.14.0"

%struct.s = type { i64, i64 }

; Function Attrs: noinline nounwind optnone ssp uwtable
define i32 @f(i64 %s.coerce0, i64 %s.coerce1) #0 !dbg !8 {
entry:

Here you'd have two dbg.values referring to the same DILocalVariable, but with different DIExpressions (with different DW_OP_LLVM_fragment). Since this case is so easy to reproduce, it would be good to at least handle it. The opposite (two DILocalVariables referring to the same function parameter) seems less interesting to me.

5025–5032

Maybe there can be scenarios when we have multiple fragments mapping to one argument on IR level? (probably easy to do when doing a handwritten test case, but does it happen in reality)

The Swift compiler splats the elements of structs into individual function arguments and has a function signature optimization pass. If the optimizer were better at preserving debug info this combination could trigger what you are describing.

[bjope]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5011

One missing piece in the example might be that the function on C level looks like this

void foo(long a, long b) {
  ...
  b = 123;
  ...
  b = a;
}

so "b" is an argument on C level, and %a and` %b` are function argument values.

The assignment b = a results in the last dbg.value where we say that "b" has the value of %a. So both the DIVariable and the value is associated with function arguments, but it is not correct to hoist this dbg.value to the function entry (for example hoisting above the dbg.value mapping "b" to 123), because the assignment is not done at function entry.

So the above maps to the @foo_other_param test.

I also made an assumption that we could not get something like this:

define @foo(i32 %a, i32 %b)  {
      entry:
        call void @llvm.dbg.value(metadata i32 %b, "b",
        ...
        call void @llvm.dbg.value(metadata i32 123, "b"
        ...
        call void @llvm.dbg.value(metadata i32 %b, "b"

where "b" first gets the value %b (at the function entry), then is reassigned another value, and then gets the value %b again. Because then the value of "b" must saved somethere so the value in the last assignment of "b" will not be %b. Now I realize that the above pattern isn't that hard to trigger, just use an input like this:

extern void fie(long);

void foo(long a, long b) {
  long c = b;
  b = 123;
  fie(b);
  b = c;
  fie(b);
}

So we can't really consider a dbg.value as describing a function argument just because "the parameter value V actually maps to the argument described by Variable". I will remove that code comment.

The only solution I can imagine right now is the one that I have implemented (assuming that only the first dbg.value that describes an argument (on IR level) is a "function argument dbg.value").

[bjope]

Updated code comments.

Added a test case involving fragments (a struct split between two input
registers). The test case also involves some alloca statements that appear
before the dbg.value statements that describe the input arguments. So the new
test case should demonstrate why it isn't enough to have the "IsInPrologue"
check and the "VariableIsFunctionInputArg" check. We also need the
FuncInfo.DescribedArgs solution if we want to get the DBG_VALUE instructions
that descibe the functions arguments at the function entry.

[bjope]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5023

The code handles the situation with fragments from your example. I've now added test/DebugInfo/X86/dbg-value-funcarg2.ll which is a little bit inspired by your example.

[bjope]

Minor adjustments to the test case (added some missing CHECK-NOT).

[jmorse]

In D57702#1391414, @aprantl wrote:

plus it'd be good to hear Adrians view on the fragment scenario.

Do you have any specific questions in mind?

I think I meant to write something like "Perhaps Adrian knows a situation where multiple fragments map to one function Argument", but had an attack of vagueness, sorry. (FWIW I think your comment on line 5032 covered that).

[aprantl]

Thanks for the explanation!

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5005

we usually write this as !DL->getInlinedAt()

5041

I assume the in-between elements are zeroed out by default?

[bjope]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5005

Yes, I'll fix that before commit.

5041

Yes, but I can add an explicit argument to resize to indicate that we want to initialize new bits to "false" to avoid relying on default arguments.

llvm/include/llvm/CodeGen/FunctionLoweringInfo.h

 // Machine IR.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CODEGEN_FUNCTIONLOWERINGINFO_H
 #define LLVM_CODEGEN_FUNCTIONLOWERINGINFO_H
 
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IndexedMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/ISDOpcodes.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
public:
 
   /// ByValArgFrameIndexMap - Keep track of frame indices for byval arguments.
   DenseMap<const Argument*, int> ByValArgFrameIndexMap;
 
   /// ArgDbgValues - A list of DBG_VALUE instructions created during isel for
   /// function arguments that are inserted after scheduling is completed.
   SmallVector<MachineInstr*, 8> ArgDbgValues;
 
+  /// Bitvector with a bit set if corresponding argument is described in
+  /// ArgDbgValues. Using arg numbers according to Argument numbering.
+  BitVector DescribedArgs;
+
   /// RegFixups - Registers which need to be replaced after isel is done.
   DenseMap<unsigned, unsigned> RegFixups;
 
   DenseSet<unsigned> RegsWithFixups;
 
   /// StatepointStackSlots - A list of temporary stack slots (frame indices)
   /// used to spill values at a statepoint.  We store them here to enable
   /// reuse of the same stack slots across different statepoints in different

llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp

 void FunctionLoweringInfo::clear() {
   MBBMap.clear();
   ValueMap.clear();
   VirtReg2Value.clear();
   StaticAllocaMap.clear();
   LiveOutRegInfo.clear();
   VisitedBBs.clear();
   ArgDbgValues.clear();
+  DescribedArgs.clear();
   ByValArgFrameIndexMap.clear();
   RegFixups.clear();
   RegsWithFixups.clear();
   StatepointStackSlots.clear();
   StatepointSpillMaps.clear();
   PreferredExtendType.clear();
 }
 

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

for (auto &DDI : DDIV) {
     unsigned ValSDNodeOrder = Val.getNode()->getIROrder();
     unsigned DbgSDNodeOrder = DDI.getSDNodeOrder();
     DILocalVariable *Variable = DI->getVariable();
     DIExpression *Expr = DI->getExpression();
     assert(Variable->isValidLocationForIntrinsic(dl) &&
            "Expected inlined-at fields to agree");
     SDDbgValue *SDV;
     if (Val.getNode()) {
+      // FIXME: I doubt that it is correct to resolve a dangling DbgValue as a
+      // FuncArgumentDbgValue (it would be hoisted to the function entry, and if
+      // we couldn't resolve it directly when examining the DbgValue intrinsic
+      // in the first place we should not be more successful here). Unless we
+      // have some test case that prove this to be correct we should avoid
+      // calling EmitFuncArgumentDbgValue here.
       if (!EmitFuncArgumentDbgValue(V, Variable, Expr, dl, false, Val)) {
         LLVM_DEBUG(dbgs() << "Resolve dangling debug info [order="
                           << DbgSDNodeOrder << "] for:\n  " << *DI << "\n");
         LLVM_DEBUG(dbgs() << "  By mapping to:\n    "; Val.dump());
         // Increase the SDNodeOrder for the DbgValue here to make sure it is
         // inserted after the definition of Val when emitting the instructions
         // after ISel. An alternative could be to teach
         // ScheduleDAGSDNodes::EmitSchedule to delay the insertion properly.

 /// instruction selection, they will be inserted to the entry BB.
 bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
     const Value *V, DILocalVariable *Variable, DIExpression *Expr,
     DILocation *DL, bool IsDbgDeclare, const SDValue &N) {
   const Argument *Arg = dyn_cast<Argument>(V);
   if (!Arg)
     return false;
 
+  if (!IsDbgDeclare) {
+    // ArgDbgValues are hoisted to the beginning of the entry block. So we
+    // should only emit as ArgDbgValue if the dbg.value intrinsic is found in
+    // the entry block.
+    bool IsInEntryBlock = FuncInfo.MBB == &FuncInfo.MF->front();
+    if (!IsInEntryBlock)
+      return false;
+
+    // ArgDbgValues are hoisted to the beginning of the entry block.  So we
+    // should only emit as ArgDbgValue if the dbg.value intrinsic describes a
+    // variable that also is a param.
+    //
+    // Although, if we are at the top of the entry block already, we can still
+    // emit using ArgDbgValue. This might catch some situations when the
+    // dbg.value refers to an argument that isn't used in the entry block, so
+    // any CopyToReg node would be optimized out and the only way to express
+    // this DBG_VALUE is by using the physical reg (or FI) as done in this
+    // method.  ArgDbgValues are hoisted to the beginning of the entry block. So
+    // we should only emit as ArgDbgValue if the Variable is an argument to the
+    // current function, and the dbg.value intrinsic is found in the entry
+    // block.
+    bool VariableIsFunctionInputArg = Variable->isParameter() &&
+        DL->getInlinedAt() == nullptr;
+    bool IsInPrologue = SDNodeOrder == LowestSDNodeOrder;
+    if (!IsInPrologue && !VariableIsFunctionInputArg)
+      return false;
+
+    // Here we assume that a function argument on IR level only can be used to
+    // describe one input parameter on source level. If we for example have
+    // source code like this
+    //
+    //    struct A { long x, y; };
+    //    void foo(struct A a, long b) {
+    //      ...
+    //      b = a.x;
+    //      ...
+    //    }
+    //
+    // and IR like this
+    //
+    //  define void @foo(i32 %a1, i32 %a2, i32 %b)  {
+    //  entry:
+    //    call void @llvm.dbg.value(metadata i32 %a1, "a", DW_OP_LLVM_fragment
+    //    call void @llvm.dbg.value(metadata i32 %a2, "a", DW_OP_LLVM_fragment
+    //    call void @llvm.dbg.value(metadata i32 %b, "b",
+    //    ...
+    //    call void @llvm.dbg.value(metadata i32 %a1, "b"
+    //    ...
+    //
+    // then the last dbg.value is describing a parameter "b" using a value that
+    // is an argument. But since we already has used %a1 to describe a parameter
+    // we should not handle that last dbg.value here (that would result in an
+    // incorrect hoisting of the DBG_VALUE to the function entry).
+    // Notice that we allow one dbg.value per IR level argument, to accomodate
+    // for the situation with fragments above.
+    if (VariableIsFunctionInputArg) {
+      unsigned ArgNo = Arg->getArgNo();
+      if (ArgNo >= FuncInfo.DescribedArgs.size())
+        FuncInfo.DescribedArgs.resize(ArgNo + 1);
+      else if (!IsInPrologue && FuncInfo.DescribedArgs.test(ArgNo))
+        return false;
+      FuncInfo.DescribedArgs.set(ArgNo);
+    }
+  }
+
   MachineFunction &MF = DAG.getMachineFunction();
   const TargetInstrInfo *TII = DAG.getSubtarget().getInstrInfo();
 
   bool IsIndirect = false;
   Optional<MachineOperand> Op;
   // Some arguments' frame index is recorded during argument lowering.
   int FI = FuncInfo.getArgumentFrameIndex(Arg);
   if (FI != std::numeric_limits<int>::max())

llvm/test/DebugInfo/X86/dbg-value-funcarg.ll

+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -start-after=codegenprepare -stop-before=expand-isel-pseudos -o - %s | FileCheck %s
+
+; Input to this test looked like this and was compiled using: clang -g -O1 -mllvm -stop-after=codegenprepare -S
+;
+;    extern void bar(int);
+;
+;    void foo_local(int t1a) {
+;      int local = 123;
+;      bar(local);
+;      local = t1a;
+;      bar(local);
+;    }
+;
+;    void foo_other_param(int t2a, int t2b) {
+;      bar(t2b);
+;      t2b = 123;
+;      bar(t2b);
+;      t2b = t2a;
+;      bar(t2b);
+;    }
+;
+;    void foo_same_param(int t3a) {
+;      bar(t3a);
+;      int tmp = t3a;
+;      t3a = 123;
+;      bar(t3a);
+;      t3a = tmp;
+;      bar(t3a);
+;    }
+;
+
+; Catch metadata references for involved variables.
+;
+; CHECK-DAG: ![[T1A:.*]] = !DILocalVariable(name: "t1a"
+; CHECK-DAG: ![[LOCAL:.*]] = !DILocalVariable(name: "local"
+; CHECK-DAG: ![[T2A:.*]] = !DILocalVariable(name: "t2a"
+; CHECK-DAG: ![[T2B:.*]] = !DILocalVariable(name: "t2b"
+; CHECK-DAG: ![[T3A:.*]] = !DILocalVariable(name: "t3a"
+; CHECK-DAG: ![[TMP:.*]] = !DILocalVariable(name: "tmp"
+
+
+define dso_local void @foo_local(i32 %t1a) local_unnamed_addr #0 !dbg !7 {
+; CHECK-LABEL: name:            foo_local
+; CHECK-NOT: DBG_VALUE
+; CHECK:      DBG_VALUE $edi, $noreg, ![[T1A]], !DIExpression(),
+; CHECK-NEXT: %0:gr32 = COPY $edi
+; CHECK-NEXT: DBG_VALUE %0, $noreg, ![[T1A]], !DIExpression(),
+; CHECK-NEXT: DBG_VALUE 123, $noreg, ![[LOCAL]], !DIExpression(),
+; CHECK-NOT: DBG_VALUE
+; CHECK:    CALL64pcrel32 @bar,
+; CHECK:    DBG_VALUE %0, $noreg, ![[LOCAL]], !DIExpression(),
+; CHECK:    DBG_VALUE $edi, $noreg, ![[T1A]], !DIExpression(),
+; CHECK-NOT: DBG_VALUE
+; CHECK:    TCRETURNdi64 @bar,
+entry:
+  call void @llvm.dbg.value(metadata i32 %t1a, metadata !12, metadata !DIExpression()), !dbg !14
+  call void @llvm.dbg.value(metadata i32 123, metadata !13, metadata !DIExpression()), !dbg !15
+  tail call void @bar(i32 123) #3, !dbg !16
+  call void @llvm.dbg.value(metadata i32 %t1a, metadata !13, metadata !DIExpression()), !dbg !15
+  tail call void @bar(i32 %t1a) #3, !dbg !17
+  ret void, !dbg !18
+}
+
+define dso_local void @foo_other_param(i32 %t2a, i32 %t2b) local_unnamed_addr #0 !dbg !19 {
+; CHECK-LABEL: name:            foo_other_param
+; CHECK: DBG_VALUE $edi, $noreg, ![[T2A]], !DIExpression(),
+; CHECK: DBG_VALUE $esi, $noreg, ![[T2B]], !DIExpression(),
+; CHECK: %1:gr32 = COPY $esi
+; CHECK: DBG_VALUE %1, $noreg, ![[T2B]], !DIExpression(),
+; CHECK: %0:gr32 = COPY $edi
+; CHECK: DBG_VALUE %0, $noreg, ![[T2A]], !DIExpression(),
+; CHECK: DBG_VALUE $edi, $noreg, ![[T2B]], !DIExpression(),
+; CHECK: CALL64pcrel32 @bar,
+; CHECK: DBG_VALUE 123, $noreg, ![[T2B]], !DIExpression(),
+; CHECK: CALL64pcrel32 @bar,
+; CHECK: DBG_VALUE %0, $noreg, ![[T2B]], !DIExpression(),
+; CHECK: DBG_VALUE $edi, $noreg, ![[T2A]], !DIExpression(),
+; CHECK: TCRETURNdi64 @bar,
+
+entry:
+  call void @llvm.dbg.value(metadata i32 %t2a, metadata !23, metadata !DIExpression()), !dbg !25
+  call void @llvm.dbg.value(metadata i32 %t2b, metadata !24, metadata !DIExpression()), !dbg !26
+  tail call void @bar(i32 %t2b) #3, !dbg !27
+  call void @llvm.dbg.value(metadata i32 123, metadata !24, metadata !DIExpression()), !dbg !26
+  tail call void @bar(i32 123) #3, !dbg !28
+  call void @llvm.dbg.value(metadata i32 %t2a, metadata !24, metadata !DIExpression()), !dbg !26
+  tail call void @bar(i32 %t2a) #3, !dbg !29
+  ret void, !dbg !30
+}
+
+define dso_local void @foo_same_param(i32 %t3a) local_unnamed_addr #0 !dbg !31 {
+; CHECK-LABEL: name:            foo_same_param
+; CHECK: DBG_VALUE $edi, $noreg, ![[T3A]], !DIExpression(),
+; CHECK: %0:gr32 = COPY $edi
+; CHECK: DBG_VALUE %0, $noreg, ![[T3A]], !DIExpression(),
+; CHECK: CALL64pcrel32 @bar,
+; CHECK: DBG_VALUE %0, $noreg, ![[TMP]], !DIExpression(),
+; CHECK: DBG_VALUE 123, $noreg, ![[T3A]], !DIExpression(),
+; CHECK: CALL64pcrel32 @bar,
+; CHECK: DBG_VALUE %0, $noreg, ![[T3A]], !DIExpression(),
+; CHECK: TCRETURNdi64 @bar,
+entry:
+  call void @llvm.dbg.value(metadata i32 %t3a, metadata !33, metadata !DIExpression()), !dbg !35
+  tail call void @bar(i32 %t3a) #3, !dbg !36
+  call void @llvm.dbg.value(metadata i32 %t3a, metadata !34, metadata !DIExpression()), !dbg !37
+  call void @llvm.dbg.value(metadata i32 123, metadata !33, metadata !DIExpression()), !dbg !35
+  tail call void @bar(i32 123) #3, !dbg !38
+  call void @llvm.dbg.value(metadata i32 %t3a, metadata !33, metadata !DIExpression()), !dbg !35
+  tail call void @bar(i32 %t3a) #3, !dbg !39
+  ret void, !dbg !40
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+declare dso_local void @bar(i32) local_unnamed_addr
+
+attributes #0 = { nounwind uwtable }
+attributes #2 = { nounwind readnone speculatable }
+attributes #3 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "foo.c", directory: "")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 9.0.0"}
+!7 = distinct !DISubprogram(name: "foo_local", scope: !1, file: !1, line: 3, type: !8, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+!8 = !DISubroutineType(types: !9)
+!9 = !{null, !10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !{!12, !13}
+!12 = !DILocalVariable(name: "t1a", arg: 1, scope: !7, file: !1, line: 3, type: !10)
+!13 = !DILocalVariable(name: "local", scope: !7, file: !1, line: 4, type: !10)
+!14 = !DILocation(line: 3, column: 20, scope: !7)
+!15 = !DILocation(line: 4, column: 7, scope: !7)
+!16 = !DILocation(line: 5, column: 3, scope: !7)
+!17 = !DILocation(line: 7, column: 3, scope: !7)
+!18 = !DILocation(line: 8, column: 1, scope: !7)
+!19 = distinct !DISubprogram(name: "foo_other_param", scope: !1, file: !1, line: 10, type: !20, scopeLine: 10, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !22)
+!20 = !DISubroutineType(types: !21)
+!21 = !{null, !10, !10}
+!22 = !{!23, !24}
+!23 = !DILocalVariable(name: "t2a", arg: 1, scope: !19, file: !1, line: 10, type: !10)
+!24 = !DILocalVariable(name: "t2b", arg: 2, scope: !19, file: !1, line: 10, type: !10)
+!25 = !DILocation(line: 10, column: 26, scope: !19)
+!26 = !DILocation(line: 10, column: 35, scope: !19)
+!27 = !DILocation(line: 11, column: 3, scope: !19)
+!28 = !DILocation(line: 13, column: 3, scope: !19)
+!29 = !DILocation(line: 15, column: 3, scope: !19)
+!30 = !DILocation(line: 16, column: 1, scope: !19)
+!31 = distinct !DISubprogram(name: "foo_same_param", scope: !1, file: !1, line: 18, type: !8, scopeLine: 18, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !32)
+!32 = !{!33, !34}
+!33 = !DILocalVariable(name: "t3a", arg: 1, scope: !31, file: !1, line: 18, type: !10)
+!34 = !DILocalVariable(name: "tmp", scope: !31, file: !1, line: 20, type: !10)
+!35 = !DILocation(line: 18, column: 25, scope: !31)
+!36 = !DILocation(line: 19, column: 3, scope: !31)
+!37 = !DILocation(line: 20, column: 7, scope: !31)
+!38 = !DILocation(line: 22, column: 3, scope: !31)
+!39 = !DILocation(line: 24, column: 3, scope: !31)
+!40 = !DILocation(line: 25, column: 1, scope: !31)

llvm/test/DebugInfo/X86/dbg-value-funcarg2.ll

+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -start-after=codegenprepare -stop-before=expand-isel-pseudos -o - %s | FileCheck %s
+
+; Test case was generated from the following C code,
+; using: clang -g -O1 -S -emit-llvm s.c -o s.ll
+;
+; struct s { long long int i, j; };
+;
+; extern void bar(struct s, struct s, struct s);
+;
+; int f(struct s s1, struct s s2) {
+;   volatile struct s tmp = {0};
+;   bar(s1, s2, tmp);
+;   s1.i = s2.i;
+;   s1.j = s2.j;
+;   return s1.j + s1.j;
+; }
+
+; Catch metadata references for involved variables.
+;
+; CHECK-DAG: ![[S1:.*]] = !DILocalVariable(name: "s1"
+; CHECK-DAG: ![[S2:.*]] = !DILocalVariable(name: "s2"
+
+define dso_local i32 @f(i64 %s1.coerce0, i64 %s1.coerce1, i64 %s2.coerce0, i64 %s2.coerce1) local_unnamed_addr #0 !dbg !7 {
+; We expect DBG_VALUE instructions for the arguments at the entry.
+; CHECK-LABEL: name:            f
+; CHECK-NOT: DBG_VALUE
+; CHECK-DAG: DBG_VALUE $rdi, $noreg, ![[S1]], !DIExpression(DW_OP_LLVM_fragment, 0, 64)
+; CHECK-DAG: DBG_VALUE $rsi, $noreg, ![[S1]], !DIExpression(DW_OP_LLVM_fragment, 64, 64)
+; CHECK-DAG: DBG_VALUE $rdx, $noreg, ![[S2]], !DIExpression(DW_OP_LLVM_fragment, 0, 64)
+; CHECK-DAG: DBG_VALUE $rcx, $noreg, ![[S2]], !DIExpression(DW_OP_LLVM_fragment, 64, 64)
+; CHECK-NOT: DBG_VALUE
+
+; Then arguments are copied to virtual registers.
+; CHECK-NOT: DBG_VALUE
+; CHECK-DAG: %[[R1:.*]]:gr64 = COPY $rcx
+; CHECK-DAG: DBG_VALUE %[[R1]], $noreg, ![[S2]], !DIExpression(DW_OP_LLVM_fragment, 64, 64)
+; CHECK-DAG: %[[R2:.*]]:gr64 = COPY $rdx
+; CHECK-DAG: DBG_VALUE %[[R2]], $noreg, ![[S2]], !DIExpression(DW_OP_LLVM_fragment, 0, 64)
+; CHECK-DAG: %[[R3:.*]]:gr64 = COPY $rsi
+; CHECK-DAG: DBG_VALUE %[[R3]], $noreg, ![[S1]], !DIExpression(DW_OP_LLVM_fragment, 64, 64)
+; CHECK-DAG: %[[R4:.*]]:gr64 = COPY $rdi
+; CHECK-DAG: DBG_VALUE %[[R4]], $noreg, ![[S1]], !DIExpression(DW_OP_LLVM_fragment, 0, 64)
+; CHECK-NOT: DBG_VALUE
+
+; We have the call to bar.
+; CHECK:     ADJCALLSTACKDOWN
+; CHECK:     CALL64pcrel32 @bar
+
+; After the call we expect to find new DBG_VALUE instructions for "s1".
+; CHECK:     ADJCALLSTACKUP
+; CHECK-NOT: DBG_VALUE
+; CHECK-DAG: DBG_VALUE %[[R2]], $noreg, ![[S1]], !DIExpression(DW_OP_LLVM_fragment, 0, 64)
+; CHECK-DAG: DBG_VALUE %[[R1]], $noreg, ![[S1]], !DIExpression(DW_OP_LLVM_fragment, 64, 64)
+
+; And then no more DBG_VALUE instructions before the add.
+; CHECK-NOT: DBG_VALUE
+; CHECK:     ADD32rr
+
+entry:
+  %tmp.sroa.0 = alloca i64, align 8
+  %tmp.sroa.4 = alloca i64, align 8
+  call void @llvm.dbg.declare(metadata i64* %tmp.sroa.0, metadata !19, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !21
+  call void @llvm.dbg.declare(metadata i64* %tmp.sroa.4, metadata !19, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !21
+  call void @llvm.dbg.value(metadata i64 %s1.coerce0, metadata !17, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !22
+  call void @llvm.dbg.value(metadata i64 %s1.coerce1, metadata !17, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !22
+  call void @llvm.dbg.value(metadata i64 %s2.coerce0, metadata !18, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !23
+  call void @llvm.dbg.value(metadata i64 %s2.coerce1, metadata !18, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !23
+  %tmp.sroa.0.0..sroa_cast = bitcast i64* %tmp.sroa.0 to i8*, !dbg !24
+  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %tmp.sroa.0.0..sroa_cast), !dbg !24
+  %tmp.sroa.4.0..sroa_cast = bitcast i64* %tmp.sroa.4 to i8*, !dbg !24
+  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %tmp.sroa.4.0..sroa_cast), !dbg !24
+  store volatile i64 0, i64* %tmp.sroa.0, align 8, !dbg !21
+  store volatile i64 0, i64* %tmp.sroa.4, align 8, !dbg !21
+  tail call void @bar(i64 %s1.coerce0, i64 %s1.coerce1, i64 %s2.coerce0, i64 %s2.coerce1, i64 0, i64 0) #4, !dbg !25
+  call void @llvm.dbg.value(metadata i64 %s2.coerce0, metadata !17, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !22
+  call void @llvm.dbg.value(metadata i64 %s2.coerce1, metadata !17, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !22
+  %s2.coerce1.tr = trunc i64 %s2.coerce1 to i32, !dbg !26
+  %conv = shl i32 %s2.coerce1.tr, 1, !dbg !26
+  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %tmp.sroa.0.0..sroa_cast), !dbg !27
+  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %tmp.sroa.4.0..sroa_cast), !dbg !27
+  ret i32 %conv, !dbg !28
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) #2
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) #2
+
+declare dso_local void @bar(i64, i64, i64, i64, i64, i64) local_unnamed_addr
+
+attributes #0 = { nounwind uwtable }
+attributes #1 = { nounwind readnone speculatable }
+attributes #2 = { argmemonly nounwind }
+attributes #4 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "llvm-svn @ 353529", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "s.c", directory: "")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"llvm-svn @ 353529"}
+!7 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 5, type: !8, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !16)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !11, !11}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s", file: !1, line: 1, size: 128, elements: !12)
+!12 = !{!13, !15}
+!13 = !DIDerivedType(tag: DW_TAG_member, name: "i", scope: !11, file: !1, line: 1, baseType: !14, size: 64)
+!14 = !DIBasicType(name: "long long int", size: 64, encoding: DW_ATE_signed)
+!15 = !DIDerivedType(tag: DW_TAG_member, name: "j", scope: !11, file: !1, line: 1, baseType: !14, size: 64, offset: 64)
+!16 = !{!17, !18, !19}
+!17 = !DILocalVariable(name: "s1", arg: 1, scope: !7, file: !1, line: 5, type: !11)
+!18 = !DILocalVariable(name: "s2", arg: 2, scope: !7, file: !1, line: 5, type: !11)
+!19 = !DILocalVariable(name: "tmp", scope: !7, file: !1, line: 6, type: !20)
+!20 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !11)
+!21 = !DILocation(line: 6, column: 21, scope: !7)
+!22 = !DILocation(line: 5, column: 16, scope: !7)
+!23 = !DILocation(line: 5, column: 29, scope: !7)
+!24 = !DILocation(line: 6, column: 3, scope: !7)
+!25 = !DILocation(line: 7, column: 3, scope: !7)
+!26 = !DILocation(line: 10, column: 10, scope: !7)
+!27 = !DILocation(line: 11, column: 1, scope: !7)
+!28 = !DILocation(line: 10, column: 3, scope: !7)