HotColdSplit: fix invalid SSA due to outlining

The test used to fail with an invalid phi node: the two predecessors were outlined
and the SSA representation was left invalid. The patch adds the exit block to the
cold region.

llvm-svn: 342277

3 files changed, 45 insertions, 24 deletions

diff --git a/llvm/lib/Transforms/IPO/HotColdSplitting.cpp b/llvm/lib/Transforms/IPO/HotColdSplitting.cpp
index c4846f9..be64da5 100644
--- a/llvm/lib/Transforms/IPO/HotColdSplitting.cpp
+++ b/llvm/lib/Transforms/IPO/HotColdSplitting.cpp
@@ -219,9 +219,8 @@ public:
 
 private:
   bool shouldOutlineFrom(const Function &F) const;
-  Function *outlineColdBlocks(Function &F,
-                              const DenseSetBB &ColdBlock,
-                              DominatorTree *DT, PostDomTree *PDT);
+  const Function *outlineColdBlocks(Function &F, const DenseSetBB &ColdBlock,
+                                    DominatorTree *DT, PostDomTree *PDT);
   Function *extractColdRegion(const SmallVectorImpl<BasicBlock *> &Region,
                               DominatorTree *DT, BlockFrequencyInfo *BFI,
                               OptimizationRemarkEmitter &ORE);
@@ -264,9 +263,12 @@ public:
 } // end anonymous namespace
 
 // Returns false if the function should not be considered for hot-cold split
-// optimization. Already outlined functions have coldcc so no need to check
-// for them here.
+// optimization.
 bool HotColdSplitting::shouldOutlineFrom(const Function &F) const {
+  // Do not try to outline again from an already outlined cold function.
+  if (OutlinedFunctions.count(&F))
+    return false;
+
   if (F.size() <= 2)
     return false;
 
@@ -314,7 +316,7 @@ HotColdSplitting::extractColdRegion(const SmallVectorImpl<BasicBlock *> &Region,
       CS.setCallingConv(CallingConv::Cold);
     }
     CI->setIsNoInline();
-    LLVM_DEBUG(llvm::dbgs() << "Outlined Region at block: " << Region.front());
+    LLVM_DEBUG(llvm::dbgs() << "Outlined Region: " << *OutF);
     return OutF;
   }
 
@@ -328,10 +330,10 @@ HotColdSplitting::extractColdRegion(const SmallVectorImpl<BasicBlock *> &Region,
 }
 
 // Return the function created after outlining, nullptr otherwise.
-Function *HotColdSplitting::outlineColdBlocks(Function &F,
-                                              const DenseSetBB &HotBlocks,
-                                              DominatorTree *DT,
-                                              PostDomTree *PDT) {
+const Function *HotColdSplitting::outlineColdBlocks(Function &F,
+                                                    const DenseSetBB &HotBlocks,
+                                                    DominatorTree *DT,
+                                                    PostDomTree *PDT) {
   auto BFI = GetBFI(F);
   auto &ORE = (*GetORE)(F);
   // Walking the dominator tree allows us to find the largest
@@ -342,12 +344,10 @@ Function *HotColdSplitting::outlineColdBlocks(Function &F,
     if (PSI->isColdBB(BB, BFI) || !HotBlocks.count(BB)) {
       SmallVector<BasicBlock *, 4> ValidColdRegion, Region;
       BasicBlock *Exit = (*PDT)[BB]->getIDom()->getBlock();
-      // We might need a virtual exit which post-dominates all basic blocks.
-      if (!Exit)
-        continue;
       BasicBlock *ExitColdRegion = nullptr;
+
       // Estimated cold region between a BB and its dom-frontier.
-      while (isSingleEntrySingleExit(BB, Exit, DT, PDT, Region) &&
+      while (Exit && isSingleEntrySingleExit(BB, Exit, DT, PDT, Region) &&
              isOutlineCandidate(Region, Exit)) {
         ExitColdRegion = Exit;
         ValidColdRegion = Region;
@@ -361,6 +361,7 @@ Function *HotColdSplitting::outlineColdBlocks(Function &F,
           continue;
 
         ++NumColdSESEFound;
+        ValidColdRegion.push_back(ExitColdRegion);
         // Candidate for outlining. FIXME: Continue outlining.
         return extractColdRegion(ValidColdRegion, DT, BFI, ORE);
       }
@@ -380,7 +381,7 @@ bool HotColdSplitting::run(Module &M) {
     if (EnableStaticAnalyis) // Static analysis of cold blocks.
       HotBlocks = getHotBlocks(F);
 
-    auto Outlined = outlineColdBlocks(F, HotBlocks, &DT, &PDT);
+    const Function *Outlined = outlineColdBlocks(F, HotBlocks, &DT, &PDT);
     if (Outlined)
       OutlinedFunctions.insert(Outlined);
   }
diff --git a/llvm/test/Transforms/HotColdSplit/split-cold-1.ll b/llvm/test/Transforms/HotColdSplit/split-cold-1.ll
index c8948bc..fdb3eb0 100644
--- a/llvm/test/Transforms/HotColdSplit/split-cold-1.ll
+++ b/llvm/test/Transforms/HotColdSplit/split-cold-1.ll
@@ -1,15 +1,9 @@
 ; RUN: opt -hotcoldsplit -S < %s | FileCheck %s
-source_filename = "bugpoint-output-054409e.bc"
-target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.13.0"
-
-declare i32 @__gxx_personality_v0(...)
 
 ; Outlined function is called from a basic block named codeRepl
 ; CHECK: codeRepl:
 ; CHECK-NEXT: call void @foo
-; Function Attrs: ssp uwtable
-define hidden void @foo() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+define void @foo() {
 entry:
   br i1 undef, label %if.then, label %if.end
 
@@ -28,5 +22,3 @@ cleanup40:                                        ; preds = %if.then12
 return:                                           ; preds = %cleanup40
   ret void
 }
-
-
diff --git a/llvm/test/Transforms/HotColdSplit/split-cold-2.ll b/llvm/test/Transforms/HotColdSplit/split-cold-2.ll
new file mode 100644
index 0000000..ce8e7b5
--- /dev/null
+++ b/llvm/test/Transforms/HotColdSplit/split-cold-2.ll
@@ -0,0 +1,28 @@
+; RUN: opt -hotcoldsplit -S < %s
+
+; Make sure this compiles. This test used to fail with an invalid phi node: the
+; two predecessors were outlined and the SSA representation was invalid.
+
+define void @fun() {
+entry:
+  br i1 undef, label %if.then, label %if.else
+
+if.then:
+  unreachable
+
+if.else:
+  br label %if.then4
+
+if.then4:
+  br i1 undef, label %if.then5, label %if.end
+
+if.then5:
+  br label %cleanup
+
+if.end:
+  br label %cleanup
+
+cleanup:
+  %cleanup.dest.slot.0 = phi i32 [ 1, %if.then5 ], [ 0, %if.end ]
+  unreachable
+}