Handle conflicting lifetime markers conservatively.

Avoid infinite looping in AllocaManager::computeInterBlockLiveness,
without using new temporary variables. This also makes it clear that
the algorithm is conservative in the face of conflicting liveness
intrinsics.

2 files changed, 43 insertions, 16 deletions

diff --git a/lib/Target/JSBackend/AllocaManager.cpp b/lib/Target/JSBackend/AllocaManager.cpp
index b49e6c4de1..a77ddc7324 100644
--- a/lib/Target/JSBackend/AllocaManager.cpp
+++ b/lib/Target/JSBackend/AllocaManager.cpp
@@ -249,15 +249,11 @@ void AllocaManager::computeInterBlockLiveness() {
     // If it contains new live blocks, prepare to propagate them.
     if (Temp.test(BLI.LiveIn)) {
       BLI.LiveIn |= Temp;
-      BitVector LiveOut = BLI.LiveOut;
+      Temp.reset(BLI.End);
       BLI.LiveOut |= Temp;
-      BLI.LiveOut.reset(BLI.End);
-      // If we actually added to live-out, re-process them
-      if (BLI.LiveOut.test(LiveOut)) {
-        for (succ_const_iterator SI = succ_begin(BB), SE = succ_end(BB);
-             SI != SE; ++SI) {
-          InterBlockWorklist.insert(*SI);
-        }
+      for (succ_const_iterator SI = succ_begin(BB), SE = succ_end(BB);
+           SI != SE; ++SI) {
+        InterBlockWorklist.insert(*SI);
       }
     }
     Temp.reset();
@@ -272,15 +268,11 @@ void AllocaManager::computeInterBlockLiveness() {
     if (Temp.test(BLI.LiveOut)) {
       // TODO: As above, what are the semantics of a standalone lifetime end?
       BLI.LiveOut |= Temp;
-      BitVector LiveIn = BLI.LiveIn;
+      Temp.reset(BLI.Start);
       BLI.LiveIn |= Temp;
-      BLI.LiveIn.reset(BLI.Start);
-      // If we actually added to live-in, re-process them
-      if (BLI.LiveIn.test(LiveIn)) {
-        for (const_pred_iterator PI = pred_begin(BB), PE = pred_end(BB);
-             PI != PE; ++PI) {
-          InterBlockWorklist.insert(*PI);
-        }
+      for (const_pred_iterator PI = pred_begin(BB), PE = pred_end(BB);
+           PI != PE; ++PI) {
+        InterBlockWorklist.insert(*PI);
       }
     }
     Temp.reset();
diff --git a/test/CodeGen/JS/alloca-contradiction.ll b/test/CodeGen/JS/alloca-contradiction.ll
new file mode 100644
index 0000000000..82b1bf87c9
--- /dev/null
+++ b/test/CodeGen/JS/alloca-contradiction.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s
+
+; In theory, the @llvm.lifetime intrinsics shouldn't contradict each other, but
+; in practice they apparently do sometimes. When they do, we should probably be
+; conservative.
+
+target datalayout = "e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-p:32:32:32-v128:32:128-n32-S128"
+target triple = "asmjs-unknown-emscripten"
+
+; Don't merge these two allocas, even though lifetime markers may initially
+; appear to indicate that it's safe, because they also indicate that it's
+; unsafe.
+
+; CHECK: foo
+; CHECK: HEAP8[$p] = 0;
+; CHECK: HEAP8[$q] = 1;
+define void @foo() nounwind {
+entry:
+  %p = alloca i8
+  %q = alloca i8
+  br label %loop
+
+loop:
+  call void @llvm.lifetime.end(i64 1, i8* %q)
+  store volatile i8 0, i8* %p
+  store volatile i8 1, i8* %q
+  call void @llvm.lifetime.start(i64 1, i8* %p)
+  br i1 undef, label %loop, label %end
+
+end:                                              ; preds = %red
+  ret void
+}
+
+declare void @llvm.lifetime.start(i64, i8* nocapture) nounwind
+declare void @llvm.lifetime.end(i64, i8* nocapture) nounwind