From 469996400c4ab89d5ddb2eb80acc1c1553439f07 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sat, 4 Sep 2010 00:12:30 +0000 Subject: [PATCH] fix a bug in my licm rewrite when a load from the promoted memory location is being re-stored to the memory location. We would get a dangling pointer from the SSAUpdate data structure and miss a use. This fixes PR8068 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@113042 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/LICM.cpp | 32 +++++++++++++++++++++++++++++--- test/Transforms/LICM/crash.ll | 27 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 test/Transforms/LICM/crash.ll diff --git a/lib/Transforms/Scalar/LICM.cpp b/lib/Transforms/Scalar/LICM.cpp index bf06fee6efb..0399329f0dd 100644 --- a/lib/Transforms/Scalar/LICM.cpp +++ b/lib/Transforms/Scalar/LICM.cpp @@ -743,6 +743,7 @@ void LICM::PromoteAliasSet(AliasSet &AS) { // Okay, now we can iterate over all the blocks in the loop with uses, // processing them. Keep track of which loads are loading a live-in value. SmallVector LiveInLoads; + DenseMap ReplacedLoads; for (unsigned LoopUse = 0, e = LoopUses.size(); LoopUse != e; ++LoopUse) { Instruction *User = LoopUses[LoopUse]; @@ -792,15 +793,17 @@ void LICM::PromoteAliasSet(AliasSet &AS) { Value *StoredValue = 0; for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) { if (LoadInst *L = dyn_cast(II)) { - // If this is a load to an unrelated pointer, ignore it. + // If this is a load from an unrelated pointer, ignore it. if (!PointerMustAliases.count(L->getOperand(0))) continue; // If we haven't seen a store yet, this is a live in use, otherwise // use the stored value. - if (StoredValue) + if (StoredValue) { L->replaceAllUsesWith(StoredValue); - else + ReplacedLoads[L] = StoredValue; + } else { LiveInLoads.push_back(L); + } continue; } @@ -846,12 +849,35 @@ void LICM::PromoteAliasSet(AliasSet &AS) { Value *NewVal = SSA.GetValueInMiddleOfBlock(ALoad->getParent()); ALoad->replaceAllUsesWith(NewVal); CurAST->copyValue(ALoad, NewVal); + ReplacedLoads[ALoad] = NewVal; } // Now that everything is rewritten, delete the old instructions from the body // of the loop. They should all be dead now. for (unsigned i = 0, e = LoopUses.size(); i != e; ++i) { Instruction *User = LoopUses[i]; + + // If this is a load that still has uses, then the load must have been added + // as a live value in the SSAUpdate data structure for a block (e.g. because + // the loaded value was stored later). In this case, we need to recursively + // propagate the updates until we get to the real value. + if (!User->use_empty()) { + Value *NewVal = ReplacedLoads[User]; + assert(NewVal && "not a replaced load?"); + + // Propagate down to the ultimate replacee. The intermediately loads + // could theoretically already have been deleted, so we don't want to + // dereference the Value*'s. + DenseMap::iterator RLI = ReplacedLoads.find(NewVal); + while (RLI != ReplacedLoads.end()) { + NewVal = RLI->second; + RLI = ReplacedLoads.find(NewVal); + } + + User->replaceAllUsesWith(NewVal); + CurAST->copyValue(User, NewVal); + } + CurAST->deleteValue(User); User->eraseFromParent(); } diff --git a/test/Transforms/LICM/crash.ll b/test/Transforms/LICM/crash.ll new file mode 100644 index 00000000000..325f250bd58 --- /dev/null +++ b/test/Transforms/LICM/crash.ll @@ -0,0 +1,27 @@ +; RUN: opt -licm %s -disable-output + +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64" +target triple = "x86_64-apple-darwin10.0.0" + + +; PR8068 +@g_12 = external global i8, align 1 +define void @test1() nounwind ssp { +entry: + br label %for.body + +for.body: ; preds = %for.cond, %bb.nph + store i8 0, i8* @g_12, align 1 + %tmp6 = load i8* @g_12, align 1 + br label %for.cond + +for.cond: ; preds = %for.body + store i8 %tmp6, i8* @g_12, align 1 + br i1 false, label %for.cond.for.end10_crit_edge, label %for.body + +for.cond.for.end10_crit_edge: ; preds = %for.cond + br label %for.end10 + +for.end10: ; preds = %for.cond.for.end10_crit_edge, %entry + ret void +}