On March 12, 2014 10:52:23 AM CET, Jakub Jelinek <ja...@redhat.com> wrote:
>On Wed, Mar 12, 2014 at 09:51:46AM +0100, Richard Biener wrote:
>> Ok in principle, but is there a possibility to factor this a bit?
>> It looks like a lot of cut&paste (without looking too close for
>subtle
>> differences).
>
>Like this?
Yes.
Thanks,
Richard.
>2014-03-12 Jakub Jelinek <ja...@redhat.com>
>
> * tree-ssa-ifcombine.c (forwarder_block_to): New function.
> (tree_ssa_ifcombine_bb_1): New function.
> (tree_ssa_ifcombine_bb): Use it. Handle also cases where else_bb
> is an empty forwarder block to then_bb or vice versa and then_bb
> and else_bb are effectively swapped.
>
> * gcc.dg/tree-ssa/ssa-ifcombine-12.c: New test.
> * gcc.dg/tree-ssa/ssa-ifcombine-13.c: New test.
> * gcc.dg/tree-ssa/phi-opt-2.c: Pass -mbranch-cost=1 if
> possible, only test for exactly one if if -mbranch-cost=1
> has been passed.
>
>--- gcc/tree-ssa-ifcombine.c.jj 2014-03-11 20:14:34.046082392 +0100
>+++ gcc/tree-ssa-ifcombine.c 2014-03-12 10:45:29.355054715 +0100
>@@ -135,6 +135,16 @@ bb_no_side_effects_p (basic_block bb)
> return true;
> }
>
>+/* Return true if BB is an empty forwarder block to TO_BB. */
>+
>+static bool
>+forwarder_block_to (basic_block bb, basic_block to_bb)
>+{
>+ return empty_block_p (bb)
>+ && single_succ_p (bb)
>+ && single_succ (bb) == to_bb;
>+}
>+
> /* Verify if all PHI node arguments in DEST for edges from BB1 or
> BB2 to DEST are the same. This makes the CFG merge point
> free from side-effects. Return true in this case, else false. */
>@@ -561,6 +571,99 @@ ifcombine_ifandif (basic_block inner_con
> return false;
> }
>
>+/* Helper function for tree_ssa_ifcombine_bb. Recognize a CFG pattern
>and
>+ dispatch to the appropriate if-conversion helper for a particular
>+ set of INNER_COND_BB, OUTER_COND_BB, THEN_BB and ELSE_BB.
>+ PHI_PRED_BB should be one of INNER_COND_BB, THEN_BB or ELSE_BB. */
>+
>+static bool
>+tree_ssa_ifcombine_bb_1 (basic_block inner_cond_bb, basic_block
>outer_cond_bb,
>+ basic_block then_bb, basic_block else_bb,
>+ basic_block phi_pred_bb)
>+{
>+ /* The && form is characterized by a common else_bb with
>+ the two edges leading to it mergable. The latter is
>+ guaranteed by matching PHI arguments in the else_bb and
>+ the inner cond_bb having no side-effects. */
>+ if (phi_pred_bb != else_bb
>+ && recognize_if_then_else (outer_cond_bb, &inner_cond_bb,
>&else_bb)
>+ && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb)
>+ && bb_no_side_effects_p (inner_cond_bb))
>+ {
>+ /* We have
>+ <outer_cond_bb>
>+ if (q) goto inner_cond_bb; else goto else_bb;
>+ <inner_cond_bb>
>+ if (p) goto ...; else goto else_bb;
>+ ...
>+ <else_bb>
>+ ...
>+ */
>+ return ifcombine_ifandif (inner_cond_bb, false, outer_cond_bb,
>false,
>+ false);
>+ }
>+
>+ /* And a version where the outer condition is negated. */
>+ if (phi_pred_bb != else_bb
>+ && recognize_if_then_else (outer_cond_bb, &else_bb,
>&inner_cond_bb)
>+ && same_phi_args_p (outer_cond_bb, phi_pred_bb, else_bb)
>+ && bb_no_side_effects_p (inner_cond_bb))
>+ {
>+ /* We have
>+ <outer_cond_bb>
>+ if (q) goto else_bb; else goto inner_cond_bb;
>+ <inner_cond_bb>
>+ if (p) goto ...; else goto else_bb;
>+ ...
>+ <else_bb>
>+ ...
>+ */
>+ return ifcombine_ifandif (inner_cond_bb, false, outer_cond_bb,
>true,
>+ false);
>+ }
>+
>+ /* The || form is characterized by a common then_bb with the
>+ two edges leading to it mergable. The latter is guaranteed
>+ by matching PHI arguments in the then_bb and the inner cond_bb
>+ having no side-effects. */
>+ if (phi_pred_bb != then_bb
>+ && recognize_if_then_else (outer_cond_bb, &then_bb,
>&inner_cond_bb)
>+ && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb)
>+ && bb_no_side_effects_p (inner_cond_bb))
>+ {
>+ /* We have
>+ <outer_cond_bb>
>+ if (q) goto then_bb; else goto inner_cond_bb;
>+ <inner_cond_bb>
>+ if (q) goto then_bb; else goto ...;
>+ <then_bb>
>+ ...
>+ */
>+ return ifcombine_ifandif (inner_cond_bb, true, outer_cond_bb,
>true,
>+ true);
>+ }
>+
>+ /* And a version where the outer condition is negated. */
>+ if (phi_pred_bb != then_bb
>+ && recognize_if_then_else (outer_cond_bb, &inner_cond_bb,
>&then_bb)
>+ && same_phi_args_p (outer_cond_bb, phi_pred_bb, then_bb)
>+ && bb_no_side_effects_p (inner_cond_bb))
>+ {
>+ /* We have
>+ <outer_cond_bb>
>+ if (q) goto inner_cond_bb; else goto then_bb;
>+ <inner_cond_bb>
>+ if (q) goto then_bb; else goto ...;
>+ <then_bb>
>+ ...
>+ */
>+ return ifcombine_ifandif (inner_cond_bb, true, outer_cond_bb,
>false,
>+ true);
>+ }
>+
>+ return false;
>+}
>+
> /* Recognize a CFG pattern and dispatch to the appropriate
> if-conversion helper. We start with BB as the innermost
> worker basic-block. Returns true if a transformation was done. */
>@@ -585,80 +688,33 @@ tree_ssa_ifcombine_bb (basic_block inner
> {
> basic_block outer_cond_bb = single_pred (inner_cond_bb);
>
>- /* The && form is characterized by a common else_bb with
>- the two edges leading to it mergable. The latter is
>- guaranteed by matching PHI arguments in the else_bb and
>- the inner cond_bb having no side-effects. */
>- if (recognize_if_then_else (outer_cond_bb, &inner_cond_bb,
>&else_bb)
>- && same_phi_args_p (outer_cond_bb, inner_cond_bb, else_bb)
>- && bb_no_side_effects_p (inner_cond_bb))
>- {
>- /* We have
>- <outer_cond_bb>
>- if (q) goto inner_cond_bb; else goto else_bb;
>- <inner_cond_bb>
>- if (p) goto ...; else goto else_bb;
>- ...
>- <else_bb>
>- ...
>- */
>- return ifcombine_ifandif (inner_cond_bb, false, outer_cond_bb,
>false,
>- false);
>- }
>-
>- /* And a version where the outer condition is negated. */
>- if (recognize_if_then_else (outer_cond_bb, &else_bb,
>&inner_cond_bb)
>- && same_phi_args_p (outer_cond_bb, inner_cond_bb, else_bb)
>- && bb_no_side_effects_p (inner_cond_bb))
>- {
>- /* We have
>- <outer_cond_bb>
>- if (q) goto else_bb; else goto inner_cond_bb;
>- <inner_cond_bb>
>- if (p) goto ...; else goto else_bb;
>- ...
>- <else_bb>
>- ...
>- */
>- return ifcombine_ifandif (inner_cond_bb, false, outer_cond_bb,
>true,
>- false);
>- }
>-
>- /* The || form is characterized by a common then_bb with the
>- two edges leading to it mergable. The latter is guaranteed
>- by matching PHI arguments in the then_bb and the inner
>cond_bb
>- having no side-effects. */
>- if (recognize_if_then_else (outer_cond_bb, &then_bb,
>&inner_cond_bb)
>- && same_phi_args_p (outer_cond_bb, inner_cond_bb, then_bb)
>- && bb_no_side_effects_p (inner_cond_bb))
>- {
>- /* We have
>- <outer_cond_bb>
>- if (q) goto then_bb; else goto inner_cond_bb;
>- <inner_cond_bb>
>- if (q) goto then_bb; else goto ...;
>- <then_bb>
>- ...
>- */
>- return ifcombine_ifandif (inner_cond_bb, true, outer_cond_bb, true,
>- true);
>- }
>-
>- /* And a version where the outer condition is negated. */
>- if (recognize_if_then_else (outer_cond_bb, &inner_cond_bb,
>&then_bb)
>- && same_phi_args_p (outer_cond_bb, inner_cond_bb, then_bb)
>- && bb_no_side_effects_p (inner_cond_bb))
>- {
>- /* We have
>- <outer_cond_bb>
>- if (q) goto inner_cond_bb; else goto then_bb;
>- <inner_cond_bb>
>- if (q) goto then_bb; else goto ...;
>- <then_bb>
>- ...
>- */
>- return ifcombine_ifandif (inner_cond_bb, true, outer_cond_bb,
>false,
>- true);
>+ if (tree_ssa_ifcombine_bb_1 (inner_cond_bb, outer_cond_bb,
>+ then_bb, else_bb, inner_cond_bb))
>+ return true;
>+
>+ if (forwarder_block_to (else_bb, then_bb))
>+ {
>+ /* Other possibilities for the && form, if else_bb is
>+ empty forwarder block to then_bb. Compared to the above simpler
>+ forms this can be treated as if then_bb and else_bb were
>swapped,
>+ and the corresponding inner_cond_bb not inverted because of
>that.
>+ For same_phi_args_p we look at equality of arguments between
>+ edge from outer_cond_bb and the forwarder block. */
>+ if (tree_ssa_ifcombine_bb_1 (inner_cond_bb, outer_cond_bb, else_bb,
>+ then_bb, else_bb))
>+ return true;
>+ }
>+ else if (forwarder_block_to (then_bb, else_bb))
>+ {
>+ /* Other possibilities for the || form, if then_bb is
>+ empty forwarder block to else_bb. Compared to the above simpler
>+ forms this can be treated as if then_bb and else_bb were
>swapped,
>+ and the corresponding inner_cond_bb not inverted because of
>that.
>+ For same_phi_args_p we look at equality of arguments between
>+ edge from outer_cond_bb and the forwarder block. */
>+ if (tree_ssa_ifcombine_bb_1 (inner_cond_bb, outer_cond_bb, else_bb,
>+ then_bb, then_bb))
>+ return true;
> }
> }
>
>--- gcc/testsuite/gcc.dg/tree-ssa/ssa-ifcombine-12.c.jj 2014-03-12
>10:23:33.231679541 +0100
>+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-ifcombine-12.c 2014-03-12
>10:23:33.231679541 +0100
>@@ -0,0 +1,20 @@
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -fno-tree-vrp -fdump-tree-optimized" } */
>+
>+/* Testcase for PR31657. */
>+
>+int f(int x, int a, int b)
>+{
>+ int t = 0;
>+ int c = 1 << a;
>+ if (!(x & 1))
>+ t = 0;
>+ else
>+ if (x & (1 << 2))
>+ t = 3;
>+ else
>+ t = 0;
>+ return t;
>+}
>+/* { dg-final { scan-tree-dump "& 5" "optimized" } } */
>+/* { dg-final { cleanup-tree-dump "optimized" } } */
>--- gcc/testsuite/gcc.dg/tree-ssa/ssa-ifcombine-13.c.jj 2014-03-12
>10:23:33.231679541 +0100
>+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-ifcombine-13.c 2014-03-12
>10:23:33.231679541 +0100
>@@ -0,0 +1,21 @@
>+/* { dg-do compile } */
>+/* { dg-options "-O1 -fdump-tree-optimized" } */
>+/* { dg-additional-options "-mbranch-cost=2" { target { i?86-*-*
>x86_64-*-* mips*-*-* s390*-*-* avr*-*-* } } } */
>+
>+_Bool f1(_Bool a, _Bool b)
>+{
>+ if (a)
>+ {
>+ if (b)
>+ return 1;
>+ else
>+ return 0;
>+ }
>+ return 0;
>+}
>+
>+
>+/* For LOGICAL_OP_NON_SHORT_CIRCUIT, this should be optimized
>+ into return a & b;, with no ifs. */
>+/* { dg-final { scan-tree-dump-not "if" "optimized" { target {
>i?86-*-* x86_64-*-* mips*-*-* s390*-*-* avr*-*-* } } } } */
>+/* { dg-final { cleanup-tree-dump "optimized" } } */
>--- gcc/testsuite/gcc.dg/tree-ssa/phi-opt-2.c.jj 2014-03-11
>20:14:33.812083721 +0100
>+++ gcc/testsuite/gcc.dg/tree-ssa/phi-opt-2.c 2014-03-12
>10:23:33.231679541 +0100
>@@ -1,5 +1,6 @@
> /* { dg-do compile } */
> /* { dg-options "-O1 -fdump-tree-optimized" } */
>+/* { dg-additional-options "-mbranch-cost=1" { target { i?86-*-*
>x86_64-*-* mips*-*-* s390*-*-* avr*-*-* } } } */
>
> _Bool f1(_Bool a, _Bool b)
> {
>@@ -17,6 +18,8 @@ _Bool f1(_Bool a, _Bool b)
> /* There should be only one if, the outer one; the inner one
> should have been changed to straight line code with the
> value of b (except that we don't fold ! (b != 0) into b
>- which can be fixed in a different patch). */
>-/* { dg-final { scan-tree-dump-times "if" 1 "optimized"} } */
>+ which can be fixed in a different patch).
>+ Test this only when known to be !LOGICAL_OP_NON_SHORT_CIRCUIT,
>+ otherwise ifcombine may convert this into return a & b;. */
>+/* { dg-final { scan-tree-dump-times "if" 1 "optimized" { target {
>i?86-*-* x86_64-*-* mips*-*-* s390*-*-* avr*-*-* } } } } */
> /* { dg-final { cleanup-tree-dump "optimized" } } */
>
> Jakub
