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

On 30/10/15 10:54, Eric Botcazou wrote:
> On 30/10/15 10:44, Richard Biener wrote:
>>
>> 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).
[...]
> But using offset_int should be OK, see for example get_ref_base_and_extent.
>

Here's a patch using offset_int. (Not as easy to construct as wide_int::from,
the sign-extend is what appeared to be done elsewhere that constructs
offset_ints).

Tested by bootstrap+check-{gcc,g++,ada,fortran} with the rest of the patchset
(which causes an array[-1..1] to be completely scalarized, among others), on
x86_64 and ARM.

I don't have a test without all that (such would have to be in Ada, and trigger
SRA of such an array but not involving the constant pool); is it OK without?

gcc/ChangeLog:

	* tree-sra.c (completely_scalarize): Properly handle negative array
	indices using offset_int.
---
 gcc/tree-sra.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

On Thu, Nov 5, 2015 at 2:22 PM, Alan Lawrence <alan.lawrence@arm.com> wrote:
> On 30/10/15 10:54, Eric Botcazou wrote:
>> On 30/10/15 10:44, Richard Biener wrote:
>>>
>>> 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).
> [...]
>> But using offset_int should be OK, see for example get_ref_base_and_extent.
>>
>
> Here's a patch using offset_int. (Not as easy to construct as wide_int::from,
> the sign-extend is what appeared to be done elsewhere that constructs
> offset_ints).
>
> Tested by bootstrap+check-{gcc,g++,ada,fortran} with the rest of the patchset
> (which causes an array[-1..1] to be completely scalarized, among others), on
> x86_64 and ARM.
>
> I don't have a test without all that (such would have to be in Ada, and trigger
> SRA of such an array but not involving the constant pool); is it OK without?

Ok.

Richard.

> gcc/ChangeLog:
>
>         * tree-sra.c (completely_scalarize): Properly handle negative array
>         indices using offset_int.
> ---
>  gcc/tree-sra.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index e15df1f..6168a7e 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);
> -           unsigned HOST_WIDE_INT idx = 0;
> -           do
> +           tree domain = TYPE_DOMAIN (decl_type);
> +           /* MINIDX and MAXIDX are inclusive, and must be interpreted in
> +              DOMAIN (e.g. signed int, whereas min/max may be size_int).  */
> +           offset_int idx = wi::to_offset (minidx);
> +           offset_int max = wi::to_offset (maxidx);
> +           if (!TYPE_UNSIGNED (domain))
>               {
> -               tree nref = build4 (ARRAY_REF, elemtype, ref, size_int (idx),
> +               idx = wi::sext (idx, TYPE_PRECISION (domain));
> +               max = wi::sext (max, TYPE_PRECISION (domain));
> +             }
> +           for (int el_off = offset; wi::les_p (idx, max); ++idx)
> +             {
> +               tree nref = build4 (ARRAY_REF, elemtype,
> +                                   ref,
> +                                   wide_int_to_tree (domain, idx),
>                                     NULL_TREE, NULL_TREE);
> -               int el_off = offset + idx * el_size;
>                 scalarize_elem (base, el_off, el_size, nref, elemtype);
> +               el_off += el_size;
>               }
> -           while (++idx <= lenp1);
>           }
>        }
>        break;
> --
> 1.9.1
>