[llvm] r227604 - Inliner: Use replaceDbgDeclareForAlloca() instead of splicing the
Adrian Prantl
aprantl at apple.com
Wed Feb 4 14:16:05 PST 2015
> On Feb 4, 2015, at 2:02 PM, Robinson, Paul <Paul_Robinson at playstation.sony.com> wrote:
>
>>> On Feb 4, 2015, at 12:06 PM, Robinson, Paul
>> <Paul_Robinson at playstation.sony.com> wrote:
>>>
>>> Hi Adrian,
>>> It turns out r227544 was causing a -g compilation to hang for us, I've
>>> attached the reduced C++ source. This patch fixed it, so I don't know
>>> whether you want to add this as a test case or not bother.
>>
>> Thanks, I received a similar report from Alexei see the thread for
>> r227544. I don’t really know what else to test for besides that the
>> testcase terminates so I didn’t add a separate testcase for the infinite
>> loop.
>>
>>>
>>> Incidentally in looking closer at the patch, we had one question,
>>> see inline comment below.
>>> Thanks,
>>> --paulr
>>>
>>>> Author: adrian
>>>> Date: Fri Jan 30 13:37:48 2015
>>>> New Revision: 227604
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=227604&view=rev
>>>> Log:
>>>> Inliner: Use replaceDbgDeclareForAlloca() instead of splicing the
>>>> instruction and generalize it to optionally dereference the variable.
>>>> Follow-up to r227544.
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/Transforms/Utils/Local.h
>>>> llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>>> llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
>>>> llvm/trunk/lib/Transforms/Utils/Local.cpp
>>>> llvm/trunk/test/Transforms/Inline/alloca-dbgdeclare.ll
>>>> llvm/trunk/test/Transforms/Inline/inline_dbg_declare.ll
>>>>
>>>> Modified: llvm/trunk/include/llvm/Transforms/Utils/Local.h
>>>> URL: http://llvm.org/viewvc/llvm-
>>>>
>> project/llvm/trunk/include/llvm/Transforms/Utils/Local.h?rev=227604&r1=227
>>>> 603&r2=227604&view=diff
>>>>
>> ==========================================================================
>>>> ====
>>>> --- llvm/trunk/include/llvm/Transforms/Utils/Local.h (original)
>>>> +++ llvm/trunk/include/llvm/Transforms/Utils/Local.h Fri Jan 30
>> 13:37:48
>>>> 2015
>>>> @@ -274,10 +274,11 @@ bool LowerDbgDeclare(Function &F);
>>>> /// an alloca, if any.
>>>> DbgDeclareInst *FindAllocaDbgDeclare(Value *V);
>>>>
>>>> -/// replaceDbgDeclareForAlloca - Replaces llvm.dbg.declare instruction
>>>> when
>>>> -/// alloca is replaced with a new value.
>>>> +/// \brief Replaces llvm.dbg.declare instruction when an alloca is
>>>> replaced with
>>>> +/// a new value. If Deref is true, tan additional DW_OP_deref is
>>>> prepended to
>>>> +/// the expression.
>>>> bool replaceDbgDeclareForAlloca(AllocaInst *AI, Value
>> *NewAllocaAddress,
>>>> - DIBuilder &Builder);
>>>> + DIBuilder &Builder, bool Deref);
>>>>
>>>> /// \brief Remove all blocks that can not be reached from the
>> function's
>>>> entry.
>>>> ///
>>>>
>>>> Modified:
>> llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>>> URL: http://llvm.org/viewvc/llvm-
>>>>
>> project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev
>>>> =227604&r1=227603&r2=227604&view=diff
>>>>
>> ==========================================================================
>>>> ====
>>>> --- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>>> (original)
>>>> +++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp Fri
>> Jan
>>>> 30 13:37:48 2015
>>>> @@ -1739,7 +1739,7 @@ void FunctionStackPoisoner::poisonStack(
>>>> Value *NewAllocaPtr = IRB.CreateIntToPtr(
>>>> IRB.CreateAdd(LocalStackBase, ConstantInt::get(IntptrTy,
>>>> Desc.Offset)),
>>>> AI->getType());
>>>> - replaceDbgDeclareForAlloca(AI, NewAllocaPtr, DIB);
>>>> + replaceDbgDeclareForAlloca(AI, NewAllocaPtr, DIB, /*Deref=*/true);
>>>> AI->replaceAllUsesWith(NewAllocaPtr);
>>>> }
>>>>
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
>>>> URL: http://llvm.org/viewvc/llvm-
>>>>
>> project/llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp?rev=227604&r1=2
>>>> 27603&r2=227604&view=diff
>>>>
>> ==========================================================================
>>>> ====
>>>> --- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Fri Jan 30
>> 13:37:48
>>>> 2015
>>>> @@ -30,6 +30,7 @@
>>>> #include "llvm/IR/DataLayout.h"
>>>> #include "llvm/IR/DebugInfo.h"
>>>> #include "llvm/IR/DerivedTypes.h"
>>>> +#include "llvm/IR/DIBuilder.h"
>>>> #include "llvm/IR/Dominators.h"
>>>> #include "llvm/IR/IRBuilder.h"
>>>> #include "llvm/IR/Instructions.h"
>>>> @@ -1112,13 +1113,10 @@ bool llvm::InlineFunction(CallSite CS, I
>>>> AI, I);
>>>> }
>>>> // Move any dbg.declares describing the allocas into the entry
>> basic
>>>> block.
>>>> + DIBuilder DIB(*Caller->getParent());
>>>> for (auto &I : IFI.StaticAllocas)
>>>> if (auto AI = dyn_cast<AllocaInst>(I))
>>>> - if (auto *DDI = FindAllocaDbgDeclare(AI))
>>>> - if (DDI->getParent() != Caller->begin())
>>>> - Caller->getEntryBlock().getInstList()
>>>> - .splice(AI->getNextNode(), FirstNewBlock->getInstList(),
>>>> - DDI, DDI->getNextNode());
>>>> + replaceDbgDeclareForAlloca(AI, AI, DIB, /*Deref=*/false);
>>>> }
>>>>
>>>> bool InlinedMustTailCalls = false;
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
>>>> URL: http://llvm.org/viewvc/llvm-
>>>>
>> project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=227604&r1=227603&r2=
>>>> 227604&view=diff
>>>>
>> ==========================================================================
>>>> ====
>>>> --- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/Utils/Local.cpp Fri Jan 30 13:37:48 2015
>>>> @@ -1106,10 +1106,11 @@ DbgDeclareInst *llvm::FindAllocaDbgDecla
>>>> }
>>>>
>>>> bool llvm::replaceDbgDeclareForAlloca(AllocaInst *AI, Value
>>>> *NewAllocaAddress,
>>>> - DIBuilder &Builder) {
>>>> + DIBuilder &Builder, bool Deref)
>> {
>>>> DbgDeclareInst *DDI = FindAllocaDbgDeclare(AI);
>>>> if (!DDI)
>>>> return false;
>>>> + DebugLoc Loc = DDI->getDebugLoc();
>>>> DIVariable DIVar(DDI->getVariable());
>>>> DIExpression DIExpr(DDI->getExpression());
>>>> assert((!DIVar || DIVar.isVariable()) &&
>>>> @@ -1117,21 +1118,24 @@ bool llvm::replaceDbgDeclareForAlloca(Al
>>>> if (!DIVar)
>>>> return false;
>>>>
>>>> - // Create a copy of the original DIDescriptor for user variable,
>>>> prepending
>>>> - // "deref" operation to a list of address elements, as new
>>>> llvm.dbg.declare
>>>> - // will take a value storing address of the memory for variable, not
>>>> - // alloca itself.
>>>> - SmallVector<int64_t, 4> NewDIExpr;
>>>> - NewDIExpr.push_back(dwarf::DW_OP_deref);
>>>> - if (DIExpr)
>>>> - for (unsigned i = 0, n = DIExpr.getNumElements(); i < n; ++i)
>>>> - NewDIExpr.push_back(DIExpr.getElement(i));
>>>> + if (Deref) {
>>>> + // Create a copy of the original DIDescriptor for user variable,
>>>> prepending
>>>> + // "deref" operation to a list of address elements, as new
>>>> llvm.dbg.declare
>>>> + // will take a value storing address of the memory for variable,
>> not
>>>> + // alloca itself.
>>>> + SmallVector<int64_t, 4> NewDIExpr;
>>>> + NewDIExpr.push_back(dwarf::DW_OP_deref);
>>>> + if (DIExpr)
>>>
>>> Does it really happen that you have Deref true but no DIExpr?
>>> The result would be a DWARF expression consisting of just a deref.
>>
>> Not sure if I got the question — this seems to be the common usecase in
>> Transforms/Instrumentations/AddressSanitizer.cpp were normal variables are
>> lowered into indirect references inside a buffer.
>
> So, you (can) end up building a new DIExpr that is nothing but DW_OP_deref;
> is the address-to-be-deref'd added somewhere else? Or is that what the
> Builder.insertDeclare() below is doing? I haven't spent a lot of time
> understanding how all the builder pieces fit together.
I haven’t actually looked at how the ASAN transformation is implemented in detail, but ASAN works by transforming local variables into a lookup into a larger buffer, so I assume that the indirection is added by the transformation.
DIBuilder::insertDeclare() only emits an llvm.dbg.declare() intrinsic. [The semantics of dbg.declare are IMO a bit broken, but that’s a separate issue. A dbg.declare describes an alloca, but it treats the alloca as a value rather than an address. This leads to tricky bugs like the one in described in http://reviews.llvm.org/D7016.]
-- adrian
> Thanks,
> --paulr
>
>>
>> -- adrian
>>>
>>>> + for (unsigned i = 0, n = DIExpr.getNumElements(); i < n; ++i)
>>>> + NewDIExpr.push_back(DIExpr.getElement(i));
>>>> + DIExpr = Builder.createExpression(NewDIExpr);
>>>> + }
>>>>
>>>> // Insert llvm.dbg.declare in the same basic block as the original
>>>> alloca,
>>>> // and remove old llvm.dbg.declare.
>>>> BasicBlock *BB = AI->getParent();
>>>> - Builder.insertDeclare(NewAllocaAddress, DIVar,
>>>> - Builder.createExpression(NewDIExpr), BB);
>>>> + Builder.insertDeclare(NewAllocaAddress, DIVar, DIExpr, BB)
>>>> + ->setDebugLoc(Loc);
>>>> DDI->eraseFromParent();
>>>> return true;
>>>> }
>>>>
>>>> Modified: llvm/trunk/test/Transforms/Inline/alloca-dbgdeclare.ll
>>>> URL: http://llvm.org/viewvc/llvm-
>>>> project/llvm/trunk/test/Transforms/Inline/alloca-
>>>> dbgdeclare.ll?rev=227604&r1=227603&r2=227604&view=diff
>>>>
>> ==========================================================================
>>>> ====
>>>> --- llvm/trunk/test/Transforms/Inline/alloca-dbgdeclare.ll (original)
>>>> +++ llvm/trunk/test/Transforms/Inline/alloca-dbgdeclare.ll Fri Jan 30
>>>> 13:37:48 2015
>>>> @@ -40,7 +40,7 @@ entry:
>>>> ; CHECK: define void @_Z3fn5v()
>>>> ; CHECK-NEXT: entry:
>>>> ; CHECK-NEXT: %agg.tmp.sroa.3.i = alloca [20 x i8], align 4
>>>> -; CHECK-NEXT: tail call void @llvm.dbg.declare(metadata [20 x i8]*
>>>> %agg.tmp.sroa.3.i,
>>>> +; 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 !46, metadata !48), !dbg !49
>>>> %agg.tmp.sroa.0.0.copyload = load i32* getelementptr inbounds
>>>> (%struct.A* @b, i64 0, i32 0), align 8, !dbg !50
>>>>
>>>> Modified: llvm/trunk/test/Transforms/Inline/inline_dbg_declare.ll
>>>> URL: http://llvm.org/viewvc/llvm-
>>>>
>> project/llvm/trunk/test/Transforms/Inline/inline_dbg_declare.ll?rev=227604
>>>> &r1=227603&r2=227604&view=diff
>>>>
>> ==========================================================================
>>>> ====
>>>> --- llvm/trunk/test/Transforms/Inline/inline_dbg_declare.ll (original)
>>>> +++ llvm/trunk/test/Transforms/Inline/inline_dbg_declare.ll Fri Jan 30
>>>> 13:37:48 2015
>>>> @@ -92,6 +92,6 @@ attributes #1 = { nounwind readnone }
>>>> !22 = !MDLocation(line: 8, column: 14, scope: !9)
>>>> !23 = !MDLocation(line: 9, column: 1, scope: !9)
>>>>
>>>> -; CHECK: [[m23]] = !{!"0x101\00x\0016777217\000", !4, !5, !8,
>>>> [[CALL_SITE:![0-9]*]]} ; [ DW_TAG_arg_variable ] [x] [line 1]
>>>> -; CHECK: [[CALL_SITE]] = distinct !MDLocation(line: 8, column: 14,
>> scope:
>>>> !9)
>>>> +; CHECK: [[CALL_SITE:![0-9]+]] = distinct !MDLocation(line: 8, column:
>>>> 14, scope: !9)
>>>> +; CHECK: [[m23]] = !{!"0x101\00x\0016777217\000", !4, !5, !8,
>>>> [[CALL_SITE]]} ; [ DW_TAG_arg_variable ] [x] [line 1]
>>>> ; CHECK: [[m24]] = !MDLocation(line: 1, column: 17, scope: !4,
>> inlinedAt:
>>>> [[CALL_SITE]])
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> <bz96056.cpp>
>
More information about the llvm-commits
mailing list