[DebugInfo][DAG] Don't reuse debug location on COPY if width changes.

[tbosch]

Summary

This caused incorrect debug information for parameters:
Previously, after a COPY of a parameter that changes the width,
we would emit a DBG_VALUE that continues to be associated to that
parameter, even though it now used a different width.
This made the LiveDebugValues pass assume the parameter value
got clobbered and it stopped tracking the parameter entry
value, leading to incorrect debug information.

Fixes https://bugs.llvm.org/show_bug.cgi?id=39715

[vsk]

Thanks for this! The discussion on the bugzilla PR covers some alternative ways to tackle this, which I won’t rehash here. I think it's reasonable to shoot for a narrow bugfix first.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

624

I missed this earlier, but: with this change, if the first copy use fails the width check, we'd emit a debug value if there’s a second copy use of the register.

A narrow fix might be to move the width check to line 629, where we check whether CopyUseMI has been set and emit a debug value if so.

[tbosch]

Do the width check at the right time

As suggested by vsk

[vsk]

Lgtm.

[MaskRay]

You can drop Reviewers: Subscribers: Tags: and the text Summary: with the following script

arcfilter () {
        arc amend
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people. You should keep the tag. (I have updated my script to use --date=now (setting author date to committer date))

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

if (LDI != LiveInMap.end()) {
            UI = RegInfo->use_instr_begin(LDI->second),
            E = RegInfo->use_instr_end(); UI != E; ) {
         MachineInstr *UseMI = &*(UI++);
         if (UseMI->isDebugValue()) continue;
         if (UseMI->isCopy() && !CopyUseMI && UseMI->getParent() == EntryMBB) {
           CopyUseMI = UseMI; continue;
         }
         // Otherwise this is another use or second copy use.
         CopyUseMI = nullptr; break;
       }
-      if (CopyUseMI) {
+      if (CopyUseMI &&
+          TRI.getRegSizeInBits(LDI->second, MRI) ==
+              TRI.getRegSizeInBits(CopyUseMI->getOperand(0).getReg(), MRI)) {
         // Use MI's debug location, which describes where Variable was
         // declared, rather than whatever is attached to CopyUseMI.
         MachineInstr *NewMI =
             BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,
                     CopyUseMI->getOperand(0).getReg(), Variable, Expr);
         MachineBasicBlock::iterator Pos = CopyUseMI;
         EntryMBB->insertAfter(Pos, NewMI);
       }

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

+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -start-after=codegenprepare -stop-before=finalize-isel -o - %s | FileCheck %s
+
+; Input to this test looked like this and was compiled using: clang -g -O1 -mllvm -stop-after=codegenprepare -S
+;
+;    int fn1(long t1) {
+;      return t;
+;    }
+;
+
+; Catch metadata references for involved variables.
+;
+; CHECK-DAG: ![[T1:.*]] = !DILocalVariable(name: "t1"
+
+
+define dso_local i32 @fn1(i64 %t1) local_unnamed_addr #0 !dbg !7 {
+; We expect that the same width COPY reuses the debug location,
+; but the width narrowing COPY does not.
+; 
+; CHECK-LABEL: name:            fn1
+; CHECK: DBG_VALUE $rdi, $noreg, ![[T1]], !DIExpression(),
+; CHECK-NEXT: %0:gr64 = COPY $rdi
+; CHECK-NEXT: DBG_VALUE %0, $noreg, ![[T1]], !DIExpression(),
+; CHECK-NEXT: %1:gr32 = COPY %0.sub_32bit
+; CHECK-NEXT: COPY
+; CHECK-NEXT: RET
+entry:
+  call void @llvm.dbg.value(metadata i64 %t1, metadata !13, metadata !DIExpression()), !dbg !14
+  %0 = trunc i64 %t1 to i32, !dbg !15
+  ret i32 %0, !dbg !16
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { norecurse nounwind readnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 11.0.0 ([e-mail]:tbosch/llvm-project.git 0b11aed869bf09ba60
+d7ed17334cf0b76e6a5922)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cc", directory: "")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 11.0.0 ([e-mail]:tbosch/llvm-project.git 0b11aed869bf09ba60d7ed17334cf0b76e6a5922)"}
+!7 = distinct !DISubprogram(name: "fn1", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !12)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !11}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
+!12 = !{!13}
+!13 = !DILocalVariable(name: "t1", arg: 1, scope: !7, file: !1, line: 1, type: !11)
+!14 = !DILocation(line: 0, scope: !7)
+!15 = !DILocation(line: 2, column: 10, scope: !7)
+!16 = !DILocation(line: 2, column: 3, scope: !7)