This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix loop interchange ICE on bitfields (PR tree-optimization/83337)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>,Bin Cheng <bin dot cheng at arm dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Sun, 10 Dec 2017 08:24:30 +0100
- Subject: Re: [PATCH] Fix loop interchange ICE on bitfields (PR tree-optimization/83337)
- Authentication-results: sourceware.org; auth=none
- References: <20171209220818.GF2353@tucnak>
On December 9, 2017 11:08:18 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>compute_access_stride does:
> tree ref = DR_REF (dr);
> tree scev_base = build_fold_addr_expr (ref);
>but that really isn't valid for bitfield drs, where we can't take
>addresses
>of the bitfield.
>As I understood this function only wants to compute strides, so
>constant bit
>offsets shouldn't make a difference, therefore this patch attempts to
>deal
>with those the cheapest way by just using the struct address it is
>component
>of instead for the purposes of stride computation.
>If the bitfield is in a VLA structure, we can use the
>DECL_BIT_FIELD_REPRESENTATIVE, otherwise punt.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
>2017-12-09 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/83337
> * gimple-loop-interchange.cc (compute_access_stride): Handle bitfield
>DRs
> properly.
>
> * gcc.dg/tree-ssa/loop-interchange-14.c: New test.
> * gcc.dg/tree-ssa/loop-interchange-15.c: New test.
>
>--- gcc/gimple-loop-interchange.cc.jj 2017-12-08 12:27:11.000000000
>+0100
>+++ gcc/gimple-loop-interchange.cc 2017-12-09 14:11:17.271026901 +0100
>@@ -1291,6 +1291,30 @@ compute_access_stride (struct loop *loop
> gcc_assert (loop == bb->loop_father);
>
> tree ref = DR_REF (dr);
>+ if (TREE_CODE (ref) == COMPONENT_REF
>+ && DECL_BIT_FIELD (TREE_OPERAND (ref, 1)))
>+ {
>+ /* We can't take address of bitfields. If the bitfield is at
>constant
>+ offset from the start of the struct, just use address of the
>+ struct, for analysis of the strides that shouldn't matter. */
>+ if (!TREE_OPERAND (ref, 2)
>+ || TREE_CODE (TREE_OPERAND (ref, 2)) == INTEGER_CST)
>+ ref = TREE_OPERAND (ref, 0);
>+ /* Otherwise, if we have a bit field representative, use that.
>*/
>+ else if (DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (ref, 1))
>+ != NULL_TREE)
>+ {
>+ tree repr = DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (ref, 1));
>+ ref = build3 (COMPONENT_REF, TREE_TYPE (repr), TREE_OPERAND (ref,
>0),
>+ repr, TREE_OPERAND (ref, 2));
>+ }
>+ /* Otherwise punt. */
>+ else
>+ {
>+ dr->aux = strides;
>+ return;
>+ }
>+ }
> tree scev_base = build_fold_addr_expr (ref);
> tree scev = analyze_scalar_evolution (loop, scev_base);
> scev = instantiate_scev (loop_preheader_edge (loop_nest), loop, scev);
>--- gcc/testsuite/gcc.dg/tree-ssa/loop-interchange-14.c.jj 2017-12-09
>14:03:38.567556128 +0100
>+++ gcc/testsuite/gcc.dg/tree-ssa/loop-interchange-14.c 2017-12-09
>14:10:07.089889192 +0100
>@@ -0,0 +1,60 @@
>+/* PR tree-optimization/83337 */
>+/* { dg-do run { target int32plus } } */
>+/* { dg-options "-O2 -floop-interchange
>-fdump-tree-linterchange-details" } */
>+
>+/* Copied from graphite/interchange-5.c */
>+
>+#define DEBUG 0
>+#if DEBUG
>+#include <stdio.h>
>+#endif
>+
>+#define N 100
>+#define M 1111
>+struct S { int a : 3; int b : 17; int c : 12; };
>+struct S A[N][M];
>+
>+static int __attribute__((noinline))
>+foo (void)
>+{
>+ int i, j;
>+
>+ for( i = 0; i < M; i++)
>+ for( j = 0; j < N; j++)
>+ A[j][i].b = 5 * A[j][i].b;
>+
>+ return A[0][0].b + A[N-1][M-1].b;
>+}
>+
>+extern void abort ();
>+
>+static void __attribute__((noinline))
>+init (int i)
>+{
>+ int j;
>+
>+ for (j = 0; j < M; j++)
>+ A[i][j].b = 2;
>+}
>+
>+int
>+main (void)
>+{
>+ int i, j, res;
>+
>+ for (i = 0; i < N; i++)
>+ init (i);
>+
>+ res = foo ();
>+
>+#if DEBUG
>+ fprintf (stderr, "res = %d \n", res);
>+#endif
>+
>+ if (res != 20)
>+ abort ();
>+
>+ return 0;
>+}
>+
>+/* { dg-final { scan-tree-dump-times "Loop_pair<outer:., inner:.> is
>interchanged" 1 "linterchange"} } */
>--- gcc/testsuite/gcc.dg/tree-ssa/loop-interchange-15.c.jj 2017-12-09
>16:43:44.469944977 +0100
>+++ gcc/testsuite/gcc.dg/tree-ssa/loop-interchange-15.c 2017-12-09
>20:14:20.469419342 +0100
>@@ -0,0 +1,53 @@
>+/* PR tree-optimization/83337 */
>+/* { dg-do run { target int32plus } } */
>+/* { dg-options "-O2 -floop-interchange" } */
>+
>+/* Copied from graphite/interchange-5.c */
>+
>+#define DEBUG 0
>+#if DEBUG
>+#include <stdio.h>
>+#endif
>+
>+#define N 100
>+#define M 1111
>+
>+extern void abort ();
>+
>+static void __attribute__((noipa))
>+foo (int n)
>+{
>+ int i, j;
>+ struct S { char d[n]; int a : 3; int b : 17; int c : 12; };
>+ struct S A[N][M];
>+
>+ for (i = 0; i < N; i++)
>+ {
>+ asm volatile ("" : : "g" (&A[0][0]) : "memory");
>+ for (j = 0; j < M; j++)
>+ A[i][j].b = 2;
>+ }
>+ asm volatile ("" : : "g" (&A[0][0]) : "memory");
>+
>+ for (i = 0; i < M; i++)
>+ for (j = 0; j < N; j++)
>+ A[j][i].b = 5 * A[j][i].b;
>+
>+ asm volatile ("" : : "g" (&A[0][0]) : "memory");
>+ int res = A[0][0].b + A[N-1][M-1].b;
>+
>+#if DEBUG
>+ fprintf (stderr, "res = %d \n", res);
>+#endif
>+
>+ if (res != 20)
>+ abort ();
>+}
>+
>+int
>+main (void)
>+{
>+ foo (1);
>+ foo (8);
>+ return 0;
>+}
>
> Jakub