diff mbox

Fix PR tree-optimization/71077

Message ID 20160530195024.24106-1-patrick@parcs.ath.cx
State New
Headers show

Commit Message

Patrick Palka May 30, 2016, 7:50 p.m. UTC 
In this PR the function simplify_control_stmt_condition_1(), which is
responsible for recursing into the operands of a GIMPLE_COND during jump
threading to check for dominating ASSERT_EXPRs, was erroneously
returning a VECTOR_CST when it should instead be returning an
INTEGER_CST.  The problem is that the zero/one constants that this
function uses share the type of the GIMPLE_COND's operands, operands
which may be vectors.  This makes no sense and there is no reason to
build and use such constants instead of using boolean_[true|false]_node
since all that matters is whether a constant returned by
simplify_control_stmt_condition() correctly satisfies
integer_[zero|one]p.  So this patch makes the combining part of
simplify_control_stmt_condition_1() just use boolean_false_node or
boolean_true_node as the designated false/true values.

Does this look OK to commit after bootstrap+regtest on
x86_64-pc-linux-gnu?  (I'm not sure if I put the test in the proper
directory since no other test in tree-ssa uses -flto.  Also not sure if
an i686 target accepts the -march=core-avx2 flag.)

gcc/ChangeLog:

	PR tree-optimization/71077
	* tree-ssa-threadedge.c (simplify_control_stmt_condition_1): In
	the combining step, use boolean_false_node and boolean_true_node
	as the designated false/true return values.

gcc/testsuite/ChangeLog:

	PR tree-optimization/71077
	* gcc.dg/tree-ssa/pr71077.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/pr71077.c | 18 ++++++++++++++++++
 gcc/tree-ssa-threadedge.c               | 26 ++++++++++++--------------
 2 files changed, 30 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71077.c

Comments

Richard Biener May 31, 2016, 9:20 a.m. UTC | #1 
On Mon, May 30, 2016 at 9:50 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> In this PR the function simplify_control_stmt_condition_1(), which is
> responsible for recursing into the operands of a GIMPLE_COND during jump
> threading to check for dominating ASSERT_EXPRs, was erroneously
> returning a VECTOR_CST when it should instead be returning an
> INTEGER_CST.  The problem is that the zero/one constants that this
> function uses share the type of the GIMPLE_COND's operands, operands
> which may be vectors.  This makes no sense and there is no reason to
> build and use such constants instead of using boolean_[true|false]_node
> since all that matters is whether a constant returned by
> simplify_control_stmt_condition() correctly satisfies
> integer_[zero|one]p.  So this patch makes the combining part of
> simplify_control_stmt_condition_1() just use boolean_false_node or
> boolean_true_node as the designated false/true values.
>
> Does this look OK to commit after bootstrap+regtest on
> x86_64-pc-linux-gnu?  (I'm not sure if I put the test in the proper
> directory since no other test in tree-ssa uses -flto.  Also not sure if
> an i686 target accepts the -march=core-avx2 flag.)

Yes.

Ok.
Thanks,
Richard.

