<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Looks like this might be incompatible with Chandler's change in r344504. On Top-of-tree I am seeing:</div><div dir="ltr"><br></div><div dir="ltr"><div dir="ltr"><font face="monospace, monospace">/path/to/llvm/lib/Transforms/IPO/HotColdSplitting.cpp:153:25: error: cannot initialize a variable of type 'const llvm::TerminatorInst *' with an rvalue of type 'const llvm::Instruction *'</font></div><div dir="ltr"><font face="monospace, monospace"> const TerminatorInst *I = BB.getTerminator();</font></div><div><br></div><div>Cheers,</div><div>Lang.</div><div><br></div><div><br></div><div><br></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 15, 2018 at 2:45 PM Sebastian Pop via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: spop<br>
Date: Mon Oct 15 14:43:11 2018<br>
New Revision: 344558<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=344558&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=344558&view=rev</a><br>
Log:<br>
[hot-cold-split] fix static analysis of cold regions<br>
<br>
Make the code of blockEndsInUnreachable to match the function<br>
blockEndsInUnreachable in CodeGen/BranchFolding.cpp. I also have<br>
added a note to make sure the code of this function will not be<br>
modified unless the back-end version is also modified.<br>
<br>
An early return before outlining has been added to avoid<br>
outlining the full function body when the first block in the<br>
function is marked cold.<br>
<br>
The static analysis of cold code has been amended to avoid<br>
marking the whole function as cold by back-propagation<br>
because the back-propagation would mark blocks with return<br>
statements as cold.<br>
<br>
The patch adds debug statements to help discover these problems.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D52904" rel="noreferrer" target="_blank">https://reviews.llvm.org/D52904</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp<br>
llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll<br>
llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp?rev=344558&r1=344557&r2=344558&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp?rev=344558&r1=344557&r2=344558&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp Mon Oct 15 14:43:11 2018<br>
@@ -101,14 +101,19 @@ static bool isSingleEntrySingleExit(Basi<br>
return true;<br>
}<br>
<br>
+// Same as blockEndsInUnreachable in CodeGen/BranchFolding.cpp. Do not modify<br>
+// this function unless you modify the MBB version as well.<br>
+//<br>
+/// A no successor, non-return block probably ends in unreachable and is cold.<br>
+/// Also consider a block that ends in an indirect branch to be a return block,<br>
+/// since many targets use plain indirect branches to return.<br>
bool blockEndsInUnreachable(const BasicBlock &BB) {<br>
+ if (!succ_empty(&BB))<br>
+ return false;<br>
if (BB.empty())<br>
return true;<br>
const Instruction *I = BB.getTerminator();<br>
- if (isa<ReturnInst>(I) || isa<IndirectBrInst>(I))<br>
- return true;<br>
- // Unreachable blocks do not have any successor.<br>
- return succ_empty(&BB);<br>
+ return !(isa<ReturnInst>(I) || isa<IndirectBrInst>(I));<br>
}<br>
<br>
static bool exceptionHandlingFunctions(const CallInst *CI) {<br>
@@ -123,8 +128,7 @@ static bool exceptionHandlingFunctions(c<br>
FName == "__cxa_end_catch";<br>
}<br>
<br>
-static<br>
-bool unlikelyExecuted(const BasicBlock &BB) {<br>
+static bool unlikelyExecuted(const BasicBlock &BB) {<br>
if (blockEndsInUnreachable(BB))<br>
return true;<br>
// Exception handling blocks are unlikely executed.<br>
@@ -145,13 +149,32 @@ bool unlikelyExecuted(const BasicBlock &<br>
return false;<br>
}<br>
<br>
+static bool returnsOrHasSideEffects(const BasicBlock &BB) {<br>
+ const TerminatorInst *I = BB.getTerminator();<br>
+ if (isa<ReturnInst>(I) || isa<IndirectBrInst>(I) || isa<InvokeInst>(I))<br>
+ return true;<br>
+<br>
+ for (const Instruction &I : BB)<br>
+ if (const CallInst *CI = dyn_cast<CallInst>(&I)) {<br>
+ if (CI->hasFnAttr(Attribute::NoReturn))<br>
+ return true;<br>
+<br>
+ if (isa<InlineAsm>(CI->getCalledValue()))<br>
+ return true;<br>
+ }<br>
+<br>
+ return false;<br>
+}<br>
+<br>
static DenseSetBB getHotBlocks(Function &F) {<br>
<br>
// Mark all cold basic blocks.<br>
DenseSetBB ColdBlocks;<br>
for (BasicBlock &BB : F)<br>
- if (unlikelyExecuted(BB))<br>
+ if (unlikelyExecuted(BB)) {<br>
+ LLVM_DEBUG(llvm::dbgs() << "\nForward propagation marks cold: " << BB);<br>
ColdBlocks.insert((const BasicBlock *)&BB);<br>
+ }<br>
<br>
// Forward propagation: basic blocks are hot when they are reachable from the<br>
// beginning of the function through a path that does not contain cold blocks.<br>
@@ -203,7 +226,12 @@ static DenseSetBB getHotBlocks(Function<br>
if (ColdBlocks.count(It))<br>
continue;<br>
<br>
+ // Do not back-propagate to blocks that return or have side effects.<br>
+ if (returnsOrHasSideEffects(*It))<br>
+ continue;<br>
+<br>
// Move the block from HotBlocks to ColdBlocks.<br>
+ LLVM_DEBUG(llvm::dbgs() << "\nBack propagation marks cold: " << *It);<br>
HotBlocks.erase(It);<br>
ColdBlocks.insert(It);<br>
<br>
@@ -353,6 +381,12 @@ const Function *HotColdSplitting::outlin<br>
// Walking the dominator tree allows us to find the largest<br>
// cold region.<br>
BasicBlock *Begin = DT->getRootNode()->getBlock();<br>
+<br>
+ // Early return if the beginning of the function has been marked cold,<br>
+ // otherwise all the function gets outlined.<br>
+ if (PSI->isColdBB(Begin, BFI) || !HotBlocks.count(Begin))<br>
+ return nullptr;<br>
+<br>
for (auto I = df_begin(Begin), E = df_end(Begin); I != E; ++I) {<br>
BasicBlock *BB = *I;<br>
if (PSI->isColdBB(BB, BFI) || !HotBlocks.count(BB)) {<br>
<br>
Modified: llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll?rev=344558&r1=344557&r2=344558&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll?rev=344558&r1=344557&r2=344558&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll (original)<br>
+++ llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll Mon Oct 15 14:43:11 2018<br>
@@ -1,9 +1,11 @@<br>
; RUN: opt -hotcoldsplit -S < %s | FileCheck %s<br>
; RUN: opt -passes=hotcoldsplit -S < %s | FileCheck %s<br>
<br>
-; Outlined function is called from a basic block named codeRepl<br>
-; CHECK: codeRepl:<br>
-; CHECK-NEXT: call void @foo<br>
+; Check that the function is not split. Outlined function is called from a<br>
+; basic block named codeRepl.<br>
+<br>
+; CHECK-LABEL: @foo<br>
+; CHECK-NOT: codeRepl<br>
define void @foo() {<br>
entry:<br>
br i1 undef, label %if.then, label %if.end<br>
@@ -23,3 +25,19 @@ cleanup40:<br>
return: ; preds = %cleanup40<br>
ret void<br>
}<br>
+<br>
+; Check that the function is not split. We used to outline the full function.<br>
+<br>
+; CHECK-LABEL: @fun<br>
+; CHECK-NOT: codeRepl<br>
+<br>
+define void @fun() {<br>
+entry:<br>
+ br i1 undef, label %if.then, label %if.end<br>
+<br>
+if.then: ; preds = %entry<br>
+ br label %if.end<br>
+<br>
+if.end: ; preds = %entry<br>
+ ret void<br>
+}<br>
<br>
Modified: llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll?rev=344558&r1=344557&r2=344558&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll?rev=344558&r1=344557&r2=344558&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll (original)<br>
+++ llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll Mon Oct 15 14:43:11 2018<br>
@@ -4,6 +4,10 @@<br>
; Make sure this compiles. This test used to fail with an invalid phi node: the<br>
; two predecessors were outlined and the SSA representation was invalid.<br>
<br>
+; CHECK-LABEL: @fun<br>
+; CHECK: codeRepl:<br>
+; CHECK-NEXT: call void @fun_if.else<br>
+<br>
define void @fun() {<br>
entry:<br>
br i1 undef, label %if.then, label %if.else<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>