[llvm-commits] [llvm] r94260 - in /llvm/trunk: lib/Transforms/Utils/PromoteMemoryToRegister.cpp test/Transforms/Mem2Reg/ConvertDebugInfo.ll
Victor Hernandez
vhernandez at apple.com
Mon Jan 25 09:43:26 PST 2010
On Jan 24, 2010, at 11:39 PM, Chris Lattner wrote:
>
> On Jan 22, 2010, at 4:17 PM, Victor Hernandez wrote:
>
>> Author: hernande
>> Date: Fri Jan 22 18:17:34 2010
>> New Revision: 94260
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=94260&view=rev
>> Log:
>> In mem2reg, for all alloca/stores that get promoted where the alloca has an associated llvm.dbg.declare instrinsic, insert an llvm.dbg.var intrinsic before each store
>
> Hi Victor,
>
>> -
>> + void ConvertDebugDeclareToDebugValue(DbgDeclareInst *DDI, StoreInst* SI,
>> + uint64_t Offset);
>
> You're placing your *'s inconsistently. Please format code like DDI, not like SI.
>
>>
>> +/// Finds the llvm.dbg.declare intrinsic corresponding to an alloca if any.
>> +static DbgDeclareInst *findDbgDeclare(AllocaInst *AI) {
>> + Function *F = AI->getParent()->getParent();
>> + for (Function::iterator FI = F->begin(), FE = F->end(); FI != FE; ++FI)
>> + for (BasicBlock::iterator BI = (*FI).begin(), BE = (*FI).end();
>> + BI != BE; ++BI)
>> + if (DbgDeclareInst *DDI = dyn_cast<DbgDeclareInst>(BI))
>> + if (DDI->getAddress() == AI)
>> + return DDI;
>
> This is *HIGHLY* inefficient. Did you look at the impact of this on compile time? I suspect that it will have a high hit particularly for stuff that doesn't use debug information.
>
> A better solution is to add a MDNode method that queries to see if an mdnode exists. Given that, you could write something like:
>
> // See if there is a function local MDNode with one argument that is the alloca.
> if (MDNode *DebugNode = MDNode::getIfExists(Ctx, &AI, 1, true))
> for (Value::use_iterator UI = DebugNode->use_begin(), E = DebugNode->use_end(); UI != E; ++UI)
> if (DbgDeclareInst *DDI = dyn_cast<DbgDeclareInst>(BI))
> ... found it ...
>
> Also, you should only do this once per alloca, not once per store to the alloca. Please revert this patch until the algorithm can be fixed.
I will revert it now.
I don't understand how MDNode::getIfExists() will be faster, because just to determine requires much work when there is not metadata. I still need to iterate over all operands of all instructions (or at least the intrinsic calls) to determine if there is a function-local metadata hiding in there. I am not aware of an existing more efficient means to do this, since metadata does not show up on use lists.
How about a function keeping a list of all its funciton-local metadata? Is that too expensive? Will it to be too easy to invalidate?
> -Chris
>
>
>> +
>> + return 0;
>> +}
>> +
>> void PromoteMem2Reg::run() {
>> Function &F = *DF.getRoot()->getParent();
>>
>> @@ -344,6 +360,8 @@
>>
>> // Finally, after the scan, check to see if the store is all that is left.
>> if (Info.UsingBlocks.empty()) {
>> + // Record debuginfo for the store before removing it.
>> + ConvertDebugDeclareToDebugValue(findDbgDeclare(AI), Info.OnlyStore, 0);
>> // Remove the (now dead) store and alloca.
>> Info.OnlyStore->eraseFromParent();
>> LBI.deleteValue(Info.OnlyStore);
>> @@ -370,8 +388,11 @@
>> if (Info.UsingBlocks.empty()) {
>>
>> // Remove the (now dead) stores and alloca.
>> + DbgDeclareInst *DDI = findDbgDeclare(AI);
>> while (!AI->use_empty()) {
>> StoreInst *SI = cast<StoreInst>(AI->use_back());
>> + // Record debuginfo for the store before removing it.
>> + ConvertDebugDeclareToDebugValue(DDI, SI, 0);
>> SI->eraseFromParent();
>> LBI.deleteValue(SI);
>> }
>> @@ -833,6 +854,18 @@
>> }
>> }
>>
>> +// Inserts a llvm.dbg.value instrinsic before the stores to an alloca'd value
>> +// that has an associated llvm.dbg.decl intrinsic.
>> +void PromoteMem2Reg::ConvertDebugDeclareToDebugValue(DbgDeclareInst *DDI,
>> + StoreInst* SI,
>> + uint64_t Offset) {
>> + if (!DDI) return;
>> +
>> + if (!DIF)
>> + DIF = new DIFactory(*SI->getParent()->getParent()->getParent());
>> + DIF->InsertDbgValueIntrinsic(SI->getOperand(0), Offset,
>> + DIVariable(DDI->getVariable()), SI);
>> +}
>>
>> // QueuePhiNode - queues a phi-node to be added to a basic-block for a specific
>> // Alloca returns true if there wasn't already a phi-node for that variable
>> @@ -946,6 +979,8 @@
>>
>> // what value were we writing?
>> IncomingVals[ai->second] = SI->getOperand(0);
>> + // Record debuginfo for the store before removing it.
>> + ConvertDebugDeclareToDebugValue(findDbgDeclare(Dest), SI, 0);
>> BB->getInstList().erase(SI);
>> }
>> }
>>
>> Added: llvm/trunk/test/Transforms/Mem2Reg/ConvertDebugInfo.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Mem2Reg/ConvertDebugInfo.ll?rev=94260&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/Mem2Reg/ConvertDebugInfo.ll (added)
>> +++ llvm/trunk/test/Transforms/Mem2Reg/ConvertDebugInfo.ll Fri Jan 22 18:17:34 2010
>> @@ -0,0 +1,31 @@
>> +; RUN: opt < %s -mem2reg -S | FileCheck %s
>> +
>> +target datalayout = "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
>> +
>> +define double @testfunc(i32 %i, double %j) {
>> + %I = alloca i32 ; <i32*> [#uses=4]
>> + call void @llvm.dbg.declare(metadata !{i32* %I}, metadata !0)
>> + %J = alloca double ; <double*> [#uses=2]
>> + call void @llvm.dbg.declare(metadata !{double* %J}, metadata !1)
>> +; CHECK: call void @llvm.dbg.value(metadata !{i32 %i}, i64 0, metadata !0)
>> + store i32 %i, i32* %I
>> +; CHECK: call void @llvm.dbg.value(metadata !{double %j}, i64 0, metadata !1)
>> + store double %j, double* %J
>> + %t1 = load i32* %I ; <i32> [#uses=1]
>> + %t2 = add i32 %t1, 1 ; <i32> [#uses=1]
>> + store i32 %t2, i32* %I
>> + %t3 = load i32* %I ; <i32> [#uses=1]
>> + %t4 = sitofp i32 %t3 to double ; <double> [#uses=1]
>> + %t5 = load double* %J ; <double> [#uses=1]
>> + %t6 = fmul double %t4, %t5 ; <double> [#uses=1]
>> + ret double %t6
>> +}
>> +
>> +declare void @llvm.dbg.declare(metadata, metadata) nounwind readnone
>> +
>> +!bar = !{!0}
>> +!foo = !{!2}
>> +
>> +!0 = metadata !{i32 459008, metadata !1, metadata !"foo", metadata !2, i32 5, metadata !"foo"} ; [ DW_TAG_auto_variable ]
>> +!1 = metadata !{i32 459008, metadata !1, metadata !"foo", metadata !0, i32 5, metadata !1} ; [ DW_TAG_auto_variable ]
>> +!2 = metadata !{i32 458804, i32 0, metadata !2, metadata !"foo", metadata !"bar", metadata !"bar", metadata !2, i32 3, metadata !0, i1 false, i1 true} ; [ DW_TAG_variable ]
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list