From 080dfb5bda983d7f8e6a95658292306761bdafa1 Mon Sep 17 00:00:00 2001
From: Chandler Carruth <chandlerc@gmail.com>
Date: Mon, 20 Oct 2014 05:34:36 +0000
Subject: [PATCH] Fix a somewhat subtle pair of issues with JumpThreading I
 introduced in r220178. First, the creation routine doesn't insert prior to
 the terminator of the basic block provided, but really at the end of the
 basic block. Instead, get the terminator and insert before that. The next
 issue was that we need to ensure multiple PHI node entries for a single
 predecessor re-use the same cast instruction rather than creating new ones.

All of the logic here was without tests previously. I've reduced and
added a test case from the test suite that crashed without both of these
fixes.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220186 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Transforms/Scalar/JumpThreading.cpp       |  9 ++++--
 test/Transforms/JumpThreading/thread-loads.ll | 31 +++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/lib/Transforms/Scalar/JumpThreading.cpp b/lib/Transforms/Scalar/JumpThreading.cpp
index fbb5c201347..25a8b0cdbd3 100644
--- a/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/lib/Transforms/Scalar/JumpThreading.cpp
@@ -1035,10 +1035,13 @@ bool JumpThreading::SimplifyPartiallyRedundantLoad(LoadInst *LI) {
            "Didn't find entry for predecessor!");
 
     // If we have an available predecessor but it requires casting, insert the
-    // cast in the predecessor and use the cast.
-    Value *PredV = I->second;
+    // cast in the predecessor and use the cast. Note that we have to update the
+    // AvailablePreds vector as we go so that all of the PHI entries for this
+    // predecessor use the same bitcast.
+    Value *&PredV = I->second;
     if (PredV->getType() != LI->getType())
-      PredV = CastInst::Create(CastInst::BitCast, PredV, LI->getType(), "", P);
+      PredV = CastInst::Create(CastInst::BitCast, PredV, LI->getType(), "",
+                               P->getTerminator());
 
     PN->addIncoming(PredV, I->first);
   }
diff --git a/test/Transforms/JumpThreading/thread-loads.ll b/test/Transforms/JumpThreading/thread-loads.ll
index e5bf64b9e25..b13b767bdea 100644
--- a/test/Transforms/JumpThreading/thread-loads.ll
+++ b/test/Transforms/JumpThreading/thread-loads.ll
@@ -75,6 +75,37 @@ bb3:		; preds = %bb1
 	ret i32 %res.0
 }
 
+define i32 @test3(i8** %x, i1 %f) {
+; Correctly thread loads of different (but compatible) types, placing bitcasts
+; as necessary in the predecessors. This is especially tricky because the same
+; predecessor ends up with two entries in the PHI node and they must share
+; a single cast.
+; CHECK-LABEL: @test3(
+entry:
+  %0 = bitcast i8** %x to i32**
+  %1 = load i32** %0, align 8
+  br i1 %f, label %if.end57, label %if.then56
+; CHECK: %[[LOAD:.*]] = load i32**
+; CHECK: %[[CAST:.*]] = bitcast i32* %[[LOAD]] to i8*
+
+if.then56:
+  br label %if.end57
+
+if.end57:
+  %2 = load i8** %x, align 8
+  %tobool59 = icmp eq i8* %2, null
+  br i1 %tobool59, label %return, label %if.then60
+; CHECK: %[[PHI:.*]] = phi i8* [ %[[CAST]], %[[PRED:[^ ]+]] ], [ %[[CAST]], %[[PRED]] ]
+; CHECK-NEXT: %[[CMP:.*]] = icmp eq i8* %[[PHI]], null
+; CHECK-NEXT: br i1 %[[CMP]]
+
+if.then60:
+  ret i32 42
+
+return:
+  ret i32 13
+}
+
 !0 = metadata !{metadata !3, metadata !3, i64 0}
 !1 = metadata !{metadata !"omnipotent char", metadata !2}
 !2 = metadata !{metadata !"Simple C/C++ TBAA", null}
-- 
2.34.1