Patchwork Improve ifcombine

login
register
mail settings
Submitter Jakub Jelinek
Date March 12, 2014, 9:52 a.m.
Message ID <20140312095223.GJ22862@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/329351/
State New
Headers show

Comments

Jakub Jelinek - March 12, 2014, 9:52 a.m.
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?

2014-03-12  Jakub Jelinek  <jakub@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.


	Jakub
Richard Guenther - March 12, 2014, 11:25 a.m.
On March 12, 2014 10:52:23 AM CET, Jakub Jelinek <jakub@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  <jakub@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

Patch

--- 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" } } */