[5/6] tree-sra.c: Fix completely_scalarize for negative array indices

The code I added to completely_scalarize for arrays isn't right in some cases
of negative array indices (e.g. arrays with indices from -1 to 1 in the Ada
testsuite). On ARM, this prevents a failure bootstrapping Ada with the next
patch, as well as a few ACATS tests (e.g. c64106a).

Some discussion here: https://gcc.gnu.org/ml/gcc/2015-10/msg00096.html .

gcc/ChangeLog:

	* tree-sra.c (completely_scalarize): Deal properly with negative array
	indices.
---
 gcc/tree-sra.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

On Thu, Oct 29, 2015 at 8:18 PM, Alan Lawrence <alan.lawrence@arm.com> wrote:
> The code I added to completely_scalarize for arrays isn't right in some cases
> of negative array indices (e.g. arrays with indices from -1 to 1 in the Ada
> testsuite). On ARM, this prevents a failure bootstrapping Ada with the next
> patch, as well as a few ACATS tests (e.g. c64106a).
>
> Some discussion here: https://gcc.gnu.org/ml/gcc/2015-10/msg00096.html .
>
> gcc/ChangeLog:
>
>         * tree-sra.c (completely_scalarize): Deal properly with negative array
>         indices.
> ---
>  gcc/tree-sra.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index e15df1f..358db79 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -1010,18 +1010,25 @@ completely_scalarize (tree base, tree decl_type, HOST_WIDE_INT offset, tree ref)
>         if (maxidx)
>           {
>             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);
> +           /* MINIDX and MAXIDX are inclusive, and must be interpreted in the
> +              TYPE_DOMAIN (e.g. signed int, whereas min/max may be size_int).
> +              Try also to avoid overflow.  */
> +           minidx = build_int_cst (TYPE_DOMAIN (decl_type),
> +                                   tree_to_shwi (minidx));
> +           maxidx = build_int_cst (TYPE_DOMAIN (decl_type),
> +                                   tree_to_shwi (maxidx));

I think you want to use wide-ints here and

   wide_int idx = wi::from (minidx, TYPE_PRECISION (TYPE_DOMAIN
(...)), TYPE_SIGN (TYPE_DOMAIN (..)));
   wide_int maxidx = ...

you can then simply iterate minidx with ++ and do the final compare
against maxidx
with while (++idx <= maxidx).  For the array ref index we want to use
TYPE_DOMAIN
as type as well, not size_int.  Thus wide_int_to_tree (TYPE_DOMAIN (...)..idx).

RIchard.

> +           HOST_WIDE_INT min = tree_to_shwi (minidx);
> +           unsigned HOST_WIDE_INT lenlessone = tree_to_shwi (maxidx) - min;
>             unsigned HOST_WIDE_INT idx = 0;
>             do
>               {
> -               tree nref = build4 (ARRAY_REF, elemtype, ref, size_int (idx),
> +               tree nref = build4 (ARRAY_REF, elemtype,
> +                                   ref, size_int (idx + min),
>                                     NULL_TREE, NULL_TREE);
>                 int el_off = offset + idx * el_size;
>                 scalarize_elem (base, el_off, el_size, nref, elemtype);
>               }
> -           while (++idx <= lenp1);
> +           while (++idx <= lenlessone);
>           }
>        }
>        break;
> --
> 1.9.1
>

> I think you want to use wide-ints here and
> 
>    wide_int idx = wi::from (minidx, TYPE_PRECISION (TYPE_DOMAIN
> (...)), TYPE_SIGN (TYPE_DOMAIN (..)));
>    wide_int maxidx = ...
> 
> you can then simply iterate minidx with ++ and do the final compare
> against maxidx
> with while (++idx <= maxidx).  For the array ref index we want to use
> TYPE_DOMAIN
> as type as well, not size_int.  Thus wide_int_to_tree (TYPE_DOMAIN
> (...)..idx).

Yes, you generally cannot use HOST_WIDE_INT to avoid overflow because this 
will break for 64-bit HOST_WIDE_INT and 32-bit sizetype in corner cases.

But using offset_int should be OK, see for example get_ref_base_and_extent.