[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:31:54 PDT 2018


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


More information about the llvm-commits mailing list