[llvm-commits] [llvm] r162230 - in /llvm/trunk: lib/CodeGen/MachineVerifier.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp test/CodeGen/ARM/constants.ll

[Dmitri Gribenko]

On Tue, Aug 21, 2012 at 12:39 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> Author: stoklund
> Date: Mon Aug 20 16:39:52 2012
> New Revision: 162230
>
> URL: http://llvm.org/viewvc/llvm-project?rev=162230&view=rev
> Log:
> Don't add CFG edges for redundant conditional branches.
>
> IR that hasn't been through SimplifyCFG can look like this:
>
>   br i1 %b, label %r, label %r
>
> Make sure we don't create duplicate Machine CFG edges in this case.
>
> Fix the machine code verifier to accept conditional branches with a
> single CFG edge.
>
> Modified:
>     llvm/trunk/lib/CodeGen/MachineVerifier.cpp
>     llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>     llvm/trunk/test/CodeGen/ARM/constants.ll
>
> Modified: llvm/trunk/lib/CodeGen/MachineVerifier.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineVerifier.cpp?rev=162230&r1=162229&r2=162230&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineVerifier.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineVerifier.cpp Mon Aug 20 16:39:52 2012
> @@ -580,7 +580,15 @@
>        ++MBBI;
>        if (MBBI == MF->end()) {
>          report("MBB conditionally falls through out of function!", MBB);
> -      } if (MBB->succ_size() != 2) {
> +      } if (MBB->succ_size() == 1) {

Hello Jakob,

Sorry to comment on an old commit, but '} if (...' looks suspicious --
'else' is probably missing.  I have just fixed two bugs of this kind
in Clang, found by grep '} if'.

Dmitri

> +        // A conditional branch with only one successor is weird, but allowed.
> +        if (&*MBBI != TBB)
> +          report("MBB exits via conditional branch/fall-through but only has "
> +                 "one CFG successor!", MBB);
> +        else if (TBB != *MBB->succ_begin())
> +          report("MBB exits via conditional branch/fall-through but the CFG "
> +                 "successor don't match the actual successor!", MBB);
> +      } else if (MBB->succ_size() != 2) {
>          report("MBB exits via conditional branch/fall-through but doesn't have "
>                 "exactly two CFG successors!", MBB);
>        } else if (!matchPair(MBB->succ_begin(), TBB, MBBI)) {
> @@ -600,7 +608,15 @@
>      } else if (TBB && FBB) {
>        // Block conditionally branches somewhere, otherwise branches
>        // somewhere else.
> -      if (MBB->succ_size() != 2) {
> +      if (MBB->succ_size() == 1) {
> +        // A conditional branch with only one successor is weird, but allowed.
> +        if (FBB != TBB)
> +          report("MBB exits via conditional branch/branch through but only has "
> +                 "one CFG successor!", MBB);
> +        else if (TBB != *MBB->succ_begin())
> +          report("MBB exits via conditional branch/branch through but the CFG "
> +                 "successor don't match the actual successor!", MBB);
> +      } else if (MBB->succ_size() != 2) {
>          report("MBB exits via conditional branch/branch but doesn't have "
>                 "exactly two CFG successors!", MBB);
>        } else if (!matchPair(MBB->succ_begin(), TBB, FBB)) {
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=162230&r1=162229&r2=162230&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Mon Aug 20 16:39:52 2012
> @@ -1601,7 +1601,10 @@
>
>    // Update successor info
>    addSuccessorWithWeight(SwitchBB, CB.TrueBB, CB.TrueWeight);
> -  addSuccessorWithWeight(SwitchBB, CB.FalseBB, CB.FalseWeight);
> +  // TrueBB and FalseBB are always different unless the incoming IR is
> +  // degenerate. This only happens when running llc on weird IR.
> +  if (CB.TrueBB != CB.FalseBB)
> +    addSuccessorWithWeight(SwitchBB, CB.FalseBB, CB.FalseWeight);
>
>    // Set NextBlock to be the MBB immediately after the current one, if any.
>    // This is used to avoid emitting unnecessary branches to the next block.
>
> Modified: llvm/trunk/test/CodeGen/ARM/constants.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/constants.ll?rev=162230&r1=162229&r2=162230&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/constants.ll (original)
> +++ llvm/trunk/test/CodeGen/ARM/constants.ll Mon Aug 20 16:39:52 2012
> @@ -1,4 +1,4 @@
> -; RUN: llc < %s -mtriple=armv4t-unknown-linux-gnueabi -disable-cgp-branch-opts | FileCheck %s
> +; RUN: llc < %s -mtriple=armv4t-unknown-linux-gnueabi -disable-cgp-branch-opts -verify-machineinstrs | FileCheck %s
>
>  define i32 @f1() {
>  ; CHECK: f1
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/