[llvm] r344558 - [hot-cold-split] fix static analysis of cold regions

Sebastian Pop via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 15 15:34:40 PDT 2018


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/e297edac/attachment.html>


More information about the llvm-commits mailing list