[PATCH] Optionally disable the BadCFGConflict check in MachineBlockPlacement.cpp

[Chandler Carruth]

Thanks for this!

I've committed this with an added flag and more tests.

On Wed, Jan 14, 2015 at 8:36 AM, Daniel Jasper <djasper at google.com> wrote:

> Hi chandlerc,
>
> Some benchmarks have shown that this could lead to a potential performance
> benefit.
>
> A possible explanation. In diamond-shaped CFGs (A followed by either B or
> C both followed by D), putting B and C both in between A and D leads to the
> code being less dense than it could be. Always either B or C have to be
> skipped increasing the chance of cache misses etc. Moving either B or C to
> after D might be beneficial on average.
>
> In the long run, but we should probably do a better job of analyzing the
> basic block and branch probabilities to move the correct one of B or C to
> after D. But even if we don't use this in the long run, it is a good
> baseline for benchmarking.
>
> http://reviews.llvm.org/D6969
>
> Files:
>   lib/CodeGen/MachineBlockPlacement.cpp
>   test/CodeGen/X86/code_placement_bad_cfg_check.ll
>
> Index: lib/CodeGen/MachineBlockPlacement.cpp
> ===================================================================
> --- lib/CodeGen/MachineBlockPlacement.cpp
> +++ lib/CodeGen/MachineBlockPlacement.cpp
> @@ -60,6 +60,12 @@
>                                                  "blocks in the
> function."),
>                                         cl::init(0), cl::Hidden);
>
> +static cl::opt<bool> NoBadCFGConflictCheck(
> +    "no-bad-cfg-conflict-check",
> +    cl::desc("Don't check whether a hot successor has a more important "
> +             "predecessor."),
> +    cl::init(false), cl::Hidden);
> +
>  // FIXME: Find a good default for this flag and remove the flag.
>  static cl::opt<unsigned>
>  ExitBlockBias("block-placement-exit-block-bias",
> @@ -374,29 +380,31 @@
>          continue;
>        }
>
> -      // Make sure that a hot successor doesn't have a globally more
> important
> -      // predecessor.
> -      BlockFrequency CandidateEdgeFreq
> -        = MBFI->getBlockFreq(BB) * SuccProb * HotProb.getCompl();
> -      bool BadCFGConflict = false;
> -      for (MachineBasicBlock::pred_iterator PI = (*SI)->pred_begin(),
> -                                            PE = (*SI)->pred_end();
> -           PI != PE; ++PI) {
> -        if (*PI == *SI || (BlockFilter && !BlockFilter->count(*PI)) ||
> -            BlockToChain[*PI] == &Chain)
> +      if (!NoBadCFGConflictCheck) {
> +        // Make sure that a hot successor doesn't have a globally more
> important
> +        // predecessor.
> +        BlockFrequency CandidateEdgeFreq =
> +            MBFI->getBlockFreq(BB) * SuccProb * HotProb.getCompl();
> +        bool BadCFGConflict = false;
> +        for (MachineBasicBlock::pred_iterator PI = (*SI)->pred_begin(),
> +                                              PE = (*SI)->pred_end();
> +             PI != PE; ++PI) {
> +          if (*PI == *SI || (BlockFilter && !BlockFilter->count(*PI)) ||
> +              BlockToChain[*PI] == &Chain)
> +            continue;
> +          BlockFrequency PredEdgeFreq =
> +              MBFI->getBlockFreq(*PI) * MBPI->getEdgeProbability(*PI,
> *SI);
> +          if (PredEdgeFreq >= CandidateEdgeFreq) {
> +            BadCFGConflict = true;
> +            break;
> +          }
> +        }
> +        if (BadCFGConflict) {
> +          DEBUG(dbgs() << "    " << getBlockName(*SI) << " -> " <<
> SuccProb
> +                       << " (prob) (non-cold CFG conflict)\n");
>            continue;
> -        BlockFrequency PredEdgeFreq
> -          = MBFI->getBlockFreq(*PI) * MBPI->getEdgeProbability(*PI, *SI);
> -        if (PredEdgeFreq >= CandidateEdgeFreq) {
> -          BadCFGConflict = true;
> -          break;
>          }
>        }
> -      if (BadCFGConflict) {
> -        DEBUG(dbgs() << "    " << getBlockName(*SI) << " -> " << SuccProb
> -                     << " (prob) (non-cold CFG conflict)\n");
> -        continue;
> -      }
>      }
>
>      DEBUG(dbgs() << "    " << getBlockName(*SI) << " -> " << SuccProb
> Index: test/CodeGen/X86/code_placement_bad_cfg_check.ll
> ===================================================================
> --- /dev/null
> +++ test/CodeGen/X86/code_placement_bad_cfg_check.ll
> @@ -0,0 +1,25 @@
> +; RUN: llc  -mcpu=corei7 -mtriple=x86_64-linux -no-bad-cfg-conflict-check
> < %s | FileCheck %s
> +
> +;CHECK: callq h
> +;CHECK: ret
> +;CHECK: callq
> +define void @foo(i32 %t) nounwind readnone ssp uwtable {
> +  %cmp = icmp eq i32 %t, 0
> +  br i1 %cmp, label %if.then, label %if.else
> +
> +if.then:
> +  call void @f()
> +  br label %if.end
> +
> +if.else:
> +  call void @g()
> +  br label %if.end
> +
> +if.end:
> +  call void @h()
> +  ret void
> +}
> +
> +declare void @f()
> +declare void @g()
> +declare void @h()
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150114/6df10cf8/attachment.html>