[llvm] r344558 - [hot-cold-split] fix static analysis of cold regions
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 15 15:59:13 PDT 2018
No worries. :)
-- Lang.
On Mon, Oct 15, 2018 at 3:35 PM Sebastian Pop <sebpop at gmail.com> wrote:
> Sorry for having broke the build.
> Lang, thanks for your quick fix.
>
>
> On Mon, Oct 15, 2018 at 5:32 PM Lang Hames <lhames at gmail.com> wrote:
>
>> Hi Sebastian,
>>
>> I have changed the TermianatorInst* to an Instruction* in r344566 to get
>> the tree building again.
>>
>> I think this is the right fix, but wanted to double check with you.
>>
>> Cheers,
>> Lang.
>>
>> On Mon, Oct 15, 2018 at 3:14 PM Lang Hames <lhames at gmail.com> wrote:
>>
>>> Looks like this might be incompatible with Chandler's change in r344504.
>>> On Top-of-tree I am seeing:
>>>
>>> /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 *'
>>> const TerminatorInst *I = BB.getTerminator();
>>>
>>> Cheers,
>>> Lang.
>>>
>>>
>>>
>>>
>>> On Mon, Oct 15, 2018 at 2:45 PM Sebastian Pop via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: spop
>>>> Date: Mon Oct 15 14:43:11 2018
>>>> New Revision: 344558
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=344558&view=rev
>>>> Log:
>>>> [hot-cold-split] fix static analysis of cold regions
>>>>
>>>> Make the code of blockEndsInUnreachable to match the function
>>>> blockEndsInUnreachable in CodeGen/BranchFolding.cpp. I also have
>>>> added a note to make sure the code of this function will not be
>>>> modified unless the back-end version is also modified.
>>>>
>>>> An early return before outlining has been added to avoid
>>>> outlining the full function body when the first block in the
>>>> function is marked cold.
>>>>
>>>> The static analysis of cold code has been amended to avoid
>>>> marking the whole function as cold by back-propagation
>>>> because the back-propagation would mark blocks with return
>>>> statements as cold.
>>>>
>>>> The patch adds debug statements to help discover these problems.
>>>>
>>>> Differential Revision: https://reviews.llvm.org/D52904
>>>>
>>>> Modified:
>>>> llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp
>>>> llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll
>>>> llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp?rev=344558&r1=344557&r2=344558&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/IPO/HotColdSplitting.cpp Mon Oct 15
>>>> 14:43:11 2018
>>>> @@ -101,14 +101,19 @@ static bool isSingleEntrySingleExit(Basi
>>>> return true;
>>>> }
>>>>
>>>> +// Same as blockEndsInUnreachable in CodeGen/BranchFolding.cpp. Do not
>>>> modify
>>>> +// this function unless you modify the MBB version as well.
>>>> +//
>>>> +/// A no successor, non-return block probably ends in unreachable and
>>>> is cold.
>>>> +/// Also consider a block that ends in an indirect branch to be a
>>>> return block,
>>>> +/// since many targets use plain indirect branches to return.
>>>> bool blockEndsInUnreachable(const BasicBlock &BB) {
>>>> + if (!succ_empty(&BB))
>>>> + return false;
>>>> if (BB.empty())
>>>> return true;
>>>> const Instruction *I = BB.getTerminator();
>>>> - if (isa<ReturnInst>(I) || isa<IndirectBrInst>(I))
>>>> - return true;
>>>> - // Unreachable blocks do not have any successor.
>>>> - return succ_empty(&BB);
>>>> + return !(isa<ReturnInst>(I) || isa<IndirectBrInst>(I));
>>>> }
>>>>
>>>> static bool exceptionHandlingFunctions(const CallInst *CI) {
>>>> @@ -123,8 +128,7 @@ static bool exceptionHandlingFunctions(c
>>>> FName == "__cxa_end_catch";
>>>> }
>>>>
>>>> -static
>>>> -bool unlikelyExecuted(const BasicBlock &BB) {
>>>> +static bool unlikelyExecuted(const BasicBlock &BB) {
>>>> if (blockEndsInUnreachable(BB))
>>>> return true;
>>>> // Exception handling blocks are unlikely executed.
>>>> @@ -145,13 +149,32 @@ bool unlikelyExecuted(const BasicBlock &
>>>> return false;
>>>> }
>>>>
>>>> +static bool returnsOrHasSideEffects(const BasicBlock &BB) {
>>>> + const TerminatorInst *I = BB.getTerminator();
>>>> + if (isa<ReturnInst>(I) || isa<IndirectBrInst>(I) ||
>>>> isa<InvokeInst>(I))
>>>> + return true;
>>>> +
>>>> + for (const Instruction &I : BB)
>>>> + if (const CallInst *CI = dyn_cast<CallInst>(&I)) {
>>>> + if (CI->hasFnAttr(Attribute::NoReturn))
>>>> + return true;
>>>> +
>>>> + if (isa<InlineAsm>(CI->getCalledValue()))
>>>> + return true;
>>>> + }
>>>> +
>>>> + return false;
>>>> +}
>>>> +
>>>> static DenseSetBB getHotBlocks(Function &F) {
>>>>
>>>> // Mark all cold basic blocks.
>>>> DenseSetBB ColdBlocks;
>>>> for (BasicBlock &BB : F)
>>>> - if (unlikelyExecuted(BB))
>>>> + if (unlikelyExecuted(BB)) {
>>>> + LLVM_DEBUG(llvm::dbgs() << "\nForward propagation marks cold: "
>>>> << BB);
>>>> ColdBlocks.insert((const BasicBlock *)&BB);
>>>> + }
>>>>
>>>> // Forward propagation: basic blocks are hot when they are reachable
>>>> from the
>>>> // beginning of the function through a path that does not contain
>>>> cold blocks.
>>>> @@ -203,7 +226,12 @@ static DenseSetBB getHotBlocks(Function
>>>> if (ColdBlocks.count(It))
>>>> continue;
>>>>
>>>> + // Do not back-propagate to blocks that return or have side
>>>> effects.
>>>> + if (returnsOrHasSideEffects(*It))
>>>> + continue;
>>>> +
>>>> // Move the block from HotBlocks to ColdBlocks.
>>>> + LLVM_DEBUG(llvm::dbgs() << "\nBack propagation marks cold: " <<
>>>> *It);
>>>> HotBlocks.erase(It);
>>>> ColdBlocks.insert(It);
>>>>
>>>> @@ -353,6 +381,12 @@ const Function *HotColdSplitting::outlin
>>>> // Walking the dominator tree allows us to find the largest
>>>> // cold region.
>>>> BasicBlock *Begin = DT->getRootNode()->getBlock();
>>>> +
>>>> + // Early return if the beginning of the function has been marked
>>>> cold,
>>>> + // otherwise all the function gets outlined.
>>>> + if (PSI->isColdBB(Begin, BFI) || !HotBlocks.count(Begin))
>>>> + return nullptr;
>>>> +
>>>> for (auto I = df_begin(Begin), E = df_end(Begin); I != E; ++I) {
>>>> BasicBlock *BB = *I;
>>>> if (PSI->isColdBB(BB, BFI) || !HotBlocks.count(BB)) {
>>>>
>>>> Modified: llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll?rev=344558&r1=344557&r2=344558&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll (original)
>>>> +++ llvm/trunk/test/Transforms/HotColdSplit/split-cold-1.ll Mon Oct 15
>>>> 14:43:11 2018
>>>> @@ -1,9 +1,11 @@
>>>> ; RUN: opt -hotcoldsplit -S < %s | FileCheck %s
>>>> ; RUN: opt -passes=hotcoldsplit -S < %s | FileCheck %s
>>>>
>>>> -; Outlined function is called from a basic block named codeRepl
>>>> -; CHECK: codeRepl:
>>>> -; CHECK-NEXT: call void @foo
>>>> +; Check that the function is not split. Outlined function is called
>>>> from a
>>>> +; basic block named codeRepl.
>>>> +
>>>> +; CHECK-LABEL: @foo
>>>> +; CHECK-NOT: codeRepl
>>>> define void @foo() {
>>>> entry:
>>>> br i1 undef, label %if.then, label %if.end
>>>> @@ -23,3 +25,19 @@ cleanup40:
>>>> return: ; preds = %cleanup40
>>>> ret void
>>>> }
>>>> +
>>>> +; Check that the function is not split. We used to outline the full
>>>> function.
>>>> +
>>>> +; CHECK-LABEL: @fun
>>>> +; CHECK-NOT: codeRepl
>>>> +
>>>> +define void @fun() {
>>>> +entry:
>>>> + br i1 undef, label %if.then, label %if.end
>>>> +
>>>> +if.then: ; preds = %entry
>>>> + br label %if.end
>>>> +
>>>> +if.end: ; preds = %entry
>>>> + ret void
>>>> +}
>>>>
>>>> Modified: llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll?rev=344558&r1=344557&r2=344558&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll (original)
>>>> +++ llvm/trunk/test/Transforms/HotColdSplit/split-cold-2.ll Mon Oct 15
>>>> 14:43:11 2018
>>>> @@ -4,6 +4,10 @@
>>>> ; 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.
>>>>
>>>> +; CHECK-LABEL: @fun
>>>> +; CHECK: codeRepl:
>>>> +; CHECK-NEXT: call void @fun_if.else
>>>> +
>>>> define void @fun() {
>>>> entry:
>>>> br i1 undef, label %if.then, label %if.else
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181015/713bef0e/attachment.html>
More information about the llvm-commits
mailing list