Re: [PATCH] tree-optimization/96579 - another special-operands fix in reassoc

2020-08-28 Thread Christophe Lyon via Gcc-patches
Hi,

On Thu, 27 Aug 2020 at 10:04, Richard Biener  wrote:
>
> This makes sure to put special-ops expanded rhs left where
> expression rewrite expects it.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>
> 2020-08-27  Richard Biener  
>
> PR tree-optimization/96579
> * tree-ssa-reassoc.c (linearize_expr_tree): If we expand
> rhs via special ops make sure to swap operands.
>
> * gcc.dg/pr96579.c: New testcase.
> ---
>  gcc/testsuite/gcc.dg/pr96579.c |  4 
>  gcc/tree-ssa-reassoc.c | 13 +++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr96579.c
>
> diff --git a/gcc/testsuite/gcc.dg/pr96579.c b/gcc/testsuite/gcc.dg/pr96579.c
> new file mode 100644
> index 000..49fdcb40359
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr96579.c
> @@ -0,0 +1,4 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fno-tree-forwprop -ffast-math -fno-tree-vrp" } */
> +
> +#include "pr96370.c"

pr96370 has
/* { dg-do compile { target dfp } } */
which is needed here too, otherwise the test fails on arm/aarch64 and others.

I've pushed the obvious fix.

Christophe


> diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
> index fed463b0350..a5f5d52edab 100644
> --- a/gcc/tree-ssa-reassoc.c
> +++ b/gcc/tree-ssa-reassoc.c
> @@ -5651,13 +5651,20 @@ linearize_expr_tree (vec *ops, 
> gimple *stmt,
>
>if (!binrhsisreassoc)
> {
> - if (!try_special_add_to_ops (ops, rhscode, binrhs, binrhsdef))
> + bool swap = false;
> + if (try_special_add_to_ops (ops, rhscode, binrhs, binrhsdef))
> +   /* If we add ops for the rhs we expect to be able to recurse
> +  to it via the lhs during expression rewrite so swap
> +  operands.  */
> +   swap = true;
> + else
> add_to_ops_vec (ops, binrhs);
>
>   if (!try_special_add_to_ops (ops, rhscode, binlhs, binlhsdef))
> add_to_ops_vec (ops, binlhs);
>
> - return;
> + if (!swap)
> +   return;
> }
>
>if (dump_file && (dump_flags & TDF_DETAILS))
> @@ -5676,6 +5683,8 @@ linearize_expr_tree (vec *ops, gimple 
> *stmt,
>   fprintf (dump_file, " is now ");
>   print_gimple_stmt (dump_file, stmt, 0);
> }
> +  if (!binrhsisreassoc)
> +   return;
>
>/* We want to make it so the lhs is always the reassociative op,
>  so swap.  */
> --
> 2.26.2


[PATCH] tree-optimization/96579 - another special-operands fix in reassoc

2020-08-27 Thread Richard Biener
This makes sure to put special-ops expanded rhs left where
expression rewrite expects it.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

2020-08-27  Richard Biener  

PR tree-optimization/96579
* tree-ssa-reassoc.c (linearize_expr_tree): If we expand
rhs via special ops make sure to swap operands.

* gcc.dg/pr96579.c: New testcase.
---
 gcc/testsuite/gcc.dg/pr96579.c |  4 
 gcc/tree-ssa-reassoc.c | 13 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr96579.c

diff --git a/gcc/testsuite/gcc.dg/pr96579.c b/gcc/testsuite/gcc.dg/pr96579.c
new file mode 100644
index 000..49fdcb40359
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr96579.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fno-tree-forwprop -ffast-math -fno-tree-vrp" } */
+
+#include "pr96370.c"
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index fed463b0350..a5f5d52edab 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -5651,13 +5651,20 @@ linearize_expr_tree (vec *ops, gimple 
*stmt,
 
   if (!binrhsisreassoc)
{
- if (!try_special_add_to_ops (ops, rhscode, binrhs, binrhsdef))
+ bool swap = false;
+ if (try_special_add_to_ops (ops, rhscode, binrhs, binrhsdef))
+   /* If we add ops for the rhs we expect to be able to recurse
+  to it via the lhs during expression rewrite so swap
+  operands.  */
+   swap = true;
+ else
add_to_ops_vec (ops, binrhs);
 
  if (!try_special_add_to_ops (ops, rhscode, binlhs, binlhsdef))
add_to_ops_vec (ops, binlhs);
 
- return;
+ if (!swap)
+   return;
}
 
   if (dump_file && (dump_flags & TDF_DETAILS))
@@ -5676,6 +5683,8 @@ linearize_expr_tree (vec *ops, gimple 
*stmt,
  fprintf (dump_file, " is now ");
  print_gimple_stmt (dump_file, stmt, 0);
}
+  if (!binrhsisreassoc)
+   return;
 
   /* We want to make it so the lhs is always the reassociative op,
 so swap.  */
-- 
2.26.2