diff mbox

tree-scalar-evolution.c: Handle LSHIFT by constant

Message ID 1446545727-27064-1-git-send-email-alan.lawrence@arm.com
State New
Headers show

Commit Message

Alan Lawrence Nov. 3, 2015, 10:15 a.m. UTC
On 27/10/15 22:27, H.J. Lu wrote:
>
> It caused:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68112

Bah :(.

So yes, in general case, we can't rewrite (a << 1) to (a * 2) as for signed
types (0x7f...f) << 1 == -2 whereas (0x7f...f * 2) is undefined behaviour.
Oh well :(...

I don't have a really good fix for this. The best way I can see would be to try
to make definedness of overflow a property of either the type, or maybe of the
chrec, and settable on a finer granularity than at present, rather than
TYPE_OVERFLOW_UNDEFINED = (type is signed) && !(a bunch of global flags).
However, I don't think I'm going to have time for that patch before end of
stage 1.

So, I've reverted my r229437. There is a simpler fix: to only apply the rewrite
for unsigned types. I attach that patch, which I've bootstrapped on x86; but
although I think this way is correct, I'm not really sure whether this is
something that should go in. Thoughts?

--Alan
---
 gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c | 33 ++++++++++++++++++++++++
 gcc/tree-scalar-evolution.c                      | 19 ++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c

Comments

Richard Biener Nov. 3, 2015, 11:35 a.m. UTC | #1
On Tue, Nov 3, 2015 at 11:15 AM, Alan Lawrence <alan.lawrence@arm.com> wrote:
> On 27/10/15 22:27, H.J. Lu wrote:
>>
>> It caused:
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68112
>
> Bah :(.
>
> So yes, in general case, we can't rewrite (a << 1) to (a * 2) as for signed
> types (0x7f...f) << 1 == -2 whereas (0x7f...f * 2) is undefined behaviour.
> Oh well :(...
>
> I don't have a really good fix for this. The best way I can see would be to try
> to make definedness of overflow a property of either the type, or maybe of the
> chrec, and settable on a finer granularity than at present, rather than
> TYPE_OVERFLOW_UNDEFINED = (type is signed) && !(a bunch of global flags).
> However, I don't think I'm going to have time for that patch before end of
> stage 1.
>
> So, I've reverted my r229437. There is a simpler fix: to only apply the rewrite
> for unsigned types. I attach that patch, which I've bootstrapped on x86; but
> although I think this way is correct, I'm not really sure whether this is
> something that should go in. Thoughts?
>
> --Alan
> ---
>  gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c | 33 ++++++++++++++++++++++++
>  gcc/tree-scalar-evolution.c                      | 19 ++++++++++++++
>  2 files changed, 52 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c
>
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c b/gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c
> new file mode 100644
> index 0000000..40e6561
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c
> @@ -0,0 +1,33 @@
> +/* PR tree-optimization/65963.  */
> +#include "tree-vect.h"
> +
> +#define N 512
> +
> +int in[2*N], out[N];
> +
> +__attribute__ ((noinline)) void
> +loop (void)
> +{
> +  for (unsigned i = 0; i < N; i++)
> +    out[i] = in[i << 1] + 7;
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  check_vect ();
> +  for (int i = 0; i < 2*N; i++)
> +    {
> +      in[i] = i;
> +      __asm__ volatile ("" : : : "memory");
> +    }
> +  loop ();
> +  __asm__ volatile ("" : : : "memory");
> +  for (int i = 0; i < N; i++)
> +    {
> +      if (out[i] != i*2 + 7)
> +       abort ();
> +    }
> +  return 0;
> +}
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops in function" 1 "vect" { target { vect_strided2 } } } } */
> diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c
> index 0753bf3..d8f3d46 100644
> --- a/gcc/tree-scalar-evolution.c
> +++ b/gcc/tree-scalar-evolution.c
> @@ -1840,6 +1840,25 @@ interpret_rhs_expr (struct loop *loop, gimple *at_stmt,
>        res = chrec_fold_multiply (type, chrec1, chrec2);
>        break;
>
> +    case LSHIFT_EXPR:
> +      if (!TYPE_OVERFLOW_UNDEFINED (type))

I think this should simply re-write A << B to (type) (unsigned-type) A
* (1U << B).

Does that then still vectorize the signed case?

> +       {
> +         /* Handle A<<B as A * (1<<B).  */
> +         chrec1 = analyze_scalar_evolution (loop, rhs1);
> +         chrec2 = analyze_scalar_evolution (loop, rhs2);
> +         chrec1 = chrec_convert (type, chrec1, at_stmt);
> +         chrec1 = instantiate_parameters (loop, chrec1);
> +         chrec2 = instantiate_parameters (loop, chrec2);
> +
> +         chrec2 = fold_build2 (LSHIFT_EXPR, type,
> +                               build_int_cst (TREE_TYPE (rhs1), 1),
> +                               chrec2);
> +         res = chrec_fold_multiply (type, chrec1, chrec2);
> +       }
> +      else
> +       res = chrec_dont_know;
> +      break;
> +
>      CASE_CONVERT:
>        /* In case we have a truncation of a widened operation that in
>           the truncated type has undefined overflow behavior analyze
> --
> 1.9.1
>
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c b/gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c
new file mode 100644
index 0000000..40e6561
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-strided-shift-1.c
@@ -0,0 +1,33 @@ 
+/* PR tree-optimization/65963.  */
+#include "tree-vect.h"
+
+#define N 512
+
+int in[2*N], out[N];
+
+__attribute__ ((noinline)) void
+loop (void)
+{
+  for (unsigned i = 0; i < N; i++)
+    out[i] = in[i << 1] + 7;
+}
+
+int
+main (int argc, char **argv)
+{
+  check_vect ();
+  for (int i = 0; i < 2*N; i++)
+    {
+      in[i] = i;
+      __asm__ volatile ("" : : : "memory");
+    }
+  loop ();
+  __asm__ volatile ("" : : : "memory");
+  for (int i = 0; i < N; i++)
+    {
+      if (out[i] != i*2 + 7)
+	abort ();
+    }
+  return 0;
+}
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops in function" 1 "vect" { target { vect_strided2 } } } } */
diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c
index 0753bf3..d8f3d46 100644
--- a/gcc/tree-scalar-evolution.c
+++ b/gcc/tree-scalar-evolution.c
@@ -1840,6 +1840,25 @@  interpret_rhs_expr (struct loop *loop, gimple *at_stmt,
       res = chrec_fold_multiply (type, chrec1, chrec2);
       break;
 
+    case LSHIFT_EXPR:
+      if (!TYPE_OVERFLOW_UNDEFINED (type))
+	{
+	  /* Handle A<<B as A * (1<<B).  */
+	  chrec1 = analyze_scalar_evolution (loop, rhs1);
+	  chrec2 = analyze_scalar_evolution (loop, rhs2);
+	  chrec1 = chrec_convert (type, chrec1, at_stmt);
+	  chrec1 = instantiate_parameters (loop, chrec1);
+	  chrec2 = instantiate_parameters (loop, chrec2);
+
+	  chrec2 = fold_build2 (LSHIFT_EXPR, type,
+				build_int_cst (TREE_TYPE (rhs1), 1),
+				chrec2);
+	  res = chrec_fold_multiply (type, chrec1, chrec2);
+	}
+      else
+	res = chrec_dont_know;
+      break;
+
     CASE_CONVERT:
       /* In case we have a truncation of a widened operation that in
          the truncated type has undefined overflow behavior analyze