Don't install unnecessary ARRAY_REF element sizes

Message ID mpt7e3er8lj.fsf@arm.com
State New
Headers show
Series
  • Don't install unnecessary ARRAY_REF element sizes
Related show

Commit Message

Richard Sandiford Dec. 2, 2019, 4:32 p.m.
Even EXACT_DIV_EXPR doesn't distribute across addition for wrapping
types, so in general we can't fold EXACT_DIV_EXPRs of POLY_INT_CSTs
at compile time.  This was causing an ICE when trying to gimplify the
element size field in an ARRAY_REF.

If the result of that EXACT_DIV_EXPR is an invariant, we don't bother
recording it in the ARRAY_REF and simply read the element size from the
element type.  This avoids the overhead of doing:

      /* ??? tree_ssa_useless_type_conversion will eliminate casts to
	 sizetype from another type of the same width and signedness.  */
      if (TREE_TYPE (aligned_size) != sizetype)
	aligned_size = fold_convert_loc (loc, sizetype, aligned_size);
      return size_binop_loc (loc, MULT_EXPR, aligned_size,
			     size_int (TYPE_ALIGN_UNIT (elmt_type)));

each time array_ref_element_size is called.

So rather than read array_ref_element_size, do some arithmetic on it,
and only then check whether the result is an invariant, we might as
well check whether the element size is an invariant to start with.
We're then directly testing whether array_ref_element_size gives
a reusable value.

For consistency, the patch makes the same change for the offset field
in a COMPONENT_REF, although I don't think that can trigger yet.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2019-12-02  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* gimplify.c (gimplify_compound_lval): Don't gimplify and install
	an array element size if array_element_size is already an invariant.
	Similarly don't gimplify and install a field offset if
	component_ref_field_offset is already an invariant.

gcc/testsuite/
	* gcc.target/aarch64/sve/acle/general-c/struct_1.c: New test.

Comments

Richard Biener Dec. 3, 2019, 2:54 p.m. | #1
On Mon, Dec 2, 2019 at 5:33 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>

> Even EXACT_DIV_EXPR doesn't distribute across addition for wrapping

> types, so in general we can't fold EXACT_DIV_EXPRs of POLY_INT_CSTs

> at compile time.  This was causing an ICE when trying to gimplify the

> element size field in an ARRAY_REF.

>

> If the result of that EXACT_DIV_EXPR is an invariant, we don't bother

> recording it in the ARRAY_REF and simply read the element size from the

> element type.  This avoids the overhead of doing:

>

>       /* ??? tree_ssa_useless_type_conversion will eliminate casts to

>          sizetype from another type of the same width and signedness.  */

>       if (TREE_TYPE (aligned_size) != sizetype)

>         aligned_size = fold_convert_loc (loc, sizetype, aligned_size);

>       return size_binop_loc (loc, MULT_EXPR, aligned_size,

>                              size_int (TYPE_ALIGN_UNIT (elmt_type)));

>

> each time array_ref_element_size is called.

>

> So rather than read array_ref_element_size, do some arithmetic on it,

> and only then check whether the result is an invariant, we might as

> well check whether the element size is an invariant to start with.

> We're then directly testing whether array_ref_element_size gives

> a reusable value.

>

> For consistency, the patch makes the same change for the offset field

> in a COMPONENT_REF, although I don't think that can trigger yet.

>

> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?


OK.

Thanks,
Richard.

> Richard

>

>

> 2019-12-02  Richard Sandiford  <richard.sandiford@arm.com>

>

> gcc/

>         * gimplify.c (gimplify_compound_lval): Don't gimplify and install

>         an array element size if array_element_size is already an invariant.

>         Similarly don't gimplify and install a field offset if

>         component_ref_field_offset is already an invariant.

>

> gcc/testsuite/

>         * gcc.target/aarch64/sve/acle/general-c/struct_1.c: New test.

>

> Index: gcc/gimplify.c

> ===================================================================

> --- gcc/gimplify.c      2019-11-22 09:57:52.178273436 +0000

> +++ gcc/gimplify.c      2019-12-02 16:32:08.063499557 +0000

