[llvm] 140757b - [DebugInfo] Prevent invalid debug info being produced during LoopStrengthReduce
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 8 05:05:34 PDT 2021
Author: Stephen Tozer
Date: 2021-04-08T13:04:48+01:00
New Revision: 140757bfaaa00110a92d2247a910c847e6e3bcc8
URL: https://github.com/llvm/llvm-project/commit/140757bfaaa00110a92d2247a910c847e6e3bcc8
DIFF: https://github.com/llvm/llvm-project/commit/140757bfaaa00110a92d2247a910c847e6e3bcc8.diff
LOG: [DebugInfo] Prevent invalid debug info being produced during LoopStrengthReduce
During LoopStrengthReduce, some of the SSA values that are used by debug values
may be lost and/or salvaged. After LSR we attempt to recover any undef debug
values, including any that were salvaged but then lost their values afterwards,
by replacing the lost values with any live equal values (plus a possible
constant offset) that have been gathered prior to running LSR. When we do this
we restore the debug value's original DIExpression, to undo any salvaging (as we
have gone back to using the original debug value).
This process can currently produce invalid debug info if the number of operands
has changed by salvaging during LSR. Replacing old values during the
applyEqualValues step does not change the number of location operands, which
means that when we restore the old DIExpression we may have a mismatch between
the number of operands used by the debug value and the number of operands
referenced by the DIExpression. This patch fixes this by restoring the full
original location metadata at the start of the applyEqualValues step, so that
there is no mismatch in operand count between the debug value and its
DIExpression.
Differential Revision: https://reviews.llvm.org/D98644
Added:
llvm/test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll
Modified:
llvm/include/llvm/IR/IntrinsicInst.h
llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index 9c3ec7fcfec9..6c825d380fc9 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -265,6 +265,13 @@ class DbgVariableIntrinsic : public DbgInfoIntrinsic {
return cast<MetadataAsValue>(getArgOperand(2))->getMetadata();
}
+ /// Use of this should generally be avoided; instead,
+ /// replaceVariableLocationOp and addVariableLocationOps should be used where
+ /// possible to avoid creating invalid state.
+ void setRawLocation(Metadata *Location) {
+ return setArgOperand(0, MetadataAsValue::get(getContext(), Location));
+ }
+
/// Get the size (in bits) of the variable, or fragment of the variable that
/// is described.
Optional<uint64_t> getFragmentSizeInBits() const;
diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index ae1ed681d998..45578fbbfde4 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -5831,12 +5831,13 @@ void LoopStrengthReduce::getAnalysisUsage(AnalysisUsage &AU) const {
using EqualValues = SmallVector<std::tuple<WeakVH, int64_t>, 4>;
using EqualValuesMap =
- DenseMap<std::pair<DbgValueInst *, unsigned>, EqualValues>;
-using ExpressionMap = DenseMap<DbgValueInst *, DIExpression *>;
+ DenseMap<DbgValueInst *, SmallVector<std::pair<unsigned, EqualValues>>>;
+using LocationMap =
+ DenseMap<DbgValueInst *, std::pair<DIExpression *, Metadata *>>;
static void DbgGatherEqualValues(Loop *L, ScalarEvolution &SE,
EqualValuesMap &DbgValueToEqualSet,
- ExpressionMap &DbgValueToExpression) {
+ LocationMap &DbgValueToLocation) {
for (auto &B : L->getBlocks()) {
for (auto &I : *B) {
auto DVI = dyn_cast<DbgValueInst>(&I);
@@ -5862,39 +5863,51 @@ static void DbgGatherEqualValues(Loop *L, ScalarEvolution &SE,
EqSet.emplace_back(
std::make_tuple(&Phi, Offset.getValue().getSExtValue()));
}
- DbgValueToEqualSet[{DVI, Idx}] = std::move(EqSet);
- DbgValueToExpression[DVI] = DVI->getExpression();
+ DbgValueToEqualSet[DVI].push_back({Idx, std::move(EqSet)});
+ // If we fall back to using this raw location, at least one location op
+ // must be dead. A DIArgList will automatically undef arguments when
+ // they become unavailable, but a ValueAsMetadata will not; since we
+ // know the value should be undef, we use the undef value directly here.
+ Metadata *RawLocation =
+ DVI->hasArgList() ? DVI->getRawLocation()
+ : ValueAsMetadata::get(UndefValue::get(
+ DVI->getVariableLocationOp(0)->getType()));
+ DbgValueToLocation[DVI] = {DVI->getExpression(), RawLocation};
}
}
}
}
static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet,
- ExpressionMap &DbgValueToExpression) {
+ LocationMap &DbgValueToLocation) {
for (auto A : DbgValueToEqualSet) {
- auto DVI = A.first.first;
- auto Idx = A.first.second;
+ auto *DVI = A.first;
// Only update those that are now undef.
- if (!isa_and_nonnull<UndefValue>(DVI->getVariableLocationOp(Idx)))
+ if (!DVI->isUndef())
continue;
- for (auto EV : A.second) {
- auto EVHandle = std::get<WeakVH>(EV);
- if (!EVHandle)
- continue;
- // The dbg.value may have had its value changed by LSR; refresh it from
- // the map, but continue to update the mapped expression as it may be
- // updated multiple times in this function.
- auto DbgDIExpr = DbgValueToExpression[DVI];
- auto Offset = std::get<int64_t>(EV);
- DVI->replaceVariableLocationOp(Idx, EVHandle);
- if (Offset) {
- SmallVector<uint64_t, 8> Ops;
- DIExpression::appendOffset(Ops, Offset);
- DbgDIExpr = DIExpression::appendOpsToArg(DbgDIExpr, Ops, Idx, true);
+ // The dbg.value may have had its value or expression changed during LSR by
+ // a failed salvage attempt; refresh them from the map.
+ auto *DbgDIExpr = DbgValueToLocation[DVI].first;
+ DVI->setRawLocation(DbgValueToLocation[DVI].second);
+ DVI->setExpression(DbgDIExpr);
+ assert(DVI->isUndef() && "dbg.value with non-undef location should not "
+ "have been modified by LSR.");
+ for (auto IdxEV : A.second) {
+ unsigned Idx = IdxEV.first;
+ for (auto EV : IdxEV.second) {
+ auto EVHandle = std::get<WeakVH>(EV);
+ if (!EVHandle)
+ continue;
+ int64_t Offset = std::get<int64_t>(EV);
+ DVI->replaceVariableLocationOp(Idx, EVHandle);
+ if (Offset) {
+ SmallVector<uint64_t, 8> Ops;
+ DIExpression::appendOffset(Ops, Offset);
+ DbgDIExpr = DIExpression::appendOpsToArg(DbgDIExpr, Ops, Idx, true);
+ }
+ DVI->setExpression(DbgDIExpr);
+ break;
}
- DVI->setExpression(DbgDIExpr);
- DbgValueToExpression[DVI] = DbgDIExpr;
- break;
}
}
}
@@ -5917,8 +5930,8 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
// Debug preservation - before we start removing anything create equivalence
// sets for the llvm.dbg.value intrinsics.
EqualValuesMap DbgValueToEqualSet;
- ExpressionMap DbgValueToExpression;
- DbgGatherEqualValues(L, SE, DbgValueToEqualSet, DbgValueToExpression);
+ LocationMap DbgValueToLocation;
+ DbgGatherEqualValues(L, SE, DbgValueToEqualSet, DbgValueToLocation);
// Remove any extra phis created by processing inner loops.
Changed |= DeleteDeadPHIs(L->getHeader(), &TLI, MSSAU.get());
@@ -5938,7 +5951,7 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
}
}
- DbgApplyEqualValues(DbgValueToEqualSet, DbgValueToExpression);
+ DbgApplyEqualValues(DbgValueToEqualSet, DbgValueToLocation);
return Changed;
}
diff --git a/llvm/test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll b/llvm/test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll
new file mode 100644
index 000000000000..030c5bb3e5d3
--- /dev/null
+++ b/llvm/test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll
@@ -0,0 +1,84 @@
+; RUN: opt < %s -loop-reduce -S | FileCheck %s
+
+; Test that LSR does not produce invalid debug info when a debug value is
+; salvaged during LSR by adding additional location operands, then becomes
+; undef, and finally recovered by using equal values gathered before LSR.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+define i32 @_Z3fooiii(i32 %Result, i32 %Step, i32 %Last) local_unnamed_addr !dbg !7 {
+; CHECK-LABEL: @_Z3fooiii(
+entry:
+ call void @llvm.dbg.value(metadata i32 %Result, metadata !12, metadata !DIExpression()), !dbg !17
+ call void @llvm.dbg.value(metadata i32 %Step, metadata !13, metadata !DIExpression()), !dbg !17
+ call void @llvm.dbg.value(metadata i32 %Last, metadata !14, metadata !DIExpression()), !dbg !17
+ call void @llvm.dbg.value(metadata i32 0, metadata !16, metadata !DIExpression()), !dbg !17
+ br label %do.body, !dbg !18
+
+do.body: ; preds = %do.body, %entry
+; CHECK-LABEL: do.body:
+ %Result.addr.0 = phi i32 [ %Result, %entry ], [ %or, %do.body ]
+ %Itr.0 = phi i32 [ 0, %entry ], [ %add, %do.body ], !dbg !17
+; CHECK-NOT: call void @llvm.dbg.value(metadata !DIArgList
+; CHECK: call void @llvm.dbg.value(metadata i32 %lsr.iv, metadata ![[VAR_ITR:[0-9]+]], metadata !DIExpression()
+ call void @llvm.dbg.value(metadata i32 %Itr.0, metadata !16, metadata !DIExpression()), !dbg !17
+ call void @llvm.dbg.value(metadata i32 %Result.addr.0, metadata !12, metadata !DIExpression()), !dbg !17
+ %add = add nsw i32 %Itr.0, %Step, !dbg !19
+ call void @llvm.dbg.value(metadata i32 %add, metadata !16, metadata !DIExpression()), !dbg !17
+ %call = tail call i32 @_Z3barv(), !dbg !21
+ call void @llvm.dbg.value(metadata i32 %call, metadata !15, metadata !DIExpression()), !dbg !17
+ %shl = shl i32 %call, %add, !dbg !22
+ %or = or i32 %shl, %Result.addr.0, !dbg !23
+ call void @llvm.dbg.value(metadata i32 %or, metadata !12, metadata !DIExpression()), !dbg !17
+ %and = and i32 %call, %Last, !dbg !24
+ %tobool.not = icmp eq i32 %and, 0, !dbg !25
+ br i1 %tobool.not, label %do.end, label %do.body, !dbg !26, !llvm.loop !27
+
+do.end: ; preds = %do.body
+ ret i32 %or, !dbg !30
+}
+
+declare !dbg !31 dso_local i32 @_Z3barv() local_unnamed_addr
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+; CHECK: ![[VAR_ITR]] = !DILocalVariable(name: "Itr"
+
+!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 13.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", 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 13.0.0"}
+!7 = distinct !DISubprogram(name: "foo", linkageName: "_Z3fooiii", scope: !1, file: !1, line: 3, type: !8, scopeLine: 3, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !10, !10, !10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !{!12, !13, !14, !15, !16}
+!12 = !DILocalVariable(name: "Result", arg: 1, scope: !7, file: !1, line: 3, type: !10)
+!13 = !DILocalVariable(name: "Step", arg: 2, scope: !7, file: !1, line: 3, type: !10)
+!14 = !DILocalVariable(name: "Last", arg: 3, scope: !7, file: !1, line: 3, type: !10)
+!15 = !DILocalVariable(name: "Bar", scope: !7, file: !1, line: 4, type: !10)
+!16 = !DILocalVariable(name: "Itr", scope: !7, file: !1, line: 5, type: !10)
+!17 = !DILocation(line: 0, scope: !7)
+!18 = !DILocation(line: 6, column: 3, scope: !7)
+!19 = !DILocation(line: 7, column: 9, scope: !20)
+!20 = distinct !DILexicalBlock(scope: !7, file: !1, line: 6, column: 6)
+!21 = !DILocation(line: 8, column: 11, scope: !20)
+!22 = !DILocation(line: 9, column: 20, scope: !20)
+!23 = !DILocation(line: 9, column: 12, scope: !20)
+!24 = !DILocation(line: 10, column: 16, scope: !7)
+!25 = !DILocation(line: 10, column: 12, scope: !7)
+!26 = !DILocation(line: 10, column: 3, scope: !20)
+!27 = distinct !{!27, !18, !28, !29}
+!28 = !DILocation(line: 10, column: 22, scope: !7)
+!29 = !{!"llvm.loop.mustprogress"}
+!30 = !DILocation(line: 11, column: 3, scope: !7)
+!31 = !DISubprogram(name: "bar", linkageName: "_Z3barv", scope: !1, file: !1, line: 1, type: !32, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+!32 = !DISubroutineType(types: !33)
+!33 = !{!10}
More information about the llvm-commits
mailing list