[llvm] 8e77b33 - [Local] Do not move around dbg.declares during replaceDbgDeclare
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 13 14:35:10 PST 2020
Author: Vedant Kumar
Date: 2020-02-13T14:35:02-08:00
New Revision: 8e77b33b3c67aa8f2580671b019eeb3862651ecb
URL: https://github.com/llvm/llvm-project/commit/8e77b33b3c67aa8f2580671b019eeb3862651ecb
DIFF: https://github.com/llvm/llvm-project/commit/8e77b33b3c67aa8f2580671b019eeb3862651ecb.diff
LOG: [Local] Do not move around dbg.declares during replaceDbgDeclare
replaceDbgDeclare is used to update the descriptions of stack variables
when they are moved (e.g. by ASan or SafeStack). A side effect of
replaceDbgDeclare is that it moves dbg.declares around in the
instruction stream (typically by hoisting them into the entry block).
This behavior was introduced in llvm/r227544 to fix an assertion failure
(llvm.org/PR22386), but no longer appears to be necessary.
Hoisting a dbg.declare generally does not create problems. Usually,
dbg.declare either describes an argument or an alloca in the entry
block, and backends have special handling to emit locations for these.
In optimized builds, LowerDbgDeclare places dbg.values in the right
spots regardless of where the dbg.declare is. And no one uses
replaceDbgDeclare to handle things like VLAs.
However, there doesn't seem to be a positive case for moving
dbg.declares around anymore, and this reordering can get in the way of
understanding other bugs. I propose getting rid of it.
Testing: stage2 RelWithDebInfo sanitized build, check-llvm
rdar://59397340
Differential Revision: https://reviews.llvm.org/D74517
Added:
Modified:
llvm/include/llvm/Transforms/Utils/Local.h
llvm/lib/CodeGen/SafeStack.cpp
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
llvm/test/Transforms/Inline/alloca-dbgdeclare-merge.ll
llvm/test/Transforms/Inline/alloca-dbgdeclare.ll
llvm/test/Transforms/Inline/inline_dbg_declare.ll
llvm/unittests/Transforms/Utils/LocalTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index 38e3c14f7ef9..8ccd9031b568 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -332,20 +332,9 @@ void findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgInsts, Value *V);
/// additional DW_OP_deref is prepended to the expression. If Offset
/// is non-zero, a constant displacement is added to the expression
/// (between the optional Deref operations). Offset can be negative.
-bool replaceDbgDeclare(Value *Address, Value *NewAddress,
- Instruction *InsertBefore, DIBuilder &Builder,
+bool replaceDbgDeclare(Value *Address, Value *NewAddress, DIBuilder &Builder,
uint8_t DIExprFlags, int Offset);
-/// Replaces llvm.dbg.declare instruction when the alloca it describes
-/// is replaced with a new value. If Deref is true, an additional
-/// DW_OP_deref is prepended to the expression. If Offset is non-zero,
-/// a constant displacement is added to the expression (between the
-/// optional Deref operations). Offset can be negative. The new
-/// llvm.dbg.declare is inserted immediately after AI.
-bool replaceDbgDeclareForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
- DIBuilder &Builder, uint8_t DIExprFlags,
- int Offset);
-
/// Replaces multiple llvm.dbg.value instructions when the alloca it describes
/// is replaced with a new value. If Offset is non-zero, a constant displacement
/// is added to the expression (after the mandatory Deref). Offset can be
diff --git a/llvm/lib/CodeGen/SafeStack.cpp b/llvm/lib/CodeGen/SafeStack.cpp
index 136a10fb607e..8808d90175a8 100644
--- a/llvm/lib/CodeGen/SafeStack.cpp
+++ b/llvm/lib/CodeGen/SafeStack.cpp
@@ -576,8 +576,8 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
Arg->getName() + ".unsafe-byval");
// Replace alloc with the new location.
- replaceDbgDeclare(Arg, BasePointer, BasePointer->getNextNode(), DIB,
- DIExpression::ApplyOffset, -Offset);
+ replaceDbgDeclare(Arg, BasePointer, DIB, DIExpression::ApplyOffset,
+ -Offset);
Arg->replaceAllUsesWith(NewArg);
IRB.SetInsertPoint(cast<Instruction>(NewArg)->getNextNode());
IRB.CreateMemCpy(Off, Align, Arg, Arg->getParamAlign(), Size);
@@ -588,8 +588,7 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
IRB.SetInsertPoint(AI);
unsigned Offset = SSL.getObjectOffset(AI);
- replaceDbgDeclareForAlloca(AI, BasePointer, DIB, DIExpression::ApplyOffset,
- -Offset);
+ replaceDbgDeclare(AI, BasePointer, DIB, DIExpression::ApplyOffset, -Offset);
replaceDbgValueForAlloca(AI, BasePointer, DIB, -Offset);
// Replace uses of the alloca with the new location.
@@ -676,7 +675,7 @@ void SafeStack::moveDynamicAllocasToUnsafeStack(
if (AI->hasName() && isa<Instruction>(NewAI))
NewAI->takeName(AI);
- replaceDbgDeclareForAlloca(AI, NewAI, DIB, DIExpression::ApplyOffset, 0);
+ replaceDbgDeclare(AI, NewAI, DIB, DIExpression::ApplyOffset, 0);
AI->replaceAllUsesWith(NewAI);
AI->eraseFromParent();
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index ae537a83ccb0..98cbd2ca841f 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3132,8 +3132,8 @@ void FunctionStackPoisoner::processStaticAllocas() {
// Replace Alloca instructions with base+offset.
for (const auto &Desc : SVD) {
AllocaInst *AI = Desc.AI;
- replaceDbgDeclareForAlloca(AI, LocalStackBaseAllocaPtr, DIB, DIExprFlags,
- Desc.Offset);
+ replaceDbgDeclare(AI, LocalStackBaseAllocaPtr, DIB, DIExprFlags,
+ Desc.Offset);
Value *NewAllocaPtr = IRB.CreateIntToPtr(
IRB.CreateAdd(LocalStackBase, ConstantInt::get(IntptrTy, Desc.Offset)),
AI->getType());
diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 808937724ea7..fcbe96d34061 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1848,10 +1848,6 @@ llvm::InlineResult llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
Caller->getEntryBlock().getInstList().splice(
InsertPoint, FirstNewBlock->getInstList(), AI->getIterator(), I);
}
- // Move any dbg.declares describing the allocas into the entry basic block.
- DIBuilder DIB(*Caller->getParent());
- for (auto &AI : IFI.StaticAllocas)
- replaceDbgDeclareForAlloca(AI, AI, DIB, DIExpression::ApplyOffset, 0);
}
SmallVector<Value*,4> VarArgsToForward;
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index aad960230f5c..0fb5ac8ebcc3 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1568,8 +1568,8 @@ void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
}
bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
- Instruction *InsertBefore, DIBuilder &Builder,
- uint8_t DIExprFlags, int Offset) {
+ DIBuilder &Builder, uint8_t DIExprFlags,
+ int Offset) {
auto DbgAddrs = FindDbgAddrUses(Address);
for (DbgVariableIntrinsic *DII : DbgAddrs) {
DebugLoc Loc = DII->getDebugLoc();
@@ -1577,23 +1577,14 @@ bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
auto *DIExpr = DII->getExpression();
assert(DIVar && "Missing variable");
DIExpr = DIExpression::prepend(DIExpr, DIExprFlags, Offset);
- // Insert llvm.dbg.declare immediately before InsertBefore, and remove old
+ // Insert llvm.dbg.declare immediately before DII, and remove old
// llvm.dbg.declare.
- Builder.insertDeclare(NewAddress, DIVar, DIExpr, Loc, InsertBefore);
- if (DII == InsertBefore)
- InsertBefore = InsertBefore->getNextNode();
+ Builder.insertDeclare(NewAddress, DIVar, DIExpr, Loc, DII);
DII->eraseFromParent();
}
return !DbgAddrs.empty();
}
-bool llvm::replaceDbgDeclareForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
- DIBuilder &Builder, uint8_t DIExprFlags,
- int Offset) {
- return replaceDbgDeclare(AI, NewAllocaAddress, AI->getNextNode(), Builder,
- DIExprFlags, Offset);
-}
-
static void replaceOneDbgValueForAlloca(DbgValueInst *DVI, Value *NewAddress,
DIBuilder &Builder, int Offset) {
DebugLoc Loc = DVI->getDebugLoc();
diff --git a/llvm/test/Instrumentation/AddressSanitizer/debug_info.ll b/llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
index 97faa1aff469..c0389daddacd 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/debug_info.ll
@@ -21,15 +21,11 @@ entry:
}
; CHECK: define i32 @_Z3zzzi
-; CHECK: entry:
-; Verify that llvm.dbg.declare calls are in the entry basic block.
-; CHECK-NEXT: [[MyAlloca:%.*]] = alloca i8, i64 64
-; CHECK-NOT: %entry
+; CHECK: [[MyAlloca:%.*]] = alloca i8, i64 64
; Note: these dbg.declares used to contain `ptrtoint` operands. The instruction
; selector would then decline to put the variable in the MachineFunction side
; table. Check that the dbg.declares have `alloca` operands.
; CHECK: call void @llvm.dbg.declare(metadata i8* [[MyAlloca]], metadata ![[ARG_ID:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 32))
-; CHECK-NOT: %entry
; CHECK: call void @llvm.dbg.declare(metadata i8* [[MyAlloca]], metadata ![[VAR_ID:[0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 48))
declare void @llvm.dbg.declare(metadata, metadata, metadata) nounwind readnone
diff --git a/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll b/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
index 67e13e56414f..7bf294cb6b60 100644
--- a/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll
@@ -19,12 +19,16 @@ entry:
; CHECK: %[[ALLOCA:.*]] = ptrtoint i8* %MyAlloca to i64
; CHECK: %[[PHI:.*]] = phi i64 {{.*}} %[[ALLOCA]],
; CHECK: store i64 %[[PHI]], i64* %asan_local_stack_base
- ; CHECK: call void @llvm.dbg.declare(metadata i64* %asan_local_stack_base, metadata !12, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 32)), !dbg !13
+; CHECK: call void @llvm.dbg.declare(metadata i64* %asan_local_stack_base, metadata [[VAR_I:![0-9]+]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 32)), !dbg [[LOC_I:![0-9]+]]
%0 = load i32, i32* %i.addr, align 4, !dbg !14
%add = add nsw i32 %0, 2, !dbg !15
ret i32 %add, !dbg !16
}
+; CHECK: [[SCOPE:![0-9]+]] = distinct !DISubprogram(name: "foo"
+; CHECK: [[VAR_I]] = !DILocalVariable(name: "i", arg: 1, scope: [[SCOPE]]
+; CHECK: [[LOC_I]] = !DILocation(line: 1, column: 13, scope: [[SCOPE]]
+
; Function Attrs: nounwind readnone speculatable
declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
diff --git a/llvm/test/Transforms/Inline/alloca-dbgdeclare-merge.ll b/llvm/test/Transforms/Inline/alloca-dbgdeclare-merge.ll
index e0e51b268fe7..d0a2bf3b1c9f 100644
--- a/llvm/test/Transforms/Inline/alloca-dbgdeclare-merge.ll
+++ b/llvm/test/Transforms/Inline/alloca-dbgdeclare-merge.ll
@@ -1,6 +1,5 @@
-; Test that alloca merging in the inliner places dbg.declare calls immediately
-; after the merged alloca. Not at the end of the entry BB, and definitely not
-; before the alloca.
+; Test that alloca merging in the inliner places dbg.declare calls after the
+; merged alloca, but does not otherwise reorder them.
;
; clang -g -S -emit-llvm -Xclang -disable-llvm-optzns
;
@@ -20,13 +19,20 @@
;}
;
; RUN: opt -always-inline -S < %s | FileCheck %s
+
+; FIXME: Why does the dbg.declare for "aaa" occur later in @h than the
+; dbg.declare for "bbb"? I'd expect the opposite, given @f is inlined earlier.
;
; CHECK: define void @h()
; CHECK-NEXT: entry:
; CHECK-NEXT: %[[AI:.*]] = alloca [100 x i8]
-; CHECK-NEXT: call void @llvm.dbg.declare(metadata [100 x i8]* %[[AI]],
-; CHECK-NEXT: call void @llvm.dbg.declare(metadata [100 x i8]* %[[AI]],
+; CHECK-NEXT: call void @llvm.dbg.declare(metadata [100 x i8]* %[[AI]], metadata [[BBB:![0-9]+]]
+; CHECK-NEXT: bitcast
+; CHECK-NEXT: llvm.lifetime.start
+; CHECK-NEXT: call void @llvm.dbg.declare(metadata [100 x i8]* %[[AI]], metadata [[AAA:![0-9]+]]
+; CHECK: [[AAA]] = !DILocalVariable(name: "aaa"
+; CHECK: [[BBB]] = !DILocalVariable(name: "bbb"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/Transforms/Inline/alloca-dbgdeclare.ll b/llvm/test/Transforms/Inline/alloca-dbgdeclare.ll
index 07e931d0d227..1dbe996343b2 100644
--- a/llvm/test/Transforms/Inline/alloca-dbgdeclare.ll
+++ b/llvm/test/Transforms/Inline/alloca-dbgdeclare.ll
@@ -42,6 +42,10 @@ entry:
; CHECK: define void @_Z3fn5v()
; CHECK-NEXT: entry:
; CHECK-NEXT: %agg.tmp.sroa.3.i = alloca [20 x i8], align 4
+; CHECK-NEXT: br label %while.body
+; CHECK: while.body:
+; CHECK-NEXT: bitcast
+; CHECK-NEXT: llvm.lifetime.start
; CHECK-NEXT: call void @llvm.dbg.declare(metadata [20 x i8]* %agg.tmp.sroa.3.i,
%agg.tmp.sroa.3 = alloca [20 x i8], align 4
tail call void @llvm.dbg.declare(metadata [20 x i8]* %agg.tmp.sroa.3, metadata !25, metadata !30), !dbg !31
diff --git a/llvm/test/Transforms/Inline/inline_dbg_declare.ll b/llvm/test/Transforms/Inline/inline_dbg_declare.ll
index 74ab8537970c..bfbfb8e76d51 100644
--- a/llvm/test/Transforms/Inline/inline_dbg_declare.ll
+++ b/llvm/test/Transforms/Inline/inline_dbg_declare.ll
@@ -42,7 +42,8 @@ declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
define void @bar(float* %dst) #0 !dbg !9 {
entry:
-; CHECK: [[x_addr_i:%[a-zA-Z0-9.]+]] = alloca float, align 4
+; CHECK: [[x_addr_i:%.+]] = alloca float, align 4
+; CHECK: store float {{.*}}, float* [[x_addr_i]]
; CHECK-NEXT: void @llvm.dbg.declare(metadata float* [[x_addr_i]], metadata [[m23:![0-9]+]], metadata !DIExpression()), !dbg [[m24:![0-9]+]]
%dst.addr = alloca float*, align 4
diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp
index 5551a07beb90..cd32669ca2f6 100644
--- a/llvm/unittests/Transforms/Utils/LocalTest.cpp
+++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp
@@ -154,7 +154,7 @@ TEST(Local, ReplaceDbgDeclare) {
ASSERT_TRUE(DII);
Value *NewBase = Constant::getNullValue(Type::getInt32PtrTy(C));
DIBuilder DIB(*M);
- replaceDbgDeclare(AI, NewBase, DII, DIB, DIExpression::ApplyOffset, 0);
+ replaceDbgDeclare(AI, NewBase, DIB, DIExpression::ApplyOffset, 0);
// There should be exactly two dbg.declares.
int Declares = 0;
More information about the llvm-commits
mailing list