Re: [patch] Fix wrong code on dynamic array slices

[Richard Biener <richard dot guenther at gmail dot com>]

Ok.

Thanks,
Richard.

On Wed, Sep 16, 2015 at 9:07 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> the attached Ada testcase fails on x86/Linux (32-bit only) because the FRE
> pass wrongly deletes stores as redundant:
>
> Deleted redundant store VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[0]
> {lb: 4294967292 sz: 4} = 100;
> Deleted redundant store VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[1]
> {lb: 4294967292 sz: 4} = 1;
> Deleted redundant store VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[2]
> {lb: 4294967292 sz: 4} = 2;
> Deleted redundant store VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[3]
> {lb: 4294967292 sz: 4} = 3;
> Deleted redundant store VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[4]
> {lb: 4294967292 sz: 4} = 4;
>
> While the last 4 stores happen to be redundant (however the compiler cannot
> really determine it), the first one isn't and deleting it yields wrong code.
>
> These stores are part of a larger group of stores:
>
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[4294967292]{lb: 4294967292
> sz: 4} = -4;
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[4294967293]{lb: 4294967292
> sz: 4} = -3;
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[4294967294]{lb: 4294967292
> sz: 4} = -2;
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[4294967295]{lb: 4294967292
> sz: 4} = -1;
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[0]{lb: 4294967292 sz: 4} =
> 100;
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[1]{lb: 4294967292 sz: 4} =
> 1;
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[2]{lb: 4294967292 sz: 4} =
> 2;
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[3]{lb: 4294967292 sz: 4} =
> 3;
> VIEW_CONVERT_EXPR<integer[4294967292:4]>(*a.18_41)[4]{lb: 4294967292 sz: 4} =
> 4;
>
> and the first 4 ones are correctly detected as *not* redundant.  What changes
> between the two groups of stores is the index or, more precisely, the sign of
> the difference between the index and the low bound (4294967292 aka -4); it's
> positive for the first group and negative for the second group.  The latter
> case fools ao_ref_init_from_vn_reference because it doesn't properly truncate
> it to the precision of the index, like for example get_ref_base_and_extent,
> so as to make it positive again in this case, which in turn fools FRE.
>
> So the attached patch offset_int'ifies ao_ref_init_from_vn_reference the same
> way get_ref_base_and_extent was offset_int'ified (in fact double_int'ified at
> the time), adding the missing truncation in the process.
>
> Bootstrapped/regtested on x86_64-suse-linux, OK for the mainline?
>
>
> 2015-09-16  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * tree-ssa-sccvn.c (ao_ref_init_from_vn_reference): Use offset_int for
>         offset and size computations instead of HOST_WIDE_INT.
>
>
> 2015-09-16  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/opt49.adb: New test.
>
> --
> Eric Botcazou