[PATCH 2/5] completely_scalarize arrays as well as records

[Christophe Lyon]

On 27 August 2015 at 17:43, Alan Lawrence <alan.lawrence@arm.com> wrote:
> Martin Jambor wrote:
>>
>> First, I would be much
>> happier if you added a proper comment to scalarize_elem function which
>> you forgot completely.  The name is not very descriptive and it has
>> quite few parameters too.
>>
>> Second, this patch should also fix PR 67283.  It would be great if you
>> could verify that and add it to the changelog when committing if that
>> is indeed the case.
>
> Thanks for pointing both of those out. I've added a comment to scalarize_elem,
> deleted the bogus comment in the new test, and yes I can confirm that the patch
> fixes PR 67283 on x86_64, and also AArch64 if
>  --param sra-max-scalarization-size-Ospeed is passed. (I've not added any
> testcase specifically taken from that PR, however.)
>
> Pushed as r277265.

Actually, is r227265.

Since since commit I've noticed that
g++.dg/torture/pr64312.C
fails at -O1 in my config, saying "virtual memory exhaustion" (arm* targets)
I run my validations under ulimit -v 10GB, which seems already large enough.

Do we consider this a bug?

Christophe.

> ---
>  gcc/testsuite/gcc.dg/tree-ssa/sra-15.c |  37 ++++++++
>  gcc/tree-sra.c                         | 149 ++++++++++++++++++++++-----------
>  2 files changed, 138 insertions(+), 48 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/sra-15.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/tree-sra.c b/gcc/tree-sra.c
> index 8b3a0ad..3caf84a 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -915,73 +915,126 @@ 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 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:
> +    {
> +      tree elem = TREE_TYPE (type);
> +      if (DECL_P (elem) && DECL_BIT_FIELD (elem))
> +       return false;
> +      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_uhwi_p (elem_size));
> +       int el_size = tree_to_uhwi (elem_size);
> +       gcc_assert (el_size);
> +
> +       tree minidx = TYPE_MIN_VALUE (TYPE_DOMAIN (decl_type));
> +       tree maxidx = TYPE_MAX_VALUE (TYPE_DOMAIN (decl_type));
> +       gcc_assert (TREE_CODE (minidx) == INTEGER_CST
> +                   && TREE_CODE (maxidx) == INTEGER_CST);
> +       unsigned HOST_WIDE_INT len = tree_to_uhwi (maxidx)
> +                                    + 1 - tree_to_uhwi (minidx);
> +       /* 4th operand to ARRAY_REF is size in units of the type alignment.  */
> +       for (unsigned HOST_WIDE_INT idx = 0; idx < len; idx++)
>           {
> -           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.  */
> +           tree t_idx = build_int_cst (TYPE_DOMAIN (decl_type), idx);
> +           tree nref = build4 (ARRAY_REF, elemtype, ref, t_idx, NULL_TREE,
> +                               NULL_TREE);
> +           int el_off = offset + idx * el_size;
> +           scalarize_elem (base, el_off, el_size, nref, elemtype);
>           }
> -       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 +2574,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 ");
> --
> 1.8.3
>