[SimplifyCFG] Remap rewritten debug intrinsic operands.

[rickyz]

Summary

FoldBranchToCommonDest clones instructions to a different basic block,
but handles debug intrinsics in a separate path. Previously, when
cloning debug intrinsics, their operands were not updated to reference
the correct cloned values. As a result, we would emit debug.value
intrinsics with broken operand references which are discarded in later
passes. This leads to incorrect debuginfo that reports incorrect values
for variables.

Fix this by remapping debug intrinsic operands when cloning them.

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

[rickyz]

Oops, forgot to write a commit message.

[rickyz]

Typo fix.

[rickyz]

One more commit message typo, sorry for the noise!

[vsk]

This looks correct to me. Please wait for another +1 before committing this.

llvm/test/DebugInfo/simplify-cfg-preserve-dbg-values.ll

116

It's probably safe to strip all (or most) of these attributes out, as they're unlikely to affect the test.

[rickyz]

Strip unneeded attributes from test.

[rickyz]

In D79602#2026367, @vsk wrote:

This looks correct to me. Please wait for another +1 before committing this.

Thanks! I'm not a committer, so someone else will probably need to commit this once it has the additional +1.

[djtodoro]

Thanks for this!
The change looks good to me, but I think we should trim the test a bit. Please find some advices below.

llvm/test/DebugInfo/simplify-cfg-preserve-dbg-values.ll

88

Can we get rid of using this?

91

This is unused, so please delete this.

104

Can we get rid of using this?

117

filename: "test.cc", directory: "/dir"

124

directory: /dir

137

I guess we don't need TBAA for this test.

138

Little trick to trim the test: you can use only one location for all other locations with the same scope, e.g.:

all !dbg !23, !dbg !24, !dbg !25, !dbg !26 replace with !dbg !22, and then delete these unused`DILocations`

[rickyz]

Minimize simplify-cfg-preserve-dbg-values.ll some more.

[rickyz]

llvm/test/DebugInfo/simplify-cfg-preserve-dbg-values.ll

117

I completely missed that these snuck in - thanks for catching!

138

Done - also removed the !range metadata, which didn't seem to be needed.

[djtodoro]

Looks good to me! Thanks!

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

PHIs[i]->setIncomingValueForBlock(PBI->getParent(), MergedCond);
     // We need to copy it to the new latch, that is, PBI.
     if (MDNode *LoopMD = BI->getMetadata(LLVMContext::MD_loop))
       PBI->setMetadata(LLVMContext::MD_loop, LoopMD);
 
     // TODO: If BB is reachable from all paths through PredBlock, then we
     // could replace PBI's branch probabilities with BI's.
 
     // Copy any debug value intrinsics into the end of PredBlock.
-    for (Instruction &I : *BB)
-      if (isa<DbgInfoIntrinsic>(I))
-        I.clone()->insertBefore(PBI);
+    for (Instruction &I : *BB) {
+      if (isa<DbgInfoIntrinsic>(I)) {
+        Instruction *NewI = I.clone();
+        RemapInstruction(NewI, VMap,
+                         RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
+        NewI->insertBefore(PBI);
+      }
+    }
 
     return true;
   }
   return false;
 }
 
 // If there is only one store in BB1 and BB2, return it, otherwise return
 // nullptr.

llvm/test/DebugInfo/simplify-cfg-preserve-dbg-values.ll

