On Thu, 17 Sep 2015, Alan Lawrence wrote:
> On 15/09/15 08:43, Richard Biener wrote:
> >
> > Sorry for chiming in so late...
>
> Not at all, TYVM for your help!
>
> > TREE_CONSTANT isn't the correct thing to test. You should use
> > TREE_CODE () == INTEGER_CST instead.
>
> Done (in some cases, via tree_fits_shwi_p).
>
> > Also you need to handle
> > NULL_TREE TYPE_MIN/MAX_VALUE as that can happen as well.
>
> I've not found any documentation as to what these mean, but from experiment,
> it seems that a zero-length array has MIN_VALUE 0 and MAX_VALUE null (as well
> as zero TYPE_SIZE) - so I allow that. In contrast a variable-length array also
> has zero TYPE_SIZE, but a large range of MIN-MAX, and I think I want to rule
> those out.
>
> Another awkward case is Ada arrays with large offset (e.g.
> array(2^32...2^32+1)
> which has only two elements); I don't see either of tree_to_shwi or
> tree_to_uhwi
> as necessarily being "better" here, each will handle (some) (rare) cases the
> other will not, so I've tried to use tree_to_shwi throughout for consistency.
>
> Summary: taken advantage of tree_fits_shwi_p, as this includes a check against
> NULL_TREE and that TREE_CODE () == INTEGER_CST.
>
> > + if (DECL_P (elem) && DECL_BIT_FIELD (elem))
> > + return false;
> >
> > that can't happen (TREE_TYPE (array-type) is never a DECL).
>
> Removed.
>
> > + int el_size = tree_to_uhwi (elem_size);
> > + gcc_assert (el_size);
> >
> > so you assert on el_size being > 0 later but above you test
> > only array size ...
>
> Good point, thanks.
>
> > + tree t_idx = build_int_cst (TYPE_DOMAIN (decl_type), idx);
> >
> > use t_idx = size_int (idx);
>
> Done.
>
> I've also added another test, of scalarizing a structure containing a
> zero-length array, as earlier attempts accidentally prevented this.
>
> Bootstrapped + check-gcc,g++,ada,fortran on ARM and x86_64;
> Bootstrapped + check-gcc,g++,fortran on AArch64.
>
> OK for trunk?
Ok.
Thanks,
Richard.
> Thanks,
> Alan
>
> gcc/ChangeLog:
>
> PR tree-optimization/67283
> * tree-sra.c (type_consists_of_records_p): Rename to...
> (scalarizable_type_p): ...this, add case for ARRAY_TYPE.
> (completely_scalarize_record): Rename to...
> (completely_scalarize): ...this, add ARRAY_TYPE case, move some code to:
> (scalarize_elem): New.
> (analyze_all_variable_accesses): Follow renamings.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/67283
> * gcc.dg/tree-ssa/sra-15.c: New.
> * gcc.dg/tree-ssa/sra-16.c: New.
> ---
> gcc/testsuite/gcc.dg/tree-ssa/sra-15.c | 37 ++++++++
> gcc/testsuite/gcc.dg/tree-ssa/sra-16.c | 37 ++++++++
> gcc/tree-sra.c | 165
> +++++++++++++++++++++++----------
> 3 files changed, 191 insertions(+), 48 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-15.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-16.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c
> b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c
> new file mode 100644
> index 0000000..a22062e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-15.c
> @@ -0,0 +1,37 @@
> +/* Verify that SRA total scalarization works on records containing arrays.
> */
> +/* { dg-do run } */
> +/* { dg-options "-O1 -fdump-tree-release_ssa --param
> sra-max-scalarization-size-Ospeed=32" } */
> +
> +extern void abort (void);
> +
> +struct S
> +{
> + char c;
> + unsigned short f[2][2];
> + int i;
> + unsigned short f3, f4;
> +};
> +
> +
> +int __attribute__ ((noinline))
> +foo (struct S *p)
> +{
> + struct S l;
> +
> + l = *p;
> + l.i++;
> + l.f[1][0] += 3;
> + *p = l;
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> + struct S a = {0, { {5, 7}, {9, 11} }, 4, 0, 0};
> + foo (&a);
> + if (a.i != 5 || a.f[1][0] != 12)
> + abort ();
> + return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "l;" 0 "release_ssa" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/sra-16.c
> b/gcc/testsuite/gcc.dg/tree-ssa/sra-16.c
> new file mode 100644
> index 0000000..fef34c0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/sra-16.c
> @@ -0,0 +1,37 @@
> +/* Verify that SRA total scalarization works on records containing arrays.
> */
> +/* { dg-do run } */
> +/* { dg-options "-O1 -fdump-tree-release_ssa --param
> sra-max-scalarization-size-Ospeed=16" } */
> +
> +extern void abort (void);
> +
> +struct S
> +{
> + long zilch[0];
> + char c;
> + int i;
> + unsigned short f3, f4;
> +};
> +
> +
> +int __attribute__ ((noinline))
> +foo (struct S *p)
> +{
> + struct S l;
> +
> + l = *p;
> + l.i++;
> + l.f3++;
> + *p = l;
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> + struct S a = { { }, 0, 4, 0, 0};
> + foo (&a);
> + if (a.i != 5 || a.f3 != 1)
> + abort ();
> + return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "l;" 0 "release_ssa" } } */
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index 8b3a0ad..8247554 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -915,73 +915,142 @@ create_access (tree expr, gimple stmt, bool write)
> }
>
>
> -/* Return true iff TYPE is a RECORD_TYPE with fields that are either of
> gimple
> - register types or (recursively) records with only these two kinds of
> fields.
> - It also returns false if any of these records contains a bit-field. */
> +/* Return true iff TYPE is scalarizable - i.e. a RECORD_TYPE or fixed-length
> + ARRAY_TYPE with fields that are either of gimple register types (excluding
> + bit-fields) or (recursively) scalarizable types. */
>
> static bool
> -type_consists_of_records_p (tree type)
> +scalarizable_type_p (tree type)
> {
> - tree fld;
> + gcc_assert (!is_gimple_reg_type (type));
>
> - if (TREE_CODE (type) != RECORD_TYPE)
> - return false;
> + switch (TREE_CODE (type))
> + {
> + case RECORD_TYPE:
> + for (tree fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
> + if (TREE_CODE (fld) == FIELD_DECL)
> + {
> + tree ft = TREE_TYPE (fld);
>
> - for (fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
> - if (TREE_CODE (fld) == FIELD_DECL)
> - {
> - tree ft = TREE_TYPE (fld);
> + if (DECL_BIT_FIELD (fld))
> + return false;
>
> - if (DECL_BIT_FIELD (fld))
> - return false;
> + if (!is_gimple_reg_type (ft)
> + && !scalarizable_type_p (ft))
> + return false;
> + }
>
> - if (!is_gimple_reg_type (ft)
> - && !type_consists_of_records_p (ft))
> - return false;
> - }
> + return true;
>
> - return true;
> + case ARRAY_TYPE:
> + {
> + if (TYPE_DOMAIN (type) == NULL_TREE
> + || !tree_fits_shwi_p (TYPE_SIZE (type))
> + || !tree_fits_shwi_p (TYPE_SIZE (TREE_TYPE (type)))
> + || (tree_to_shwi (TYPE_SIZE (TREE_TYPE (type))) <= 0)
> + || !tree_fits_shwi_p (TYPE_MIN_VALUE (TYPE_DOMAIN (type))))
> + return false;
> + if (tree_to_shwi (TYPE_SIZE (type)) == 0
> + && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE)
> + /* Zero-element array, should not prevent scalarization. */
> + ;
> + else if ((tree_to_shwi (TYPE_SIZE (type)) <= 0)
> + || !tree_fits_shwi_p (TYPE_MAX_VALUE (TYPE_DOMAIN (type))))
> + return false;
> +
> + tree elem = TREE_TYPE (type);
> + if (!is_gimple_reg_type (elem)
> + && !scalarizable_type_p (elem))
> + return false;
> + return true;
> + }
> + default:
> + return false;
> + }
> }
>
> -/* Create total_scalarization accesses for all scalar type fields in DECL
> that
> - must be of a RECORD_TYPE conforming to type_consists_of_records_p. BASE
> - must be the top-most VAR_DECL representing the variable, OFFSET must be
> the
> - offset of DECL within BASE. REF must be the memory reference expression
> for
> - the given decl. */
> +static void scalarize_elem (tree, HOST_WIDE_INT, HOST_WIDE_INT, tree, tree);
> +
> +/* Create total_scalarization accesses for all scalar fields of a member
> + of type DECL_TYPE conforming to scalarizable_type_p. BASE
> + must be the top-most VAR_DECL representing the variable; within that,
> + OFFSET locates the member and REF must be the memory reference expression
> for
> + the member. */
>
> static void
> -completely_scalarize_record (tree base, tree decl, HOST_WIDE_INT offset,
> - tree ref)
> +completely_scalarize (tree base, tree decl_type, HOST_WIDE_INT offset, tree
> ref)
> {
> - tree fld, decl_type = TREE_TYPE (decl);
> + switch (TREE_CODE (decl_type))
> + {
> + case RECORD_TYPE:
> + for (tree fld = TYPE_FIELDS (decl_type); fld; fld = DECL_CHAIN (fld))
> + if (TREE_CODE (fld) == FIELD_DECL)
> + {
> + HOST_WIDE_INT pos = offset + int_bit_position (fld);
> + tree ft = TREE_TYPE (fld);
> + tree nref = build3 (COMPONENT_REF, ft, ref, fld, NULL_TREE);
>
> - for (fld = TYPE_FIELDS (decl_type); fld; fld = DECL_CHAIN (fld))
> - if (TREE_CODE (fld) == FIELD_DECL)
> + scalarize_elem (base, pos, tree_to_uhwi (DECL_SIZE (fld)), nref,
> + ft);
> + }
> + break;
> + case ARRAY_TYPE:
> {
> - HOST_WIDE_INT pos = offset + int_bit_position (fld);
> - tree ft = TREE_TYPE (fld);
> - tree nref = build3 (COMPONENT_REF, TREE_TYPE (fld), ref, fld,
> - NULL_TREE);
> -
> - if (is_gimple_reg_type (ft))
> + tree elemtype = TREE_TYPE (decl_type);
> + tree elem_size = TYPE_SIZE (elemtype);
> + gcc_assert (elem_size && tree_fits_shwi_p (elem_size));
> + HOST_WIDE_INT el_size = tree_to_shwi (elem_size);
> + gcc_assert (el_size > 0);
> +
> + tree minidx = TYPE_MIN_VALUE (TYPE_DOMAIN (decl_type));
> + gcc_assert (TREE_CODE (minidx) == INTEGER_CST);
> + tree maxidx = TYPE_MAX_VALUE (TYPE_DOMAIN (decl_type));
> + if (maxidx)
> {
> - struct access *access;
> - HOST_WIDE_INT size;
> -
> - size = tree_to_uhwi (DECL_SIZE (fld));
> - access = create_access_1 (base, pos, size);
> - access->expr = nref;
> - access->type = ft;
> - access->grp_total_scalarization = 1;
> - /* Accesses for intraprocedural SRA can have their stmt NULL. */
> + gcc_assert (TREE_CODE (maxidx) == INTEGER_CST);
> + /* MINIDX and MAXIDX are inclusive. Try to avoid overflow. */
> + unsigned HOST_WIDE_INT lenp1 = tree_to_shwi (maxidx)
> + - tree_to_shwi (minidx);
> + unsigned HOST_WIDE_INT idx = 0;
> + do
> + {
> + tree nref = build4 (ARRAY_REF, elemtype, ref, size_int (idx),
> + NULL_TREE, NULL_TREE);
> + int el_off = offset + idx * el_size;
> + scalarize_elem (base, el_off, el_size, nref, elemtype);
> + }
> + while (++idx <= lenp1);
> }
> - else
> - completely_scalarize_record (base, fld, pos, nref);
> }
> + break;
> + default:
> + gcc_unreachable ();
> + }
> +}
> +
> +/* Create total_scalarization accesses for a member of type TYPE, which must
> + satisfy either is_gimple_reg_type or scalarizable_type_p. BASE must be
> the
> + top-most VAR_DECL representing the variable; within that, POS and SIZE
> locate
> + the member and REF must be the reference expression for it. */
> +
> +static void
> +scalarize_elem (tree base, HOST_WIDE_INT pos, HOST_WIDE_INT size,
> + tree ref, tree type)
> +{
> + if (is_gimple_reg_type (type))
> + {
> + struct access *access = create_access_1 (base, pos, size);
> + access->expr = ref;
> + access->type = type;
> + access->grp_total_scalarization = 1;
> + /* Accesses for intraprocedural SRA can have their stmt NULL. */
> + }
> + else
> + completely_scalarize (base, type, pos, ref);
> }
>
> /* Create a total_scalarization access for VAR as a whole. VAR must be of a
> - RECORD_TYPE conforming to type_consists_of_records_p. */
> + RECORD_TYPE or ARRAY_TYPE conforming to scalarizable_type_p. */
>
> static void
> create_total_scalarization_access (tree var)
> @@ -2521,13 +2590,13 @@ analyze_all_variable_accesses (void)
> tree var = candidate (i);
>
> if (TREE_CODE (var) == VAR_DECL
> - && type_consists_of_records_p (TREE_TYPE (var)))
> + && scalarizable_type_p (TREE_TYPE (var)))
> {
> if (tree_to_uhwi (TYPE_SIZE (TREE_TYPE (var)))
> <= max_scalarization_size)
> {
> create_total_scalarization_access (var);
> - completely_scalarize_record (var, var, 0, var);
> + completely_scalarize (var, TREE_TYPE (var), 0, var);
> if (dump_file && (dump_flags & TDF_DETAILS))
> {
> fprintf (dump_file, "Will attempt to totally scalarize ");
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
