[llvm] r242884 - Fix mem2reg to correctly handle allocas only used in a single block

[Michael Kuperstein]

Author: mkuper
Date: Wed Jul 22 05:29:29 2015
New Revision: 242884

URL: http://llvm.org/viewvc/llvm-project?rev=242884&view=rev
Log:
Fix mem2reg to correctly handle allocas only used in a single block

Currently, a load from an alloca that is used in as single block and is not preceded
by a store is replaced by undef. This is not always correct if the single block is
inside a loop.
Fix the logic so that:
1) If there are no stores in the block, replace the load with an undef, as before.
2) If there is a store (regardless of where it is in the block w.r.t the load), bail
out, and let the rest of mem2reg handle this alloca.

Patch by: gil.rapaport at intel.com
Differential Revision: http://reviews.llvm.org/D11355

Added:
    llvm/trunk/test/Transforms/Mem2Reg/pr24179.ll
Modified:
    llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp

Modified: llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp?rev=242884&r1=242883&r2=242884&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp Wed Jul 22 05:29:29 2015
@@ -425,14 +425,17 @@ static bool rewriteSingleStoreAlloca(All
 /// using the Alloca.
 ///
 /// If we cannot promote this alloca (because it is read before it is written),
-/// return true.  This is necessary in cases where, due to control flow, the
-/// alloca is potentially undefined on some control flow paths.  e.g. code like
-/// this is potentially correct:
-///
-///   for (...) { if (c) { A = undef; undef = B; } }
-///
-/// ... so long as A is not used before undef is set.
-static void promoteSingleBlockAlloca(AllocaInst *AI, const AllocaInfo &Info,
+/// return false.  This is necessary in cases where, due to control flow, the
+/// alloca is undefined only on some control flow paths.  e.g. code like
+/// this is correct in LLVM IR:
+///  // A is an alloca with no stores so far
+///  for (...) {
+///    int t = *A;
+///    if (!first_iteration)
+///      use(t);
+///    *A = 42;
+///  }
+static bool promoteSingleBlockAlloca(AllocaInst *AI, const AllocaInfo &Info,
                                      LargeBlockInfo &LBI,
                                      AliasSetTracker *AST) {
   // The trickiest case to handle is when we have large blocks. Because of this,
@@ -467,10 +470,15 @@ static void promoteSingleBlockAlloca(All
                          std::make_pair(LoadIdx,
                                         static_cast<StoreInst *>(nullptr)),
                          less_first());
-
-    if (I == StoresByIndex.begin())
-      // If there is no store before this load, the load takes the undef value.
-      LI->replaceAllUsesWith(UndefValue::get(LI->getType()));
+    if (I == StoresByIndex.begin()) {
+      if (StoresByIndex.empty())
+        // If there are no stores, the load takes the undef value.
+        LI->replaceAllUsesWith(UndefValue::get(LI->getType()));
+      else
+        // There is no store before this load, bail out (load may be affected
+        // by the following stores - see main comment).
+        return false;
+    }
     else
       // Otherwise, there was a store before this load, the load takes its value.
       LI->replaceAllUsesWith(std::prev(I)->second->getOperand(0));
@@ -506,6 +514,7 @@ static void promoteSingleBlockAlloca(All
   }
 
   ++NumLocalPromoted;
+  return true;
 }
 
 void PromoteMem2Reg::run() {
@@ -557,9 +566,8 @@ void PromoteMem2Reg::run() {
 
     // If the alloca is only read and written in one basic block, just perform a
     // linear sweep over the block to eliminate it.
-    if (Info.OnlyUsedInOneBlock) {
-      promoteSingleBlockAlloca(AI, Info, LBI, AST);
-
+    if (Info.OnlyUsedInOneBlock &&
+        promoteSingleBlockAlloca(AI, Info, LBI, AST)) {
       // The alloca has been processed, move on.
       RemoveFromAllocasList(AllocaNum);
       continue;

Added: llvm/trunk/test/Transforms/Mem2Reg/pr24179.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Mem2Reg/pr24179.ll?rev=242884&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Mem2Reg/pr24179.ll (added)
+++ llvm/trunk/test/Transforms/Mem2Reg/pr24179.ll Wed Jul 22 05:29:29 2015
@@ -0,0 +1,44 @@
+; RUN: opt -mem2reg < %s -S | FileCheck %s
+
+declare i32 @def(i32)
+declare i1 @use(i32)
+
+; Special case of a single-BB alloca does not apply here since the load
+; is affected by the following store. Expect this case to be identified
+; and a PHI node to be created.
+define void @test1() {
+; CHECK-LABEL: @test1(
+ entry:
+  %t = alloca i32
+  br label %loop
+
+ loop:
+  %v = load i32, i32* %t
+  %c = call i1 @use(i32 %v)
+; CHECK: [[PHI:%.*]] = phi i32 [ undef, %entry ], [ %n, %loop ]
+; CHECK: call i1 @use(i32 [[PHI]])
+  %n = call i32 @def(i32 7)
+  store i32 %n, i32* %t
+  br i1 %c, label %loop, label %exit
+
+ exit:
+  ret void
+}
+
+; Same as above, except there is no following store. The alloca should just be
+; replaced with an undef
+define void @test2() {
+; CHECK-LABEL: @test2(
+ entry:
+  %t = alloca i32
+  br label %loop
+
+ loop:
+  %v = load i32, i32* %t
+  %c = call i1 @use(i32 %v)
+; CHECK: %c = call i1 @use(i32 undef)
+  br i1 %c, label %loop, label %exit
+
+ exit:
+  ret void
+}