[Assignment Tracking] Trunc fragments for stores to vars smaller than the alloca

[Orlando]

Summary

In D147777 emitDbgAssign was fixed to discard assignments which touched any bits outside the bounds of a variable. This patch changes emitDbgAssign to discard assignments which touch bits only outside the variable bounds, and creates a truncated fragment expression for stores partially overlapping the variable. This is necessary because the alloca is interpreted as a store (of undef), meaning without this patch emitDbgAssign would discard the inital dbg.assign for a variable that is smaller than the alloca.

[jmorse]

LGTM with some fairly standard nits

llvm/lib/IR/DebugInfo.cpp

1854–1865

Can we assert this, for when it likely changes in the future and we forget this code?

1860

Seeing how both inputs are unsigned and one argument is zero, isn't this guaranteed to always be FragStartBit? (Could produce a warning somewhere)

llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/var-not-alloca-sized.ll

5–7

"looks right" is a bit too vague when someone has to update this test in the future, spelling it out a bit more would be better.

[Orlando]

Thanks

+ Address nits + respond in-line

llvm/lib/IR/DebugInfo.cpp

1854–1865

That info would come from the VarRecord which doesn't yet support any kind of offset. We could add a "dummy" field to that struct and then assert that it is zero here, but I'm not sure that's much better than the VarStartBit = 0; below. wdyt?

1860

Yeah that's right, I was setting up for a future where it's not always zero. But good point about potential warning. I've replaced the line with a FIXME.

[jmorse]

LGTM

llvm/lib/IR/DebugInfo.cpp

1854–1865

Ehhh, yeah, that's still going to require us to "just remember" what to change. It doesn't sound like adding an assertion will help.

1860

cool cool cool

llvm/lib/IR/DebugInfo.cpp

 /// Returns nullptr if the assignment shouldn't be attributed to this variable.
 static CallInst *emitDbgAssign(AssignmentInfo Info, Value *Val, Value *Dest,
                                Instruction &StoreLikeInst,
                                const VarRecord &VarRec, DIBuilder &DIB) {
   auto *ID = StoreLikeInst.getMetadata(LLVMContext::MD_DIAssignID);
   assert(ID && "Store instruction must have DIAssignID metadata");
   (void)ID;
 
+  const uint64_t StoreStartBit = Info.OffsetInBits;
+  const uint64_t StoreEndBit = Info.OffsetInBits + Info.SizeInBits;
+
+  uint64_t FragStartBit = StoreStartBit;
+  uint64_t FragEndBit = StoreEndBit;
+
   bool StoreToWholeVariable = Info.StoreToWholeAlloca;
   if (auto Size = VarRec.Var->getSizeInBits()) {
-    // Discard stores to bits outside this variable. NOTE: trackAssignments
-    // doesn't understand base expressions yet, so all variables that reach
-    // here are guaranteed to start at offset 0 in the alloca.
-    // TODO: Could we truncate the fragment expression instead of discarding
-    // the assignment?
-    if (Info.OffsetInBits + Info.SizeInBits > *Size)
+    // NOTE: trackAssignments doesn't understand base expressions yet, so all
+    // variables that reach here are guaranteed to start at offset 0 in the
+    // alloca.
+    const uint64_t VarStartBit = 0;
+    const uint64_t VarEndBit = *Size;
+
+    // FIXME: trim FragStartBit when nonzero VarStartBit is supported.
+    FragEndBit = std::min(FragEndBit, VarEndBit);
+
+    // Discard stores to bits outside this variable.
+    if (FragStartBit >= FragEndBit)
       return nullptr;
-    // FIXME: As noted above - only variables at offset 0 are handled
-    // currently.
-    StoreToWholeVariable = Info.OffsetInBits == /*VarOffsetInAlloca*/ 0 &&
-                           Info.SizeInBits == *Size;
+
+    StoreToWholeVariable = FragStartBit <= VarStartBit && FragEndBit >= *Size;
   }
 
   DIExpression *Expr =
       DIExpression::get(StoreLikeInst.getContext(), std::nullopt);
   if (!StoreToWholeVariable) {
-    auto R = DIExpression::createFragmentExpression(Expr, Info.OffsetInBits,
-                                                    Info.SizeInBits);
+    auto R = DIExpression::createFragmentExpression(Expr, FragStartBit,
+                                                    FragEndBit - FragStartBit);
     assert(R.has_value() && "failed to create fragment expression");
     Expr = *R;
   }
   DIExpression *AddrExpr =
       DIExpression::get(StoreLikeInst.getContext(), std::nullopt);
   return DIB.insertDbgAssign(&StoreLikeInst, Val, VarRec.Var, Expr, Dest,
                              AddrExpr, VarRec.DL);
 }

llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/structured-bindings.ll

+; RUN: opt -passes=declare-to-assign -S %s -o - | FileCheck %s
+
+;; Check assignment tracking debug info for structured bindings. FIXME only
+;; variables at offset 0 in the backing alloca are currently tracked with the
+;; feature.
+
+;; struct two { int a, b; };
+;; two get();
+;; int fun() {
+;;   auto [a,b] = get();
+;;   return a * b;
+;; }
+
+; CHECK: %0 = alloca %struct.two, align 4, !DIAssignID ![[ID1:[0-9]+]]
+; CHECK-NEXT: llvm.dbg.assign(metadata i1 undef, metadata ![[AGGR:[0-9]+]], metadata !DIExpression(), metadata ![[ID1]], metadata ptr %0, metadata !DIExpression())
+; CHECK-NEXT: llvm.dbg.assign(metadata i1 undef, metadata ![[A:[0-9]+]], metadata !DIExpression(), metadata ![[ID1]], metadata ptr %0, metadata !DIExpression())
+; CHECK-NEXT: llvm.dbg.declare(metadata ptr %0, metadata ![[B:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 4))
+
+; CHECK: store i64 %call, ptr %0, align 4,{{.*}}, !DIAssignID ![[ID2:[0-9]+]]
+; CHECK-NEXT: llvm.dbg.assign(metadata i64 %call, metadata ![[AGGR]], metadata !DIExpression(), metadata ![[ID2]], metadata ptr %0, metadata !DIExpression())
+; CHECK-NEXT: llvm.dbg.assign(metadata i64 %call, metadata ![[A]], metadata !DIExpression(), metadata ![[ID2]], metadata ptr %0, metadata !DIExpression())
+
+; CHECK: ![[AGGR]] = !DILocalVariable(scope:
+; CHECK: ![[A]] = !DILocalVariable(name: "a", scope:
+; CHECK: ![[B]] = !DILocalVariable(name: "b", scope:
+
+%struct.two = type { i32, i32 }
+
+define dso_local noundef i32 @_Z3funv() #0 !dbg !10 {
+entry:
+  %0 = alloca %struct.two, align 4
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !15, metadata !DIExpression()), !dbg !16
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !17, metadata !DIExpression(DW_OP_plus_uconst, 4)), !dbg !18
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !19, metadata !DIExpression()), !dbg !24
+  %call = call i64 @_Z3getv(), !dbg !25
+  store i64 %call, ptr %0, align 4, !dbg !25
+  %a = getelementptr inbounds %struct.two, ptr %0, i32 0, i32 0, !dbg !16
+  %1 = load i32, ptr %a, align 4, !dbg !26
+  %b = getelementptr inbounds %struct.two, ptr %0, i32 0, i32 1, !dbg !18
+  %2 = load i32, ptr %b, align 4, !dbg !27
+  %mul = mul nsw i32 %1, %2, !dbg !28
+  ret i32 %mul, !dbg !29
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+declare i64 @_Z3getv()
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 17.0.0)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "/")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 17.0.0)"}
+!10 = distinct !DISubprogram(name: "fun", linkageName: "_Z3funv", scope: !1, file: !1, line: 3, type: !11, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13}
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !{}
+!15 = !DILocalVariable(name: "a", scope: !10, file: !1, line: 4, type: !13)
+!16 = !DILocation(line: 4, column: 9, scope: !10)
+!17 = !DILocalVariable(name: "b", scope: !10, file: !1, line: 4, type: !13)
+!18 = !DILocation(line: 4, column: 11, scope: !10)
+!19 = !DILocalVariable(scope: !10, file: !1, line: 4, type: !20)
+!20 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "two", file: !1, line: 1, size: 64, flags: DIFlagTypePassByValue, elements: !21, identifier: "_ZTS3two")
+!21 = !{!22, !23}
+!22 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !20, file: !1, line: 1, baseType: !13, size: 32)
+!23 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !20, file: !1, line: 1, baseType: !13, size: 32, offset: 32)
+!24 = !DILocation(line: 4, column: 8, scope: !10)
+!25 = !DILocation(line: 4, column: 16, scope: !10)
+!26 = !DILocation(line: 5, column: 10, scope: !10)
+!27 = !DILocation(line: 5, column: 14, scope: !10)
+!28 = !DILocation(line: 5, column: 12, scope: !10)
+!29 = !DILocation(line: 5, column: 3, scope: !10)

llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/var-not-alloca-sized.ll

+; RUN: opt -passes=declare-to-assign -S %s -o - | FileCheck %s
+
+;; The variable doesn't fill the whole alloca which has a range of different
+;; sized stores to it, overlapping (or not) the variable in various ways. Check
+;; the fragment is truncated to represent the intersect between the store and
+;; the variable. If that intersect has exactly the same offset and size as the
+;; variable then a fragment should not be produced (the whole variable is
+;; covered by the store).
+;;
+;; Check directives written inline.
+
+%struct.two = type { i32, i32 }
+
+define dso_local noundef i32 @_Z3funv() #0 !dbg !10 {
+entry:
+  %0 = alloca [4 x i16], align 4
+  call void @llvm.dbg.declare(metadata ptr %0, metadata !15, metadata !DIExpression()), !dbg !16
+; CHECK: %0 = alloca [4 x i16], align 4, !DIAssignID ![[ID1:[0-9]+]]
+; CHECK-NEXT: llvm.dbg.assign(metadata i1 undef, metadata ![[#]], metadata !DIExpression(), metadata ![[ID1]], metadata ptr %0, metadata !DIExpression())
+  %a = getelementptr inbounds [4 x i16], ptr %0, i32 0, i32 0
+  %a.5 = getelementptr inbounds [4 x i16], ptr %0, i32 0, i32 1
+  %b = getelementptr inbounds [4 x i16], ptr %0, i32 0, i32 2
+  store i64 1, ptr %a, align 4
+; CHECK: store i64 1, ptr %a, align 4, !DIAssignID ![[ID2:[0-9]+]]
+; CHECK-NEXT: llvm.dbg.assign(metadata i64 1, metadata ![[#]], metadata !DIExpression(), metadata ![[ID2]], metadata ptr %a, metadata !DIExpression())
+  store i64 2, ptr %b, align 4
+;; %b is outside the variable bounds, no debug intrinsic needed.
+  store i16 3, ptr %a.5, align 4
+; CHECK: store i16 3, ptr %a.5, align 4, !DIAssignID ![[ID3:[0-9]+]]
+; CHECK-NEXT: llvm.dbg.assign(metadata i16 3, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_fragment, 16, 16), metadata ![[ID3]], metadata ptr %a.5, metadata !DIExpression())
+  store i32 4, ptr %a.5, align 4
+; CHECK: store i32 4, ptr %a.5, align 4, !DIAssignID ![[ID4:[0-9]+]]
+; CHECK-NEXT: llvm.dbg.assign(metadata i32 4, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_fragment, 16, 16), metadata ![[ID4]], metadata ptr %a.5, metadata !DIExpression())
+  store i32 5, ptr %a, align 4
+; CHECK: store i32 5, ptr %a, align 4, !DIAssignID ![[ID5:[0-9]+]]
+; CHECK-NEXT: llvm.dbg.assign(metadata i32 5, metadata ![[#]], metadata !DIExpression(), metadata ![[ID5]], metadata ptr %a, metadata !DIExpression())
+  ret i32 0
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+declare i64 @_Z3getv()
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 17.0.0)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "/")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 17.0.0)"}
+!10 = distinct !DISubprogram(name: "fun", linkageName: "_Z3funv", scope: !1, file: !1, line: 3, type: !11, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13}
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !{}
+!15 = !DILocalVariable(name: "a", scope: !10, file: !1, line: 4, type: !13)
+!16 = !DILocation(line: 4, column: 9, scope: !10)
+!17 = !DILocalVariable(name: "b", scope: !10, file: !1, line: 4, type: !13)
+!18 = !DILocation(line: 4, column: 11, scope: !10)
+!19 = !DILocalVariable(scope: !10, file: !1, line: 4, type: !20)
+!20 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "two", file: !1, line: 1, size: 64, flags: DIFlagTypePassByValue, elements: !21, identifier: "_ZTS3two")
+!21 = !{!22, !23}
+!22 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !20, file: !1, line: 1, baseType: !13, size: 32)
+!23 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !20, file: !1, line: 1, baseType: !13, size: 32, offset: 32)
+!25 = !DILocation(line: 4, column: 16, scope: !10)