> gcc/ChangeLog:
>
>         PR tree-optimization/71077
>         * tree-ssa-threadedge.c (simplify_control_stmt_condition_1): In
>         the combining step, use boolean_false_node and boolean_true_node
>         as the designated false/true return values.
>
> gcc/testsuite/ChangeLog:
>
>         PR tree-optimization/71077
>         * gcc.dg/tree-ssa/pr71077.c: New test.
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/pr71077.c | 18 ++++++++++++++++++
>  gcc/tree-ssa-threadedge.c               | 26 ++++++++++++--------------
>  2 files changed, 30 insertions(+), 14 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71077.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c
> new file mode 100644
> index 0000000..4753740
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c
> @@ -0,0 +1,18 @@
> +/* PR c++/71077  */
> +/* { dg-do link { target { i?86-*-* x86_64-*-* } } }  */
> +/* { dg-options "-O3 -flto -march=core-avx2" }  */
> +
> +int *a;
> +int b, c, d, e;
> +int sched_analyze(void) {
> + for (; b; b++) {
> +   c = 0;
> +   for (; c < 32; c++)
> +     if (b & 1 << c)
> +       a[b + c] = d;
> + }
> + return 0;
> +}
> +
> +void schedule_insns(void) { e = sched_analyze(); }
> +int main(void) { schedule_insns(); }
> diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
> index eca3812..826d620 100644
> --- a/gcc/tree-ssa-threadedge.c
> +++ b/gcc/tree-ssa-threadedge.c
> @@ -573,8 +573,6 @@ simplify_control_stmt_condition_1 (edge e,
>           enum tree_code rhs_code = gimple_assign_rhs_code (def_stmt);
>           const tree rhs1 = gimple_assign_rhs1 (def_stmt);
>           const tree rhs2 = gimple_assign_rhs2 (def_stmt);
> -         const tree zero_cst = build_zero_cst (TREE_TYPE (op0));
> -         const tree one_cst = build_one_cst (TREE_TYPE (op0));
>
>           /* Is A != 0 ?  */
>           const tree res1
> @@ -589,19 +587,19 @@ simplify_control_stmt_condition_1 (edge e,
>             {
>               /* If A == 0 then (A & B) != 0 is always false.  */
>               if (cond_code == NE_EXPR)
> -               return zero_cst;
> +               return boolean_false_node;
>               /* If A == 0 then (A & B) == 0 is always true.  */
>               if (cond_code == EQ_EXPR)
> -               return one_cst;
> +               return boolean_true_node;
>             }
>           else if (rhs_code == BIT_IOR_EXPR && integer_nonzerop (res1))
>             {
>               /* If A != 0 then (A | B) != 0 is always true.  */
>               if (cond_code == NE_EXPR)
> -               return one_cst;
> +               return boolean_true_node;
>               /* If A != 0 then (A | B) == 0 is always false.  */
>               if (cond_code == EQ_EXPR)
> -               return zero_cst;
> +               return boolean_false_node;
>             }
>
>           /* Is B != 0 ?  */
> @@ -617,19 +615,19 @@ simplify_control_stmt_condition_1 (edge e,
>             {
>               /* If B == 0 then (A & B) != 0 is always false.  */
>               if (cond_code == NE_EXPR)
> -               return zero_cst;
> +               return boolean_false_node;
>               /* If B == 0 then (A & B) == 0 is always true.  */
>               if (cond_code == EQ_EXPR)
> -               return one_cst;
> +               return boolean_true_node;
>             }
>           else if (rhs_code == BIT_IOR_EXPR && integer_nonzerop (res2))
>             {
>               /* If B != 0 then (A | B) != 0 is always true.  */
>               if (cond_code == NE_EXPR)
> -               return one_cst;
> +               return boolean_true_node;
>               /* If B != 0 then (A | B) == 0 is always false.  */
>               if (cond_code == EQ_EXPR)
> -               return zero_cst;
> +               return boolean_false_node;
>             }
>
>           if (res1 != NULL_TREE && res2 != NULL_TREE)
> @@ -641,10 +639,10 @@ simplify_control_stmt_condition_1 (edge e,
>                 {
>                   /* If A != 0 and B != 0 then (bool)(A & B) != 0 is true.  */
>                   if (cond_code == NE_EXPR)
> -                   return one_cst;
> +                   return boolean_true_node;
>                   /* If A != 0 and B != 0 then (bool)(A & B) == 0 is false.  */
>                   if (cond_code == EQ_EXPR)
> -                   return zero_cst;
> +                   return boolean_false_node;
>                 }
>
>               if (rhs_code == BIT_IOR_EXPR
> @@ -653,10 +651,10 @@ simplify_control_stmt_condition_1 (edge e,
>                 {
>                   /* If A == 0 and B == 0 then (A | B) != 0 is false.  */
>                   if (cond_code == NE_EXPR)
> -                   return zero_cst;
> +                   return boolean_false_node;
>                   /* If A == 0 and B == 0 then (A | B) == 0 is true.  */
>                   if (cond_code == EQ_EXPR)
> -                   return one_cst;
> +                   return boolean_true_node;
>                 }
>             }
>         }
> --
> 2.9.0.rc0.29.gabd6606
>
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c
new file mode 100644
index 0000000..4753740
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c
@@ -0,0 +1,18 @@ 
+/* PR c++/71077  */
+/* { dg-do link { target { i?86-*-* x86_64-*-* } } }  */
+/* { dg-options "-O3 -flto -march=core-avx2" }  */
+
+int *a;
+int b, c, d, e;
+int sched_analyze(void) {
+ for (; b; b++) {
+   c = 0;
+   for (; c < 32; c++)
+     if (b & 1 << c)
+       a[b + c] = d;
+ }
+ return 0;
+}
+
+void schedule_insns(void) { e = sched_analyze(); }
+int main(void) { schedule_insns(); }
diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
index eca3812..826d620 100644
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@ -573,8 +573,6 @@  simplify_control_stmt_condition_1 (edge e,
 	  enum tree_code rhs_code = gimple_assign_rhs_code (def_stmt);
 	  const tree rhs1 = gimple_assign_rhs1 (def_stmt);
 	  const tree rhs2 = gimple_assign_rhs2 (def_stmt);
-	  const tree zero_cst = build_zero_cst (TREE_TYPE (op0));
-	  const tree one_cst = build_one_cst (TREE_TYPE (op0));
 
 	  /* Is A != 0 ?  */
 	  const tree res1
@@ -589,19 +587,19 @@  simplify_control_stmt_condition_1 (edge e,
 	    {
 	      /* If A == 0 then (A & B) != 0 is always false.  */
 	      if (cond_code == NE_EXPR)
-	        return zero_cst;
+	        return boolean_false_node;
 	      /* If A == 0 then (A & B) == 0 is always true.  */
 	      if (cond_code == EQ_EXPR)
-		return one_cst;
+		return boolean_true_node;
 	    }
 	  else if (rhs_code == BIT_IOR_EXPR && integer_nonzerop (res1))
 	    {
 	      /* If A != 0 then (A | B) != 0 is always true.  */
 	      if (cond_code == NE_EXPR)
-		return one_cst;
+		return boolean_true_node;
 	      /* If A != 0 then (A | B) == 0 is always false.  */
 	      if (cond_code == EQ_EXPR)
-		return zero_cst;
+		return boolean_false_node;
 	    }
 
 	  /* Is B != 0 ?  */
@@ -617,19 +615,19 @@  simplify_control_stmt_condition_1 (edge e,
 	    {
 	      /* If B == 0 then (A & B) != 0 is always false.  */
 	      if (cond_code == NE_EXPR)
-	        return zero_cst;
+	        return boolean_false_node;
 	      /* If B == 0 then (A & B) == 0 is always true.  */
 	      if (cond_code == EQ_EXPR)
-		return one_cst;
+		return boolean_true_node;
 	    }
 	  else if (rhs_code == BIT_IOR_EXPR && integer_nonzerop (res2))
 	    {
 	      /* If B != 0 then (A | B) != 0 is always true.  */
 	      if (cond_code == NE_EXPR)
-		return one_cst;
+		return boolean_true_node;
 	      /* If B != 0 then (A | B) == 0 is always false.  */
 	      if (cond_code == EQ_EXPR)
-		return zero_cst;
+		return boolean_false_node;
 	    }
 
 	  if (res1 != NULL_TREE && res2 != NULL_TREE)
@@ -641,10 +639,10 @@  simplify_control_stmt_condition_1 (edge e,
 		{
 		  /* If A != 0 and B != 0 then (bool)(A & B) != 0 is true.  */
 		  if (cond_code == NE_EXPR)
-		    return one_cst;
+		    return boolean_true_node;
 		  /* If A != 0 and B != 0 then (bool)(A & B) == 0 is false.  */
 		  if (cond_code == EQ_EXPR)
-		    return zero_cst;
+		    return boolean_false_node;
 		}
 
 	      if (rhs_code == BIT_IOR_EXPR
@@ -653,10 +651,10 @@  simplify_control_stmt_condition_1 (edge e,
 		{
 		  /* If A == 0 and B == 0 then (A | B) != 0 is false.  */
 		  if (cond_code == NE_EXPR)
-		    return zero_cst;
+		    return boolean_false_node;
 		  /* If A == 0 and B == 0 then (A | B) == 0 is true.  */
 		  if (cond_code == EQ_EXPR)
-		    return one_cst;
+		    return boolean_true_node;
 		}
 	    }
 	}