+; RUN: opt %s -simplifycfg -verify -S -o - | FileCheck %s
+;
+; Verify that SimplifyCFG does not invalidate operands for
+; llvm.dbg.value intrinsics.
+;
+; Reduced from the following example, compiled with
+; -O1 -g -fno-exceptions -fno-discard-value-names:
+;
+; void test() {
+;   bool a = false;
+;   bool b = false;
+;   bool c = false;
+;   [&a, &b, &c] { b = true; }();
+;
+;   if (a || b || c) {
+;     __builtin_debugtrap();
+;   }
+; }
+
+; CHECK: _Z4testv
+; CHECK-NOT: llvm.dbg.value(metadata !
+; CHECK: ret void
+
+%class.anon = type { i8*, i8*, i8* }
+
+; Function Attrs: nounwind uwtable
+define dso_local void @_Z4testv() local_unnamed_addr !dbg !7 {
+entry:
+  %a = alloca i8, align 1
+  %b = alloca i8, align 1
+  %c = alloca i8, align 1
+  %ref.tmp = alloca %class.anon, align 8
+  call void @llvm.dbg.value(metadata i8* %a, metadata !12, metadata !DIExpression(DW_OP_deref)), !dbg !16
+  call void @llvm.dbg.value(metadata i8 0, metadata !12, metadata !DIExpression()), !dbg !16
+  store i8 0, i8* %a, align 1, !dbg !16
+  call void @llvm.dbg.value(metadata i8* %b, metadata !14, metadata !DIExpression(DW_OP_deref)), !dbg !16
+  call void @llvm.dbg.value(metadata i8 0, metadata !14, metadata !DIExpression()), !dbg !16
+  store i8 0, i8* %b, align 1, !dbg !16
+  call void @llvm.dbg.value(metadata i8* %c, metadata !15, metadata !DIExpression(DW_OP_deref)), !dbg !16
+  call void @llvm.dbg.value(metadata i8 0, metadata !15, metadata !DIExpression()), !dbg !16
+  store i8 0, i8* %c, align 1, !dbg !16
+  %0 = bitcast %class.anon* %ref.tmp to i8*, !dbg !16
+  %1 = getelementptr inbounds %class.anon, %class.anon* %ref.tmp, i64 0, i32 0, !dbg !16
+  store i8* %a, i8** %1, align 8, !dbg !16
+  %2 = getelementptr inbounds %class.anon, %class.anon* %ref.tmp, i64 0, i32 1, !dbg !16
+  store i8* %b, i8** %2, align 8, !dbg !16
+  %3 = getelementptr inbounds %class.anon, %class.anon* %ref.tmp, i64 0, i32 2, !dbg !16
+  store i8* %c, i8** %3, align 8, !dbg !16
+  call fastcc void @"_ZZ4testvENK3$_0clEv"(%class.anon* nonnull %ref.tmp), !dbg !16
+  %4 = load i8, i8* %a, align 1, !dbg !17
+  call void @llvm.dbg.value(metadata i8 %4, metadata !12, metadata !DIExpression()), !dbg !16
+  %tobool = icmp eq i8 %4, 0, !dbg !17
+  br i1 %tobool, label %lor.lhs.false, label %if.then, !dbg !17
+
+lor.lhs.false:                                    ; preds = %entry
+  %5 = load i8, i8* %b, align 1, !dbg !17
+  call void @llvm.dbg.value(metadata i8 %5, metadata !14, metadata !DIExpression()), !dbg !16
+  %tobool1 = icmp eq i8 %5, 0, !dbg !17
+  br i1 %tobool1, label %lor.lhs.false2, label %if.then, !dbg !17
+
+lor.lhs.false2:                                   ; preds = %lor.lhs.false
+  %6 = load i8, i8* %c, align 1, !dbg !17
+  call void @llvm.dbg.value(metadata i8 %6, metadata !15, metadata !DIExpression()), !dbg !16
+  %tobool3 = icmp eq i8 %6, 0, !dbg !17
+  br i1 %tobool3, label %if.end, label %if.then, !dbg !16
+
+if.then:                                          ; preds = %lor.lhs.false2, %lor.lhs.false, %entry
+  call void @llvm.debugtrap(), !dbg !19
+  br label %if.end, !dbg !19
+
+if.end:                                           ; preds = %lor.lhs.false2, %if.then
+  call void @llvm.dbg.value(metadata i8* %c, metadata !15, metadata !DIExpression(DW_OP_deref)), !dbg !16
+  call void @llvm.dbg.value(metadata i8* %b, metadata !14, metadata !DIExpression(DW_OP_deref)), !dbg !16
+  call void @llvm.dbg.value(metadata i8* %a, metadata !12, metadata !DIExpression(DW_OP_deref)), !dbg !16
+  ret void, !dbg !16
+}
+
+; Function Attrs: inlinehint nounwind uwtable
+define internal fastcc void @"_ZZ4testvENK3$_0clEv"(%class.anon* %this) unnamed_addr align 2 !dbg !21 {
+entry:
+  call void @llvm.dbg.value(metadata %class.anon* %this, metadata !34, metadata !DIExpression()), !dbg !36
+  %0 = getelementptr inbounds %class.anon, %class.anon* %this, i32 0, i32 1, !dbg !36
+  %1 = load i8*, i8** %0, align 8, !dbg !36
+  store i8 1, i8* %1, align 1, !dbg !36
+  ret void, !dbg !36
+}
+
+; Function Attrs: nounwind
+declare void @llvm.debugtrap()
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 8.0.1-4 (tags/RELEASE_801/final)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "test.cc", directory: "/dir")
+!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 8.0.1-4 (tags/RELEASE_801/final)"}
+!7 = distinct !DISubprogram(name: "test", linkageName: "_Z4testv", scope: !8, file: !8, line: 1, type: !9, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+!8 = !DIFile(filename: "test.cc", directory: "/dir")
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !{!12, !14, !15}
+!12 = !DILocalVariable(name: "a", scope: !7, file: !8, line: 2, type: !13)
+!13 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
+!14 = !DILocalVariable(name: "b", scope: !7, file: !8, line: 3, type: !13)
+!15 = !DILocalVariable(name: "c", scope: !7, file: !8, line: 4, type: !13)
+!16 = !DILocation(line: 2, scope: !7)
+!17 = !DILocation(line: 7, scope: !18)
+!18 = distinct !DILexicalBlock(scope: !7, file: !8, line: 7)
+!19 = !DILocation(line: 8, scope: !20)
+!20 = distinct !DILexicalBlock(scope: !18, file: !8, line: 7)
+!21 = distinct !DISubprogram(name: "operator()", linkageName: "_ZZ4testvENK3$_0clEv", scope: !22, file: !8, line: 5, type: !28, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, declaration: !32, retainedNodes: !33)
+!22 = distinct !DICompositeType(tag: DW_TAG_class_type, scope: !7, file: !8, line: 5, size: 192, flags: DIFlagTypePassByValue, elements: !23)
+!23 = !{!24, !26, !27}
+!24 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !22, file: !8, line: 5, baseType: !25, size: 64)
+!25 = !DIDerivedType(tag: DW_TAG_reference_type, baseType: !13, size: 64)
+!26 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !22, file: !8, line: 5, baseType: !25, size: 64, offset: 64)
+!27 = !DIDerivedType(tag: DW_TAG_member, name: "c", scope: !22, file: !8, line: 5, baseType: !25, size: 64, offset: 128)
+!28 = !DISubroutineType(types: !29)
+!29 = !{null, !30}
+!30 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !31, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!31 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !22)
+!32 = !DISubprogram(name: "operator()", scope: !22, file: !8, line: 5, type: !28, scopeLine: 5, flags: DIFlagPublic | DIFlagPrototyped, spFlags: DISPFlagOptimized)
+!33 = !{!34}
+!34 = !DILocalVariable(name: "this", arg: 1, scope: !21, type: !35, flags: DIFlagArtificial | DIFlagObjectPointer)
+!35 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !31, size: 64)
+!36 = !DILocation(line: 0, scope: !21)