From 58e25d390542978c93ce425410eec0278a401624 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 24 Jul 2013 12:12:17 +0000 Subject: [PATCH] Fix a problem I introduced in r187029 where we would over-eagerly schedule an alloca for another iteration in SROA. This only showed up with a mixture of promotable and unpromotable selects and phis. Added a test case for this. llvm-svn: 187031 --- llvm/lib/Transforms/Scalar/SROA.cpp | 12 +++++++--- llvm/test/Transforms/SROA/phi-and-select.ll | 37 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp index 860c90f..9ec39d2 100644 --- a/llvm/lib/Transforms/Scalar/SROA.cpp +++ b/llvm/lib/Transforms/Scalar/SROA.cpp @@ -2576,7 +2576,6 @@ private: // occurs. if (isSafePHIToSpeculate(PN, &DL)) { Pass.SpeculatablePHIs.insert(&PN); - Pass.Worklist.insert(&NewAI); IsUsedByRewrittenSpeculatableInstructions = true; return true; } @@ -2606,7 +2605,6 @@ private: // speculation occurs. if (isSafeSelectToSpeculate(SI, &DL)) { Pass.SpeculatableSelects.insert(&SI); - Pass.Worklist.insert(&NewAI); IsUsedByRewrittenSpeculatableInstructions = true; return true; } @@ -3090,10 +3088,18 @@ bool SROA::rewritePartition(AllocaInst &AI, AllocaSlices &S, if (Promotable && !Rewriter.isUsedByRewrittenSpeculatableInstructions()) { DEBUG(dbgs() << " and queuing for promotion\n"); PromotableAllocas.push_back(NewAI); - } else if (NewAI != &AI) { + } else if (NewAI != &AI || + (Promotable && + Rewriter.isUsedByRewrittenSpeculatableInstructions())) { // If we can't promote the alloca, iterate on it to check for new // refinements exposed by splitting the current alloca. Don't iterate on an // alloca which didn't actually change and didn't get promoted. + // + // Alternatively, if we could promote the alloca but have speculatable + // instructions then we will speculate them after finishing our processing + // of the original alloca. Mark the new one for re-visiting in the next + // iteration so the speculated operations can be rewritten. + // // FIXME: We should actually track whether the rewriter changed anything. Worklist.insert(NewAI); } diff --git a/llvm/test/Transforms/SROA/phi-and-select.ll b/llvm/test/Transforms/SROA/phi-and-select.ll index 9f2b191..8d82964 100644 --- a/llvm/test/Transforms/SROA/phi-and-select.ll +++ b/llvm/test/Transforms/SROA/phi-and-select.ll @@ -346,6 +346,43 @@ exit: ret i32 %load } +define i32 @test14(i1 %b1, i1 %b2, i32* %ptr) { +; Check for problems when there are both selects and phis and one is +; speculatable toward promotion but the other is not. That should block all of +; the speculation. +; CHECK-LABEL: @test14( +; CHECK: alloca +; CHECK: alloca +; CHECK: select +; CHECK: phi +; CHECK: phi +; CHECK: select +; CHECK: ret i32 + +entry: + %f = alloca i32 + %g = alloca i32 + store i32 0, i32* %f + store i32 0, i32* %g + %f.select = select i1 %b1, i32* %f, i32* %ptr + br i1 %b2, label %then, label %else + +then: + br label %exit + +else: + br label %exit + +exit: + %f.phi = phi i32* [ %f, %then ], [ %f.select, %else ] + %g.phi = phi i32* [ %g, %then ], [ %ptr, %else ] + %f.loaded = load i32* %f.phi + %g.select = select i1 %b1, i32* %g, i32* %g.phi + %g.loaded = load i32* %g.select + %result = add i32 %f.loaded, %g.loaded + ret i32 %result +} + define i32 @PR13905() { ; Check a pattern where we have a chain of dead phi nodes to ensure they are ; deleted and promotion can proceed. -- 2.7.4