> @@ -2987,17 +2987,18 @@ gimplify_compound_lval (tree *expr_p, gi

>

>           if (TREE_OPERAND (t, 3) == NULL_TREE)

>             {

> -             tree elmt_type = TREE_TYPE (TREE_TYPE (TREE_OPERAND (t, 0)));

> -             tree elmt_size = unshare_expr (array_ref_element_size (t));

> -             tree factor = size_int (TYPE_ALIGN_UNIT (elmt_type));

> -

> -             /* Divide the element size by the alignment of the element

> -                type (above).  */

> -             elmt_size

> -               = size_binop_loc (loc, EXACT_DIV_EXPR, elmt_size, factor);

> -

> +             tree elmt_size = array_ref_element_size (t);

>               if (!is_gimple_min_invariant (elmt_size))

>                 {

> +                 elmt_size = unshare_expr (elmt_size);

> +                 tree elmt_type = TREE_TYPE (TREE_TYPE (TREE_OPERAND (t, 0)));

> +                 tree factor = size_int (TYPE_ALIGN_UNIT (elmt_type));

> +

> +                 /* Divide the element size by the alignment of the element

> +                    type (above).  */

> +                 elmt_size = size_binop_loc (loc, EXACT_DIV_EXPR,

> +                                             elmt_size, factor);

> +

>                   TREE_OPERAND (t, 3) = elmt_size;

>                   tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p,

>                                         post_p, is_gimple_reg,

> @@ -3017,16 +3018,18 @@ gimplify_compound_lval (tree *expr_p, gi

>           /* Set the field offset into T and gimplify it.  */

>           if (TREE_OPERAND (t, 2) == NULL_TREE)

>             {

> -             tree offset = unshare_expr (component_ref_field_offset (t));

> -             tree field = TREE_OPERAND (t, 1);

> -             tree factor

> -               = size_int (DECL_OFFSET_ALIGN (field) / BITS_PER_UNIT);

> -

> -             /* Divide the offset by its alignment.  */

> -             offset = size_binop_loc (loc, EXACT_DIV_EXPR, offset, factor);

> -

> +             tree offset = component_ref_field_offset (t);

>               if (!is_gimple_min_invariant (offset))

>                 {

> +                 offset = unshare_expr (offset);

> +                 tree field = TREE_OPERAND (t, 1);

> +                 tree factor

> +                   = size_int (DECL_OFFSET_ALIGN (field) / BITS_PER_UNIT);

> +

> +                 /* Divide the offset by its alignment.  */

> +                 offset = size_binop_loc (loc, EXACT_DIV_EXPR,

> +                                          offset, factor);

> +

>                   TREE_OPERAND (t, 2) = offset;

>                   tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p,

>                                         post_p, is_gimple_reg,

> Index: gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/struct_1.c

> ===================================================================

> --- /dev/null   2019-09-17 11:41:18.176664108 +0100

> +++ gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/struct_1.c      2019-12-02 16:32:08.075499476 +0000

> @@ -0,0 +1,10 @@

> +/* { dg-options "-std=c99" } */

> +

> +#include <arm_sve.h>

> +

> +void

> +f (svint8_t a, svint8_t b)

> +{

> +  /* Not supported, but mustn't ICE.  */

> +  (svint8x2_t) { a, b };

> +}

Patch

Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	2019-11-22 09:57:52.178273436 +0000
+++ gcc/gimplify.c	2019-12-02 16:32:08.063499557 +0000
@@ -2987,17 +2987,18 @@  gimplify_compound_lval (tree *expr_p, gi
 
 	  if (TREE_OPERAND (t, 3) == NULL_TREE)
 	    {
-	      tree elmt_type = TREE_TYPE (TREE_TYPE (TREE_OPERAND (t, 0)));
-	      tree elmt_size = unshare_expr (array_ref_element_size (t));
-	      tree factor = size_int (TYPE_ALIGN_UNIT (elmt_type));
-
-	      /* Divide the element size by the alignment of the element
-		 type (above).  */
-	      elmt_size
-		= size_binop_loc (loc, EXACT_DIV_EXPR, elmt_size, factor);
-
+	      tree elmt_size = array_ref_element_size (t);
 	      if (!is_gimple_min_invariant (elmt_size))
 		{
+		  elmt_size = unshare_expr (elmt_size);
+		  tree elmt_type = TREE_TYPE (TREE_TYPE (TREE_OPERAND (t, 0)));
+		  tree factor = size_int (TYPE_ALIGN_UNIT (elmt_type));
+
+		  /* Divide the element size by the alignment of the element
+		     type (above).  */
+		  elmt_size = size_binop_loc (loc, EXACT_DIV_EXPR,
+					      elmt_size, factor);
+
 		  TREE_OPERAND (t, 3) = elmt_size;
 		  tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p,
 					post_p, is_gimple_reg,
@@ -3017,16 +3018,18 @@  gimplify_compound_lval (tree *expr_p, gi
 	  /* Set the field offset into T and gimplify it.  */
 	  if (TREE_OPERAND (t, 2) == NULL_TREE)
 	    {
-	      tree offset = unshare_expr (component_ref_field_offset (t));
-	      tree field = TREE_OPERAND (t, 1);
-	      tree factor
-		= size_int (DECL_OFFSET_ALIGN (field) / BITS_PER_UNIT);
-
-	      /* Divide the offset by its alignment.  */
-	      offset = size_binop_loc (loc, EXACT_DIV_EXPR, offset, factor);
-
+	      tree offset = component_ref_field_offset (t);
 	      if (!is_gimple_min_invariant (offset))
 		{
+		  offset = unshare_expr (offset);
+		  tree field = TREE_OPERAND (t, 1);
+		  tree factor
+		    = size_int (DECL_OFFSET_ALIGN (field) / BITS_PER_UNIT);
+
+		  /* Divide the offset by its alignment.  */
+		  offset = size_binop_loc (loc, EXACT_DIV_EXPR,
+					   offset, factor);
+
 		  TREE_OPERAND (t, 2) = offset;
 		  tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p,
 					post_p, is_gimple_reg,
Index: gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/struct_1.c
===================================================================
--- /dev/null	2019-09-17 11:41:18.176664108 +0100
+++ gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/struct_1.c	2019-12-02 16:32:08.075499476 +0000
@@ -0,0 +1,10 @@ 
+/* { dg-options "-std=c99" } */
+
+#include <arm_sve.h>
+
+void
+f (svint8_t a, svint8_t b)
+{
+  /* Not supported, but mustn't ICE.  */
+  (svint8x2_t) { a, b };
+}