[Assignment Tracking][15/*] Account for assignment tracking in simplifycfg
authorOCHyams <orlando.hyams@sony.com>
Fri, 18 Nov 2022 10:07:30 +0000 (10:07 +0000)
committerOCHyams <orlando.hyams@sony.com>
Fri, 18 Nov 2022 10:15:55 +0000 (10:15 +0000)
The Assignment Tracking debug-info feature is outlined in this RFC:

https://discourse.llvm.org/t/
rfc-assignment-tracking-a-better-way-of-specifying-variable-locations-in-ir

Update simplifycfg:
sinkLastInstruction - preserve debug use-before-defs.

SpeculativelyExecuteBB - replace the value component of dbg.assign intrinsics
when stores are hoisted and merged using a select, and don't delete them.

Reviewed By: jmorse

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

llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/empty-block.ll [new file with mode: 0644]
llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/speculated-store.ll [new file with mode: 0644]

index ee7f8b2..b1f301c 100644 (file)
@@ -41,6 +41,7 @@
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
@@ -1999,9 +2000,15 @@ static bool sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
   }
 
   // Finally nuke all instructions apart from the common instruction.
-  for (auto *I : Insts)
-    if (I != I0)
-      I->eraseFromParent();
+  for (auto *I : Insts) {
+    if (I == I0)
+      continue;
+    // The remaining uses are debug users, replace those with the common inst.
+    // In most (all?) cases this just introduces a use-before-def.
+    assert(I->user_empty() && "Inst unexpectedly still has non-dbg users");
+    I->replaceAllUsesWith(I0);
+    I->eraseFromParent();
+  }
 
   return true;
 }
@@ -2969,6 +2976,7 @@ bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB,
   // Insert a select of the value of the speculated store.
   if (SpeculatedStoreValue) {
     IRBuilder<NoFolder> Builder(BI);
+    Value *OrigV = SpeculatedStore->getValueOperand();
     Value *TrueV = SpeculatedStore->getValueOperand();
     Value *FalseV = SpeculatedStoreValue;
     if (Invert)
@@ -2978,6 +2986,35 @@ bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB,
     SpeculatedStore->setOperand(0, S);
     SpeculatedStore->applyMergedLocation(BI->getDebugLoc(),
                                          SpeculatedStore->getDebugLoc());
+    // The value stored is still conditional, but the store itself is now
+    // unconditonally executed, so we must be sure that any linked dbg.assign
+    // intrinsics are tracking the new stored value (the result of the
+    // select). If we don't, and the store were to be removed by another pass
+    // (e.g. DSE), then we'd eventually end up emitting a location describing
+    // the conditional value, unconditionally.
+    //
+    // === Before this transformation ===
+    // pred:
+    //   store %one, %x.dest, !DIAssignID !1
+    //   dbg.assign %one, "x", ..., !1, ...
+    //   br %cond if.then
+    //
+    // if.then:
+    //   store %two, %x.dest, !DIAssignID !2
+    //   dbg.assign %two, "x", ..., !2, ...
+    //
+    // === After this transformation ===
+    // pred:
+    //   store %one, %x.dest, !DIAssignID !1
+    //   dbg.assign %one, "x", ..., !1
+    ///  ...
+    //   %merge = select %cond, %two, %one
+    //   store %merge, %x.dest, !DIAssignID !2
+    //   dbg.assign %merge, "x", ..., !2
+    for (auto *DAI : at::getAssignmentMarkers(SpeculatedStore)) {
+      if (any_of(DAI->location_ops(), [&](Value *V) { return V == OrigV; }))
+        DAI->replaceVariableLocationOp(OrigV, S);
+    }
   }
 
   // Metadata can be dependent on the condition we are hoisting above.
@@ -2987,8 +3024,11 @@ bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB,
   // Similarly strip attributes that maybe dependent on condition we are
   // hoisting above.
   for (auto &I : make_early_inc_range(*ThenBB)) {
-    if (!SpeculatedStoreValue || &I != SpeculatedStore)
-      I.setDebugLoc(DebugLoc());
+    if (!SpeculatedStoreValue || &I != SpeculatedStore) {
+      // Don't update the DILocation of dbg.assign intrinsics.
+      if (!isa<DbgAssignIntrinsic>(&I))
+        I.setDebugLoc(DebugLoc());
+    }
     I.dropUndefImplyingAttrsAndUnknownMetadata();
 
     // Drop ephemeral values.
@@ -3028,8 +3068,12 @@ bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB,
   // Remove speculated dbg intrinsics.
   // FIXME: Is it possible to do this in a more elegant way? Moving/merging the
   // dbg value for the different flows and inserting it after the select.
-  for (Instruction *I : SpeculatedDbgIntrinsics)
-    I->eraseFromParent();
+  for (Instruction *I : SpeculatedDbgIntrinsics) {
+    // We still want to know that an assignment took place so don't remove
+    // dbg.assign intrinsics.
+    if (!isa<DbgAssignIntrinsic>(I))
+      I->eraseFromParent();
+  }
 
   ++NumSpeculations;
   return true;
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/empty-block.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/empty-block.ll
new file mode 100644 (file)
index 0000000..423fdcb
--- /dev/null
@@ -0,0 +1,132 @@
+; RUN: opt -S %s -passes=simplifycfg -o - -experimental-assignment-tracking \
+; RUN: | FileCheck %s
+
+;; $ cat test.cpp
+;; class a {};
+;; void operator*(a, float &);
+;; class b {
+;; public:
+;;   a c;
+;; };
+;; int d;
+;; class e {
+;;   b g[3];
+;;   float f;
+;;   void i();
+;; };
+;; void e::i() {
+;;   float h;
+;;   g[d].c *h;
+;;   if (h)
+;;     h = f;
+;;   else
+;;     h = f;
+;; }
+;; Generated by grabbing IR before simplifycfg in:
+;; $ clang++ -O2 -g -c test.cpp -Xclang -fexperimental-assignment-tracking
+
+;; if.then and if.else each only have a dbg.assign and br instruction.
+;; SimplifyCFG will remove these blocks. Check that the dbg.assign intrinsics
+;; are sunk into the succ beforehand.
+
+; CHECK: entry:
+;; -- alloca dbg.assign
+; CHECK: call void @llvm.dbg.assign(metadata i1 undef
+;; -- sunk dbg.assigns
+; CHECK: call void @llvm.dbg.assign(metadata float undef, metadata ![[var:[0-9]+]], metadata !DIExpression(), metadata ![[id:[0-9]+]], metadata ptr %h, metadata !DIExpression()), !dbg
+; CHECK-NEXT: call void @llvm.dbg.assign(metadata float undef, metadata ![[var]], metadata !DIExpression(), metadata ![[id]], metadata ptr %h, metadata !DIExpression()), !dbg
+; CHECK-NEXT: %storemerge.in = getelementptr
+; CHECK-NEXT: %storemerge = load float
+; CHECK-NEXT: store float %storemerge, ptr %h, align 4{{.+}}!DIAssignID ![[id]]
+; CHECK: ret void
+
+%class.e = type { [3 x %class.b], float }
+%class.b = type { %class.a }
+%class.a = type { i8 }
+
+@d = dso_local local_unnamed_addr global i32 0, align 4, !dbg !0
+
+; Function Attrs: uwtable
+define dso_local void @_ZN1e1iEv(ptr %this) local_unnamed_addr #0 align 2 !dbg !11 {
+entry:
+  %h = alloca float, align 4, !DIAssignID !32
+  call void @llvm.dbg.assign(metadata i1 undef, metadata !31, metadata !DIExpression(), metadata !32, metadata ptr %h, metadata !DIExpression()), !dbg !33
+  %0 = bitcast ptr %h to ptr, !dbg !34
+  call void @llvm.lifetime.start.p0i8(i64 4, ptr nonnull %0) #4, !dbg !34
+  call void @_Zml1aRf(ptr nonnull align 4 dereferenceable(4) %h), !dbg !35
+  %1 = load float, ptr %h, align 4, !dbg !36
+  %tobool = fcmp une float %1, 0.000000e+00, !dbg !36
+  br i1 %tobool, label %if.then, label %if.else, !dbg !42
+
+if.then:                                          ; preds = %entry
+  call void @llvm.dbg.assign(metadata float undef, metadata !31, metadata !DIExpression(), metadata !43, metadata ptr %h, metadata !DIExpression()), !dbg !33
+  br label %if.end, !dbg !44
+
+if.else:                                          ; preds = %entry
+  call void @llvm.dbg.assign(metadata float undef, metadata !31, metadata !DIExpression(), metadata !43, metadata ptr %h, metadata !DIExpression()), !dbg !33
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  %storemerge.in = getelementptr inbounds %class.e, ptr %this, i64 0, i32 1, !dbg !45
+  %storemerge = load float, ptr %storemerge.in, align 4, !dbg !45
+  store float %storemerge, ptr %h, align 4, !dbg !45, !DIAssignID !43
+  call void @llvm.lifetime.end.p0i8(i64 4, ptr nonnull %0) #4, !dbg !48
+  ret void, !dbg !48
+}
+
+declare void @llvm.lifetime.start.p0i8(i64 immarg, ptr nocapture) #1
+declare !dbg !49 dso_local void @_Zml1aRf(ptr nonnull align 4 dereferenceable(4)) local_unnamed_addr #2
+declare void @llvm.lifetime.end.p0i8(i64 immarg, ptr nocapture) #1
+declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata, metadata) #3
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8, !9}
+!llvm.ident = !{!10}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "d", scope: !2, file: !3, line: 7, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "test.cpp", directory: "/")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{i32 7, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{i32 1, !"wchar_size", i32 4}
+!10 = !{!"clang version 12.0.0"}
+!11 = distinct !DISubprogram(name: "i", linkageName: "_ZN1e1iEv", scope: !12, file: !3, line: 13, type: !25, scopeLine: 13, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, declaration: !24, retainedNodes: !28)
+!12 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "e", file: !3, line: 8, size: 64, flags: DIFlagTypePassByValue, elements: !13, identifier: "_ZTS1e")
+!13 = !{!14, !22, !24}
+!14 = !DIDerivedType(tag: DW_TAG_member, name: "g", scope: !12, file: !3, line: 9, baseType: !15, size: 24)
+!15 = !DICompositeType(tag: DW_TAG_array_type, baseType: !16, size: 24, elements: !20)
+!16 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "b", file: !3, line: 3, size: 8, flags: DIFlagTypePassByValue, elements: !17, identifier: "_ZTS1b")
+!17 = !{!18}
+!18 = !DIDerivedType(tag: DW_TAG_member, name: "c", scope: !16, file: !3, line: 5, baseType: !19, size: 8, flags: DIFlagPublic)
+!19 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "a", file: !3, line: 1, size: 8, flags: DIFlagTypePassByValue, elements: !4, identifier: "_ZTS1a")
+!20 = !{!21}
+!21 = !DISubrange(count: 3)
+!22 = !DIDerivedType(tag: DW_TAG_member, name: "f", scope: !12, file: !3, line: 10, baseType: !23, size: 32, offset: 32)
+!23 = !DIBasicType(name: "float", size: 32, encoding: DW_ATE_float)
+!24 = !DISubprogram(name: "i", linkageName: "_ZN1e1iEv", scope: !12, file: !3, line: 11, type: !25, scopeLine: 11, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+!25 = !DISubroutineType(types: !26)
+!26 = !{null, !27}
+!27 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !12, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!28 = !{!29, !31}
+!29 = !DILocalVariable(name: "this", arg: 1, scope: !11, type: !30, flags: DIFlagArtificial | DIFlagObjectPointer)
+!30 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !12, size: 64)
+!31 = !DILocalVariable(name: "h", scope: !11, file: !3, line: 14, type: !23)
+!32 = distinct !DIAssignID()
+!33 = !DILocation(line: 0, scope: !11)
+!34 = !DILocation(line: 14, column: 3, scope: !11)
+!35 = !DILocation(line: 15, column: 10, scope: !11)
+!36 = !DILocation(line: 16, column: 7, scope: !37)
+!37 = distinct !DILexicalBlock(scope: !11, file: !3, line: 16, column: 7)
+!42 = !DILocation(line: 16, column: 7, scope: !11)
+!43 = distinct !DIAssignID()
+!44 = !DILocation(line: 17, column: 5, scope: !37)
+!45 = !DILocation(line: 0, scope: !37)
+!48 = !DILocation(line: 20, column: 1, scope: !11)
+!49 = !DISubprogram(name: "operator*", linkageName: "_Zml1aRf", scope: !3, file: !3, line: 2, type: !50, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !4)
+!50 = !DISubroutineType(types: !51)
+!51 = !{null, !19, !52}
+!52 = !DIDerivedType(tag: DW_TAG_reference_type, baseType: !23, size: 64)
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/speculated-store.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/speculated-store.ll
new file mode 100644 (file)
index 0000000..afb9b70
--- /dev/null
@@ -0,0 +1,86 @@
+; RUN: opt -passes=simplifycfg %s -S -experimental-assignment-tracking  \
+; RUN: | FileCheck %s
+
+;; Ensure that we correctly update the value component of dbg.assign intrinsics
+;; after merging a conditional block with a store its the predecessor. The
+;; value stored is still conditional, but the store itself is now
+;; unconditionally run, so we must be sure that any linked dbg.assign intrinsics
+;; are tracking the new stored value (the result of the select). If we don't,
+;; and the store were to be removed by another pass (e.g. DSE), then we'd
+;; eventually end up emitting a location describing the conditional value,
+;; unconditionally.
+
+;; Created from the following source and command, with dbg.assign and DIAssignID
+;; metadata added and some other metadata removed by hand:
+;; $ cat test.c
+;; int a;
+;; void b() {
+;;   int c = 0;
+;;   if (a)
+;;      c = 1;
+;; }
+;; $ clang -O2 -g -emit-llvm -S test.c -Xclang -fexperimental-assignment-tracking
+
+; CHECK: %[[SELECT:.*]] = select i1 %tobool
+; CHECK-NEXT: store i32 %[[SELECT]], ptr %c{{.*}}, !DIAssignID ![[ID:[0-9]+]]
+; CHECK-NEXT: call void @llvm.dbg.assign(metadata i32 %[[SELECT]], metadata ![[VAR_C:[0-9]+]], metadata !DIExpression(), metadata ![[ID]], metadata ptr %c, metadata !DIExpression()), !dbg
+; CHECK: ![[VAR_C]] = !DILocalVariable(name: "c",
+
+@a = dso_local global i32 0, align 4, !dbg !0
+
+define dso_local void @b() !dbg !11 {
+entry:
+  %c = alloca i32, align 4
+  %0 = bitcast ptr %c to ptr, !dbg !16
+  call void @llvm.lifetime.start.p0i8(i64 4, ptr %0), !dbg !16
+  store i32 0, ptr %c, align 4, !dbg !17, !DIAssignID !36
+  call void @llvm.dbg.assign(metadata i32 0, metadata !15, metadata !DIExpression(), metadata !36, metadata ptr %c, metadata !DIExpression()), !dbg !17
+  %1 = load i32, ptr @a, align 4, !dbg !22
+  %tobool = icmp ne i32 %1, 0, !dbg !22
+  br i1 %tobool, label %if.then, label %if.end, !dbg !24
+
+if.then:                                          ; preds = %entry
+  store i32 1, ptr %c, align 4, !dbg !25, !DIAssignID !37
+  call void @llvm.dbg.assign(metadata i32 1, metadata !15, metadata !DIExpression(), metadata !37, metadata ptr %c, metadata !DIExpression()), !dbg !17
+  br label %if.end, !dbg !26
+
+if.end:                                           ; preds = %if.then, %entry
+  %2 = bitcast ptr %c to ptr, !dbg !27
+  call void @llvm.lifetime.end.p0i8(i64 4, ptr %2), !dbg !27
+  ret void, !dbg !27
+}
+
+declare void @llvm.lifetime.start.p0i8(i64 immarg, ptr nocapture)
+declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata, metadata)
+declare void @llvm.lifetime.end.p0i8(i64 immarg, ptr nocapture)
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8, !9}
+!llvm.ident = !{!10}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !3, line: 1, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 14.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "test.c", directory: "/")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{i32 7, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{i32 1, !"wchar_size", i32 4}
+!10 = !{!"clang version 12.0.0"}
+!11 = distinct !DISubprogram(name: "b", scope: !3, file: !3, line: 2, type: !12, scopeLine: 2, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !14)
+!12 = !DISubroutineType(types: !13)
+!13 = !{null}
+!14 = !{!15}
+!15 = !DILocalVariable(name: "c", scope: !11, file: !3, line: 3, type: !6)
+!16 = !DILocation(line: 3, column: 3, scope: !11)
+!17 = !DILocation(line: 3, column: 7, scope: !11)
+!22 = !DILocation(line: 4, column: 7, scope: !23)
+!23 = distinct !DILexicalBlock(scope: !11, file: !3, line: 4, column: 7)
+!24 = !DILocation(line: 4, column: 7, scope: !11)
+!25 = !DILocation(line: 5, column: 7, scope: !23)
+!26 = !DILocation(line: 5, column: 5, scope: !23)
+!27 = !DILocation(line: 6, column: 1, scope: !11)
+!36 = distinct !DIAssignID()
+!37 = distinct !DIAssignID()