[PATCH] tree-ssa-mathopts: Use proper poly_int64 comparison with param_avoid_fma_max_bits [PR 98766]

[PATCH] tree-ssa-mathopts: Use proper poly_int64 comparison with param_avoid_fma_max_bits [PR 98766]

[Kyrylo Tkachov]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

Hi all,

We ICE here because we end up comparing a poly_int64 with a scalar using <= rather than known_le.
This patch fixes that in the way richi suggests in the PR.

Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk?

Thanks,
Kyrill

gcc/ChangeLog:

	PR tree-optimization/98766
	* tree-ssa-math-opts.c (convert_mult_to_fma): Use known_le when
	comparing against type size with param_avoid_fma_max_bits.

gcc/testsuite/ChangeLog:

	PR tree-optimization/98766
	* gcc.dg/pr98766.c: New test.

[-- Attachment #2: math-opts.patch --]
[-- Type: application/octet-stream, Size: 1212 bytes --]

diff --git a/gcc/testsuite/gcc.dg/pr98766.c b/gcc/testsuite/gcc.dg/pr98766.c
new file mode 100644
index 0000000000000000000000000000000000000000..d388fd2aa87f90b03872a232f9334c7012136d13
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr98766.c
@@ -0,0 +1,12 @@
+/* PR tree-optimization/98766.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 --param=avoid-fma-max-bits=8 " } */
+/* { dg-additional-options "-march=armv8.2-a+sve" { target aarch64*-*-* } } */
+
+extern int a[];
+void c(short *d) {
+  for (int e = 0; e < 9; e++)
+    a[e] = d[e] * 2;
+}
+
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index d6201d3cb943e145720c18fbf3aadd853fd87b44..800815b855c759075b4326361cc4db7183f1c543 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -3252,8 +3252,8 @@ convert_mult_to_fma (gimple *mul_stmt, tree op1, tree op2,
 
   bool check_defer
     = (state->m_deferring_p
-       && (tree_to_shwi (TYPE_SIZE (type))
-	   <= param_avoid_fma_max_bits));
+       && known_le (tree_to_poly_int64 (TYPE_SIZE (type)),
+		    param_avoid_fma_max_bits));
   bool defer = check_defer;
   bool seen_negate_p = false;
   /* Make sure that the multiplication statement becomes dead after

Re: [PATCH] tree-ssa-mathopts: Use proper poly_int64 comparison with param_avoid_fma_max_bits [PR 98766]

[Richard Sandiford]

Kyrylo Tkachov via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
> index d6201d3cb943e145720c18fbf3aadd853fd87b44..800815b855c759075b4326361cc4db7183f1c543 100644
> --- a/gcc/tree-ssa-math-opts.c
> +++ b/gcc/tree-ssa-math-opts.c
> @@ -3252,8 +3252,8 @@ convert_mult_to_fma (gimple *mul_stmt, tree op1, tree op2,
>  
>    bool check_defer
>      = (state->m_deferring_p
> -       && (tree_to_shwi (TYPE_SIZE (type))
> -	   <= param_avoid_fma_max_bits));
> +       && known_le (tree_to_poly_int64 (TYPE_SIZE (type)),
> +		    param_avoid_fma_max_bits));

I'd suggest maybe_le here instead.  I.e. instead of the current
“don't do this if the vector length is X” have “don't do this if
the vector length might be X”.

OK from my point of view with that change.

Thanks,
Richard

>    bool defer = check_defer;
>    bool seen_negate_p = false;
>    /* Make sure that the multiplication statement becomes dead after

RE: [PATCH] tree-ssa-mathopts: Use proper poly_int64 comparison with param_avoid_fma_max_bits [PR 98766]

[Kyrylo Tkachov]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

Hi Richard,

> -----Original Message-----
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: 22 January 2021 13:09
> To: Kyrylo Tkachov via Gcc-patches <gcc-patches@gcc.gnu.org>
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Subject: Re: [PATCH] tree-ssa-mathopts: Use proper poly_int64 comparison
> with param_avoid_fma_max_bits [PR 98766]
> 
> Kyrylo Tkachov via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
> > index
> d6201d3cb943e145720c18fbf3aadd853fd87b44..800815b855c759075b4326
> 361cc4db7183f1c543 100644
> > --- a/gcc/tree-ssa-math-opts.c
> > +++ b/gcc/tree-ssa-math-opts.c
> > @@ -3252,8 +3252,8 @@ convert_mult_to_fma (gimple *mul_stmt, tree
> op1, tree op2,
> >
> >    bool check_defer
> >      = (state->m_deferring_p
> > -       && (tree_to_shwi (TYPE_SIZE (type))
> > -	   <= param_avoid_fma_max_bits));
> > +       && known_le (tree_to_poly_int64 (TYPE_SIZE (type)),
> > +		    param_avoid_fma_max_bits));
> 
> I'd suggest maybe_le here instead.  I.e. instead of the current
> “don't do this if the vector length is X” have “don't do this if
> the vector length might be X”.
> 
> OK from my point of view with that change.

Thanks.
Pushed with that change after a bootstrap and test on aarch64-none-linux-gnu.
Kyrill

> 
> Thanks,
> Richard
> 
> >    bool defer = check_defer;
> >    bool seen_negate_p = false;
> >    /* Make sure that the multiplication statement becomes dead after

[-- Attachment #2: math-opts.patch --]
[-- Type: application/octet-stream, Size: 1212 bytes --]

diff --git a/gcc/testsuite/gcc.dg/pr98766.c b/gcc/testsuite/gcc.dg/pr98766.c
new file mode 100644
index 0000000000000000000000000000000000000000..d388fd2aa87f90b03872a232f9334c7012136d13
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr98766.c
@@ -0,0 +1,12 @@
+/* PR tree-optimization/98766.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 --param=avoid-fma-max-bits=8 " } */
+/* { dg-additional-options "-march=armv8.2-a+sve" { target aarch64*-*-* } } */
+
+extern int a[];
+void c(short *d) {
+  for (int e = 0; e < 9; e++)
+    a[e] = d[e] * 2;
+}
+
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index d6201d3cb943e145720c18fbf3aadd853fd87b44..c4a6492b50df25b4cf296a75bd51e5af34eeacc7 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -3252,8 +3252,8 @@ convert_mult_to_fma (gimple *mul_stmt, tree op1, tree op2,
 
   bool check_defer
     = (state->m_deferring_p
-       && (tree_to_shwi (TYPE_SIZE (type))
-	   <= param_avoid_fma_max_bits));
+       && maybe_le (tree_to_poly_int64 (TYPE_SIZE (type)),
+		    param_avoid_fma_max_bits));
   bool defer = check_defer;
   bool seen_negate_p = false;
   /* Make sure that the multiplication statement becomes dead after