From 2c78f9499c162b3448aac585374e7d5d52f64605 Mon Sep 17 00:00:00 2001
From: Philip Reames <listmail@philipreames.com>
Date: Fri, 22 May 2015 02:14:05 +0000
Subject: [PATCH] [LICM] Sinking doesn't involve the preheader

PR23608 pointed out that using the preheader to gain a context instruction isn't always legal because a loop might not have a preheader.  When looking into that, I realized that using the preheader to determine legality for sinking is questionable at best.  Given no test covers that case and the original commit didn't seem to intend it, I restructured the code to only ask context sensative queries for hoising of loads and stores.  This is effectively a partial revert of 237593.



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@237985 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Transforms/Scalar/LICM.cpp  | 16 +++++++----
 test/Transforms/LICM/pr23608.ll | 50 +++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 5 deletions(-)
 create mode 100644 test/Transforms/LICM/pr23608.ll

diff --git a/lib/Transforms/Scalar/LICM.cpp b/lib/Transforms/Scalar/LICM.cpp
index b71fd56a66c..f0e6d641b18 100644
--- a/lib/Transforms/Scalar/LICM.cpp
+++ b/lib/Transforms/Scalar/LICM.cpp
@@ -84,7 +84,8 @@ static bool isSafeToExecuteUnconditionally(const Instruction &Inst,
                                            const DominatorTree *DT,
                                            const TargetLibraryInfo *TLI,
                                            const Loop *CurLoop,
-                                           const LICMSafetyInfo *SafetyInfo);
+                                           const LICMSafetyInfo *SafetyInfo,
+                                           const Instruction *CtxI = nullptr);
 static bool pointerInvalidatedByLoop(Value *V, uint64_t Size,
                                      const AAMDNodes &AAInfo, 
                                      AliasSetTracker *CurAST);
@@ -388,7 +389,8 @@ bool llvm::hoistRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI,
       //
       if (CurLoop->hasLoopInvariantOperands(&I) &&
           canSinkOrHoistInst(I, AA, DT, TLI, CurLoop, CurAST, SafetyInfo) &&
-          isSafeToExecuteUnconditionally(I, DT, TLI, CurLoop, SafetyInfo))
+          isSafeToExecuteUnconditionally(I, DT, TLI, CurLoop, SafetyInfo,
+                                 CurLoop->getLoopPreheader()->getTerminator()))
         Changed |= hoist(I, CurLoop->getLoopPreheader());
     }
 
@@ -487,7 +489,11 @@ bool canSinkOrHoistInst(Instruction &I, AliasAnalysis *AA, DominatorTree *DT,
       !isa<InsertValueInst>(I))
     return false;
 
-  return isSafeToExecuteUnconditionally(I, DT, TLI, CurLoop, SafetyInfo);
+  // TODO: Plumb the context instruction through to make hoisting and sinking
+  // more powerful. Hoisting of loads already works due to the special casing
+  // above. 
+  return isSafeToExecuteUnconditionally(I, DT, TLI, CurLoop, SafetyInfo,
+                                        nullptr);
 }
 
 /// Returns true if a PHINode is a trivially replaceable with an
@@ -647,8 +653,8 @@ static bool isSafeToExecuteUnconditionally(const Instruction &Inst,
                                            const DominatorTree *DT,
                                            const TargetLibraryInfo *TLI,
                                            const Loop *CurLoop,
-                                           const LICMSafetyInfo *SafetyInfo) {
-  const Instruction *CtxI = CurLoop->getLoopPreheader()->getTerminator();
+                                           const LICMSafetyInfo *SafetyInfo,
+                                           const Instruction *CtxI) {
   if (isSafeToSpeculativelyExecute(&Inst, CtxI, DT, TLI))
     return true;
 
diff --git a/test/Transforms/LICM/pr23608.ll b/test/Transforms/LICM/pr23608.ll
new file mode 100644
index 00000000000..249bc6bf5f6
--- /dev/null
+++ b/test/Transforms/LICM/pr23608.ll
@@ -0,0 +1,50 @@
+; RUN: opt -S -licm %s | FileCheck %s
+; ModuleID = '../pr23608.ll'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.PyFrameObject = type { i32 }
+
+@a = common global %struct.PyFrameObject* null, align 8
+@__msan_origin_tls = external thread_local(initialexec) global i32
+
+define void @fn1() {
+entry:
+  br label %indirectgoto
+
+while.cond:                                       ; preds = %indirectgoto, %bb15
+  %tmp = load %struct.PyFrameObject*, %struct.PyFrameObject** @a, align 8
+  %_msld = load i64, i64* inttoptr (i64 and (i64 ptrtoint (%struct.PyFrameObject** @a to i64), i64 -70368744177665) to i64*), align 8
+  %tmp1 = load i32, i32* inttoptr (i64 add (i64 and (i64 ptrtoint (%struct.PyFrameObject** @a to i64), i64 -70368744177665), i64 35184372088832) to i32*), align 8
+  %f_iblock = getelementptr inbounds %struct.PyFrameObject, %struct.PyFrameObject* %tmp, i64 0, i32 0
+  br label %bb2
+
+bb:                                               ; preds = %while.cond
+  call void @__msan_warning_noreturn()
+  unreachable
+
+bb2:                                              ; preds = %while.cond
+  %tmp3 = load i32, i32* %f_iblock, align 4
+  %tmp4 = ptrtoint i32* %f_iblock to i64
+  %tmp8 = inttoptr i64 %tmp4 to i32*
+  %tobool = icmp eq i64 %tmp4, 0
+  br i1 %tobool, label %bb13, label %bb15
+
+bb13:                                             ; preds = %bb2
+; CHECK-LABEL bb13:
+; CHECK: %tmp8.le = inttoptr
+  %.lcssa7 = phi i32* [ %tmp8, %bb2 ]
+  call void @__msan_warning_noreturn()
+  unreachable
+
+bb15:                                             ; preds = %bb2
+  br i1 %tobool, label %while.end, label %while.cond
+
+while.end:                                        ; preds = %bb15
+  ret void
+
+indirectgoto:                                     ; preds = %indirectgoto, %entry
+  indirectbr i8* null, [label %indirectgoto, label %while.cond]
+}
+
+declare void @__msan_warning_noreturn()
-- 
2.34.1