Skip to content

Commit 928c333

Browse files
authored
[DebugInfo] Handle additional types of stores in assignment tracking (#129070)
Fixes #126417. Currently, assignment tracking recognizes allocas, stores, and mem intrinsics as valid instructions to tag with DIAssignID, with allocas representing the allocation for a variable and the others representing instructions that may assign to the variable. There are other intrinsics that can perform these assignments however, and if we transform a store instruction into one of these intrinsics and correctly transfer the DIAssignID over, this results in a verifier error. The AssignmentTrackingAnalysis pass also does not know how to handle these intrinsics if they are untagged, as it does not know how to extract assignment information (base address, offset, size) from them. This patch adds _some_ support for some intrinsics that may perform assignments: masked store/scatter, and vp store/strided store/scatter. This patch does not add support for extracting assignment information from these, as they may store with either non-constant size or to non-contiguous blocks of memory; instead it adds support for recognizing untagged stores with "unknown" assignment info, for which we assume that the memory location of the associated variable should not be used, as we can't determine which fragments of it should or should not be used. In principle, it should be possible to handle the more complex cases mentioned above, but it would require more substantial changes to AssignmentTrackingAnalysis, and it is mostly only needed as a fallback if the DIAssignID is not preserved on these alternative stores.
1 parent f6178cd commit 928c333

File tree

4 files changed

+215
-5
lines changed

4 files changed

+215
-5
lines changed

llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp

Lines changed: 106 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "llvm/IR/Function.h"
2424
#include "llvm/IR/Instruction.h"
2525
#include "llvm/IR/IntrinsicInst.h"
26+
#include "llvm/IR/Intrinsics.h"
2627
#include "llvm/IR/Module.h"
2728
#include "llvm/IR/PassManager.h"
2829
#include "llvm/IR/PrintPasses.h"
@@ -1102,6 +1103,8 @@ class AssignmentTrackingLowering {
11021103
using UntaggedStoreAssignmentMap =
11031104
DenseMap<const Instruction *,
11041105
SmallVector<std::pair<VariableID, at::AssignmentInfo>>>;
1106+
using UnknownStoreAssignmentMap =
1107+
DenseMap<const Instruction *, SmallVector<VariableID>>;
11051108

11061109
private:
11071110
/// The highest numbered VariableID for partially promoted variables plus 1,
@@ -1112,6 +1115,9 @@ class AssignmentTrackingLowering {
11121115
/// Map untagged stores to the variable fragments they assign to. Used by
11131116
/// processUntaggedInstruction.
11141117
UntaggedStoreAssignmentMap UntaggedStoreVars;
1118+
/// Map untagged unknown stores (e.g. strided/masked store intrinsics)
1119+
/// to the variables they may assign to. Used by processUntaggedInstruction.
1120+
UnknownStoreAssignmentMap UnknownStoreVars;
11151121

11161122
// Machinery to defer inserting dbg.values.
11171123
using InstInsertMap = MapVector<VarLocInsertPt, SmallVector<VarLocInfo>>;
@@ -1354,6 +1360,8 @@ class AssignmentTrackingLowering {
13541360
/// Update \p LiveSet after encountering an instruciton without a DIAssignID
13551361
/// attachment, \p I.
13561362
void processUntaggedInstruction(Instruction &I, BlockInfo *LiveSet);
1363+
void processUnknownStoreToVariable(Instruction &I, VariableID &Var,
1364+
BlockInfo *LiveSet);
13571365
void processDbgAssign(AssignRecord Assign, BlockInfo *LiveSet);
13581366
void processDbgVariableRecord(DbgVariableRecord &DVR, BlockInfo *LiveSet);
13591367
void processDbgValue(
@@ -1603,6 +1611,45 @@ void AssignmentTrackingLowering::processNonDbgInstruction(
16031611
processUntaggedInstruction(I, LiveSet);
16041612
}
16051613

1614+
void AssignmentTrackingLowering::processUnknownStoreToVariable(
1615+
Instruction &I, VariableID &Var, BlockInfo *LiveSet) {
1616+
// We may have assigned to some unknown fragment of the variable, so
1617+
// treat the memory assignment as unknown for now.
1618+
addMemDef(LiveSet, Var, Assignment::makeNoneOrPhi());
1619+
// If we weren't already using a memory location, we don't need to do
1620+
// anything more.
1621+
if (getLocKind(LiveSet, Var) != LocKind::Mem)
1622+
return;
1623+
// If there is a live debug value for this variable, fall back to using
1624+
// that.
1625+
Assignment DbgAV = LiveSet->getAssignment(BlockInfo::Debug, Var);
1626+
if (DbgAV.Status != Assignment::NoneOrPhi && DbgAV.Source) {
1627+
LLVM_DEBUG(dbgs() << "Switching to fallback debug value: ";
1628+
DbgAV.dump(dbgs()); dbgs() << "\n");
1629+
setLocKind(LiveSet, Var, LocKind::Val);
1630+
emitDbgValue(LocKind::Val, DbgAV.Source, &I);
1631+
return;
1632+
}
1633+
// Otherwise, find a suitable insert point, before the next instruction or
1634+
// DbgRecord after I.
1635+
auto InsertBefore = getNextNode(&I);
1636+
assert(InsertBefore && "Shouldn't be inserting after a terminator");
1637+
1638+
// Get DILocation for this assignment.
1639+
DebugVariable V = FnVarLocs->getVariable(Var);
1640+
DILocation *InlinedAt = const_cast<DILocation *>(V.getInlinedAt());
1641+
const DILocation *DILoc = DILocation::get(
1642+
Fn.getContext(), 0, 0, V.getVariable()->getScope(), InlinedAt);
1643+
1644+
VarLocInfo VarLoc;
1645+
VarLoc.VariableID = static_cast<VariableID>(Var);
1646+
VarLoc.Expr = DIExpression::get(I.getContext(), {});
1647+
VarLoc.Values = RawLocationWrapper(
1648+
ValueAsMetadata::get(PoisonValue::get(Type::getInt1Ty(I.getContext()))));
1649+
VarLoc.DL = DILoc;
1650+
InsertBeforeMap[InsertBefore].push_back(VarLoc);
1651+
}
1652+
16061653
void AssignmentTrackingLowering::processUntaggedInstruction(
16071654
Instruction &I, AssignmentTrackingLowering::BlockInfo *LiveSet) {
16081655
// Interpret stack stores that are not tagged as an assignment in memory for
@@ -1618,8 +1665,21 @@ void AssignmentTrackingLowering::processUntaggedInstruction(
16181665
// "early", for example.
16191666
assert(!I.hasMetadata(LLVMContext::MD_DIAssignID));
16201667
auto It = UntaggedStoreVars.find(&I);
1621-
if (It == UntaggedStoreVars.end())
1668+
if (It == UntaggedStoreVars.end()) {
1669+
// It is possible that we have an untagged unknown store, i.e. one that
1670+
// cannot be represented as a simple (base, offset, size) - in this case we
1671+
// should undef the memory location of the variable, as if we had a tagged
1672+
// store that did not match the current assignment.
1673+
// FIXME: It should be possible to support these stores, but it would
1674+
// require more extensive changes to our representation of assignments.
1675+
if (auto UnhandledStoreIt = UnknownStoreVars.find(&I);
1676+
UnhandledStoreIt != UnknownStoreVars.end()) {
1677+
LLVM_DEBUG(dbgs() << "Processing untagged unknown store " << I << "\n");
1678+
for (auto &Var : UnhandledStoreIt->second)
1679+
processUnknownStoreToVariable(I, Var, LiveSet);
1680+
}
16221681
return; // No variables associated with the store destination.
1682+
}
16231683

16241684
LLVM_DEBUG(dbgs() << "processUntaggedInstruction on UNTAGGED INST " << I
16251685
<< "\n");
@@ -2122,6 +2182,26 @@ getUntaggedStoreAssignmentInfo(const Instruction &I, const DataLayout &Layout) {
21222182
return std::nullopt;
21232183
}
21242184

2185+
AllocaInst *getUnknownStore(const Instruction &I, const DataLayout &Layout) {
2186+
auto *II = dyn_cast<IntrinsicInst>(&I);
2187+
if (!II)
2188+
return nullptr;
2189+
Intrinsic::ID ID = II->getIntrinsicID();
2190+
if (ID != Intrinsic::experimental_vp_strided_store &&
2191+
ID != Intrinsic::masked_store && ID != Intrinsic::vp_scatter &&
2192+
ID != Intrinsic::masked_scatter && ID != Intrinsic::vp_store &&
2193+
ID != Intrinsic::masked_compressstore)
2194+
return nullptr;
2195+
Value *MemOp = II->getArgOperand(1);
2196+
// We don't actually use the constant offset for now, but we may in future,
2197+
// and the non-accumulating versions do not support a vector of pointers.
2198+
APInt Offset(Layout.getIndexTypeSizeInBits(MemOp->getType()), 0);
2199+
Value *Base = MemOp->stripAndAccumulateConstantOffsets(Layout, Offset, true);
2200+
// For Base pointers that are not an alloca instruction we don't need to do
2201+
// anything, and simply return nullptr.
2202+
return dyn_cast<AllocaInst>(Base);
2203+
}
2204+
21252205
DbgDeclareInst *DynCastToDbgDeclare(DbgVariableIntrinsic *DVI) {
21262206
return dyn_cast<DbgDeclareInst>(DVI);
21272207
}
@@ -2144,14 +2224,16 @@ DbgVariableRecord *DynCastToDbgDeclare(DbgVariableRecord *DVR) {
21442224
/// subsequent variables are either stack homed or fully promoted.
21452225
///
21462226
/// Finally, populate UntaggedStoreVars with a mapping of untagged stores to
2147-
/// the stored-to variable fragments.
2227+
/// the stored-to variable fragments, and UnknownStoreVars with a mapping
2228+
/// of untagged unknown stores to the stored-to variable aggregates.
21482229
///
21492230
/// These tasks are bundled together to reduce the number of times we need
21502231
/// to iterate over the function as they can be achieved together in one pass.
21512232
static AssignmentTrackingLowering::OverlapMap buildOverlapMapAndRecordDeclares(
21522233
Function &Fn, FunctionVarLocsBuilder *FnVarLocs,
21532234
const DenseSet<DebugAggregate> &VarsWithStackSlot,
21542235
AssignmentTrackingLowering::UntaggedStoreAssignmentMap &UntaggedStoreVars,
2236+
AssignmentTrackingLowering::UnknownStoreAssignmentMap &UnknownStoreVars,
21552237
unsigned &TrackedVariablesVectorSize) {
21562238
DenseSet<DebugVariable> Seen;
21572239
// Map of Variable: [Fragments].
@@ -2160,7 +2242,8 @@ static AssignmentTrackingLowering::OverlapMap buildOverlapMapAndRecordDeclares(
21602242
// - dbg.declare -> add single location variable record
21612243
// - dbg.* -> Add fragments to FragmentMap
21622244
// - untagged store -> Add fragments to FragmentMap and update
2163-
// UntaggedStoreVars.
2245+
// UntaggedStoreVars, or add to UnknownStoreVars if
2246+
// we can't determine the fragment overlap.
21642247
// We need to add fragments for untagged stores too so that we can correctly
21652248
// clobber overlapped fragment locations later.
21662249
SmallVector<DbgDeclareInst *> InstDeclares;
@@ -2223,6 +2306,25 @@ static AssignmentTrackingLowering::OverlapMap buildOverlapMapAndRecordDeclares(
22232306
HandleDbgAssignForStore(DAI);
22242307
for (DbgVariableRecord *DVR : at::getDVRAssignmentMarkers(Info->Base))
22252308
HandleDbgAssignForStore(DVR);
2309+
} else if (auto *AI = getUnknownStore(I, Fn.getDataLayout())) {
2310+
// Find markers linked to this alloca.
2311+
auto HandleDbgAssignForUnknownStore = [&](auto *Assign) {
2312+
// Because we can't currently represent the fragment info for this
2313+
// store, we treat it as an unusable store to the whole variable.
2314+
DebugVariable DV =
2315+
DebugVariable(Assign->getVariable(), std::nullopt,
2316+
Assign->getDebugLoc().getInlinedAt());
2317+
DebugAggregate DA = {DV.getVariable(), DV.getInlinedAt()};
2318+
if (!VarsWithStackSlot.contains(DA))
2319+
return;
2320+
2321+
// Cache this info for later.
2322+
UnknownStoreVars[&I].push_back(FnVarLocs->insertVariable(DV));
2323+
};
2324+
for (DbgAssignIntrinsic *DAI : at::getAssignmentMarkers(AI))
2325+
HandleDbgAssignForUnknownStore(DAI);
2326+
for (DbgVariableRecord *DVR : at::getDVRAssignmentMarkers(AI))
2327+
HandleDbgAssignForUnknownStore(DVR);
22262328
}
22272329
}
22282330
}
@@ -2297,7 +2399,7 @@ bool AssignmentTrackingLowering::run(FunctionVarLocsBuilder *FnVarLocsBuilder) {
22972399
// neither does LiveDebugVariables) because that is difficult to do and
22982400
// appears to be rare occurance.
22992401
VarContains = buildOverlapMapAndRecordDeclares(
2300-
Fn, FnVarLocs, *VarsWithStackSlot, UntaggedStoreVars,
2402+
Fn, FnVarLocs, *VarsWithStackSlot, UntaggedStoreVars, UnknownStoreVars,
23012403
TrackedVariablesVectorSize);
23022404

23032405
// Prepare for traversal.

llvm/lib/IR/Verifier.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4966,8 +4966,12 @@ void Verifier::visitProfMetadata(Instruction &I, MDNode *MD) {
49664966

49674967
void Verifier::visitDIAssignIDMetadata(Instruction &I, MDNode *MD) {
49684968
assert(I.hasMetadata(LLVMContext::MD_DIAssignID));
4969+
// DIAssignID metadata must be attached to either an alloca or some form of
4970+
// store/memory-writing instruction.
4971+
// FIXME: We allow all intrinsic insts here to avoid trying to enumerate all
4972+
// possible store intrinsics.
49694973
bool ExpectedInstTy =
4970-
isa<AllocaInst>(I) || isa<StoreInst>(I) || isa<MemIntrinsic>(I);
4974+
isa<AllocaInst>(I) || isa<StoreInst>(I) || isa<IntrinsicInst>(I);
49714975
CheckDI(ExpectedInstTy, "!DIAssignID attached to unexpected instruction kind",
49724976
I, MD);
49734977
// Iterate over the MetadataAsValue uses of the DIAssignID - these should
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
; RUN: llc -mtriple=riscv64 < %s | FileCheck %s --implicit-check-not=DEBUG_VALUE
2+
3+
;; Verify that tagged and untagged non-contiguous stores are handled correctly
4+
;; by assignment tracking.
5+
;; * The store to "i" is untagged, and results in the memory location being
6+
;; dropped in favour of the debug value 1010 after the store.
7+
;; * The store to "j" is tagged with a corresponding dbg_assign, which allows
8+
;; us to keep using the memory location.
9+
10+
; CHECK-LABEL: foo:
11+
; CHECK-NEXT: .Lfunc_begin0:
12+
; CHECK: # %bb.0
13+
; CHECK: addi a1, sp, 48
14+
; CHECK-NEXT: #DEBUG_VALUE: foo:i <- [DW_OP_deref] $x12
15+
; CHECK-NEXT: #DEBUG_VALUE: foo:j <- [DW_OP_deref] $x12
16+
; CHECK: vsse32.v
17+
; CHECK-NEXT: #DEBUG_VALUE: foo:i <- 1010
18+
; CHECK-NEXT: vsse32.v
19+
20+
21+
target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
22+
target triple = "riscv64-unknown-linux-gnu"
23+
24+
define void @foo() #0 !dbg !5 {
25+
entry:
26+
%i = alloca i64, align 8, !DIAssignID !6
27+
%j = alloca i64, align 8, !DIAssignID !12
28+
%sar_height.i = getelementptr i8, ptr %i, i64 24
29+
store ptr %sar_height.i, ptr null, align 8
30+
%vui.i = getelementptr i8, ptr %i, i64 44
31+
%0 = load i32, ptr %vui.i, align 4
32+
%sar_width.i = getelementptr i8, ptr %i, i64 20
33+
%i_sar_width.i = getelementptr i8, ptr %i, i64 48
34+
%j_sar_width.j = getelementptr i8, ptr %j, i64 48
35+
#dbg_assign(i32 1010, !7, !DIExpression(), !6, ptr %i_sar_width.i, !DIExpression(), !9)
36+
#dbg_assign(i32 2121, !17, !DIExpression(), !12, ptr %i_sar_width.i, !DIExpression(), !9)
37+
%1 = load <2 x i32>, ptr %sar_width.i, align 4
38+
call void @llvm.experimental.vp.strided.store.v2i32.p0.i64(<2 x i32> %1, ptr align 4 %i_sar_width.i, i64 -4, <2 x i1> splat (i1 true), i32 2)
39+
call void @llvm.experimental.vp.strided.store.v2i32.p0.i64(<2 x i32> %1, ptr align 4 %j_sar_width.j, i64 -4, <2 x i1> splat (i1 true), i32 2), !DIAssignID !13
40+
#dbg_assign(i32 1010, !7, !DIExpression(), !14, ptr %i_sar_width.i, !DIExpression(), !9)
41+
#dbg_assign(i32 2121, !17, !DIExpression(), !13, ptr %i_sar_width.i, !DIExpression(), !9)
42+
ret void
43+
}
44+
45+
attributes #0 = { "target-features"="+v" }
46+
47+
!llvm.dbg.cu = !{!0}
48+
!llvm.module.flags = !{!3, !4}
49+
50+
!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, producer: "clang version 21.0.0git")
51+
!1 = !DIFile(filename: "test.c", directory: "/")
52+
!2 = !{}
53+
!3 = !{i32 2, !"Debug Info Version", i32 3}
54+
!4 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
55+
!5 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 1, scopeLine: 1, type: !10, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
56+
!6 = distinct !DIAssignID()
57+
!7 = !DILocalVariable(name: "i", scope: !5, file: !1, line: 7, type: !8)
58+
!8 = !DIBasicType(name: "int32_t", size: 32, encoding: DW_ATE_signed)
59+
!9 = !DILocation(line: 5, scope: !5)
60+
!10 = !DISubroutineType(types: !2)
61+
!11 = !{!7, !17}
62+
!12 = distinct !DIAssignID()
63+
!13 = distinct !DIAssignID()
64+
!14 = distinct !DIAssignID()
65+
!17 = !DILocalVariable(name: "j", scope: !5, file: !1, line: 7, type: !8)
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
; RUN: llvm-as -disable-output <%s 2>&1 | FileCheck %s --implicit-check-not="attached to unexpected instruction kind"
2+
;; Check that we allow intrinsics to have !DIAssignID attachments, but we do not
3+
;; allow non-intrinsic calls to have them.
4+
;; FIXME: Ideally we would also not allow non-store intrinsics, e.g. the
5+
;; llvm.vp.load intrinsic in this test.
6+
7+
; CHECK: !DIAssignID attached to unexpected instruction kind
8+
; CHECK-NEXT: call void @g()
9+
10+
declare void @g()
11+
12+
define void @f() !dbg !5 {
13+
call void @llvm.vp.store.v2i8.p0(<2 x i8> poison, ptr poison, <2 x i1> poison, i32 poison), !DIAssignID !6
14+
call void @llvm.vp.scatter.v2i8.v2p0(<2 x i8> poison, <2 x ptr> poison, <2 x i1> poison, i32 poison), !DIAssignID !7
15+
call void @llvm.experimental.vp.strided.store.v2i8.i64(<2 x i8> poison, ptr poison, i64 poison, <2 x i1> poison, i32 poison), !DIAssignID !8
16+
call void @llvm.masked.store.v2i8.p0(<2 x i8> poison, ptr poison, i32 1, <2 x i1> poison), !DIAssignID !9
17+
call void @llvm.masked.scatter.v2i8.v2p0(<2 x i8> poison, <2 x ptr> poison, i32 1, <2 x i1> poison), !DIAssignID !10
18+
%r = call <2 x i8> @llvm.vp.load.v2i8.p0(ptr poison, <2 x i1> poison, i32 poison), !DIAssignID !11
19+
call void @g(), !DIAssignID !12
20+
ret void
21+
}
22+
23+
!llvm.module.flags = !{!0}
24+
!llvm.dbg.cu = !{!1}
25+
26+
!0 = !{i32 2, !"Debug Info Version", i32 3}
27+
!1 = distinct !DICompileUnit(language: DW_LANG_Swift, producer: "clang",
28+
file: !2, emissionKind: 2)
29+
!2 = !DIFile(filename: "path/to/file", directory: "/path/to/dir")
30+
!3 = !{null}
31+
!4 = !DISubroutineType(types: !3)
32+
!5 = distinct !DISubprogram(name: "f", scope: !2, file: !2, line: 1, type: !4, scopeLine: 2, unit: !1)
33+
!6 = distinct !DIAssignID()
34+
!7 = distinct !DIAssignID()
35+
!8 = distinct !DIAssignID()
36+
!9 = distinct !DIAssignID()
37+
!10 = distinct !DIAssignID()
38+
!11 = distinct !DIAssignID()
39+
!12 = distinct !DIAssignID()

0 commit comments

Comments
 (0)