[llvm] r368997 - [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 16 06:20:31 PDT 2019


Hi,

I’ve reverted this change in r369104, as it seems to cause a verifier failure.

It looks like this commit adds invalid predecessors to MBBs. The example below fails the verifier after MachineBlockPlacement (run llc -verify-machineinstrs):

    @global.4 = external constant i8*

    declare i32 @zot(...)

    define i16* @snork.67() personality i8* bitcast (i32 (...)* @zot to i8*) {
    bb:
      invoke void undef()
              to label %bb5 unwind label %bb4

    bb4:                                              ; preds = %bb
      %tmp = landingpad { i8*, i32 }
              catch i8* null
      unreachable

    bb5:                                              ; preds = %bb
      %tmp6 = load i32, i32* null, align 4
      %tmp7 = icmp eq i32 %tmp6, 0
      br i1 %tmp7, label %bb14, label %bb8

    bb8:                                              ; preds = %bb11, %bb5
      invoke void undef()
              to label %bb9 unwind label %bb11

    bb9:                                              ; preds = %bb8
      %tmp10 = invoke i16* undef()
              to label %bb14 unwind label %bb11

    bb11:                                             ; preds = %bb9, %bb8
      %tmp12 = landingpad { i8*, i32 }
              cleanup
              catch i8* bitcast (i8** @global.4 to i8*)
      %tmp13 = icmp ult i64 undef, undef
      br i1 %tmp13, label %bb8, label %bb14

    bb14:                                             ; preds = %bb11, %bb9, %bb5
      %tmp15 = phi i16* [ null, %bb5 ], [ null, %bb11 ], [ %tmp10, %bb9 ]
      ret i16* %tmp15
    }


Cheers,
Florian


> On Aug 15, 2019, at 14:05, Kang Zhang via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: zhangkang
> Date: Thu Aug 15 06:05:16 2019
> New Revision: 368997
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=368997&view=rev
> Log:
> [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks
> 
> Summary:
> 
> This patch has trigger a bug of r368339, and the r368339 has been reverted, So upstream this patch again.
> 
> In `block-placement` pass, it will create some patterns for unconditional we can do the simple early retrun.
> But the `early-ret` pass is before `block-placement`, we don't want to run it again.
> This patch is to do the simple early return to optimize the blocks at the last of `block-placement`.
> 
> Reviewed By: efriedma
> 
> Differential Revision: https://reviews.llvm.org/D63972
> 
> Modified:
>    llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp
>    llvm/trunk/test/CodeGen/PowerPC/block-placement.mir
> 
> Modified: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp?rev=368997&r1=368996&r2=368997&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp Thu Aug 15 06:05:16 2019
> @@ -2711,6 +2711,7 @@ void MachineBlockPlacement::optimizeBran
>   // cannot because all branches may not be analyzable.
>   // E.g., the target may be able to remove an unconditional branch to
>   // a fallthrough when it occurs after predicated terminators.
> +  SmallVector<MachineBasicBlock*, 4> EmptyBB;
>   for (MachineBasicBlock *ChainBB : FunctionChain) {
>     Cond.clear();
>     MachineBasicBlock *TBB = nullptr, *FBB = nullptr; // For AnalyzeBranch.
> @@ -2730,9 +2731,42 @@ void MachineBlockPlacement::optimizeBran
>         TII->removeBranch(*ChainBB);
>         TII->insertBranch(*ChainBB, FBB, TBB, Cond, dl);
>         ChainBB->updateTerminator();
> +      } else if (Cond.empty() && TBB && ChainBB != TBB && !TBB->empty() &&
> +                 !TBB->canFallThrough()) {
> +        // When ChainBB is unconditional branch to the TBB, and TBB has no
> +        // fallthrough predecessor and fallthrough successor, try to merge
> +        // ChainBB and TBB. This is legal under the one of following conditions:
> +        // 1. ChainBB is empty except for an unconditional branch.
> +        // 2. TBB has only one predecessor.
> +        MachineFunction::iterator I(TBB);
> +        if (((TBB == &*F->begin()) || !std::prev(I)->canFallThrough()) &&
> +             (TailDup.isSimpleBB(ChainBB) || (TBB->pred_size() == 1))) {
> +          TII->removeBranch(*ChainBB);
> +          ChainBB->removeSuccessor(TBB);
> +
> +          // Update the CFG.
> +          while (!TBB->pred_empty()) {
> +            MachineBasicBlock *Pred = *(TBB->pred_end() - 1);
> +            Pred->ReplaceUsesOfBlockWith(TBB, ChainBB);
> +          }
> +
> +          for (MachineBasicBlock *Succ : TBB->successors())
> +            ChainBB->addSuccessor(Succ, MBPI->getEdgeProbability(TBB, Succ));
> +
> +          // Move all the instructions of TBB to ChainBB.
> +          ChainBB->splice(ChainBB->end(), TBB, TBB->begin(), TBB->end());
> +          EmptyBB.push_back(TBB);
> +        }
>       }
>     }
>   }
> +
> +  for (auto BB: EmptyBB) {
> +    MLI->removeBlock(BB);
> +    FunctionChain.remove(BB);
> +    BlockToChain.erase(BB);
> +    F->erase(BB);
> +  }
> }
> 
> void MachineBlockPlacement::alignBlocks() {
> @@ -3052,6 +3086,9 @@ bool MachineBlockPlacement::runOnMachine
>     }
>   }
> 
> +  // optimizeBranches() may change the blocks, but we haven't updated the
> +  // post-dominator tree. Because the post-dominator tree won't be used after
> +  // this function and this pass don't preserve the post-dominator tree.
>   optimizeBranches();
>   alignBlocks();
> 
> 
> Modified: llvm/trunk/test/CodeGen/PowerPC/block-placement.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/block-placement.mir?rev=368997&r1=368996&r2=368997&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/PowerPC/block-placement.mir (original)
> +++ llvm/trunk/test/CodeGen/PowerPC/block-placement.mir Thu Aug 15 06:05:16 2019
> @@ -209,14 +209,10 @@ body:             |
>     BLR8 implicit $lr8, implicit $rm, implicit killed $x3
> 
>   ; CHECK:      bb.5.if.else.i:
> -  ; CHECK:        successors: %bb.11(0x80000000)
> -  ; CHECK:        B %bb.11
> +  ; CHECK-NEXT:   renamable $x3 = LI8 1
> +  ; CHECK-NEXT:   BLR8 implicit $lr8, implicit $rm, implicit killed $x3
> 
>   ; CHECK:      bb.8.while.body.i (align 4):
> -  ; CHECK:        successors: %bb.11(0x04000000), %bb.9(0x7c000000)
> -  ; CHECK:        BCC 76, killed renamable $cr0, %bb.11
> -
> -  ; CHECK:      bb.11:
> -  ; CHECK:        renamable $x3 = LI8 1
> -  ; CHECK-NEXT:   BLR8 implicit $lr8, implicit $rm, implicit killed $x3
> +  ; CHECK:        successors: %bb.5(0x04000000), %bb.9(0x7c000000)
> +  ; CHECK:        BCC 76, killed renamable $cr0, %bb.5
> ...
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://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/20190816/2abb07fc/attachment.html>


More information about the llvm-commits mailing list