Re: [patch] Fix PR tree-optimization/70884

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

On Fri, May 13, 2016 at 1:01 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Hmm, the patch looks obvious if it was the intent to allow constant
>> pool replacements
>> _not_ only when the whole constant pool entry may go away.  But I
>> think the intent was
>> to not do this otherwise it will generate worse code by forcing all
>> loads from the constant pool to appear at
>> function start.
>
> Do you mean when the whole constant pool entry is scalarized as opposed to
> partially scalarized?

When we scalarized all constant pool accesses (even if it is not fully
accessed).
The whole point was to be able to remove the constant pool entry later.

At least if I remember correctly ... (it should in the end do what we now do
at gimplification time but with better analysis on the cost/benefit).

>> So - the "real" issue may be a missing
>> should_scalarize_away_bitmap/cannot_scalarize_away_bitmap
>> check somewhere.
>
> This seems to work:
>
> Index: tree-sra.c
> ===================================================================
> --- tree-sra.c  (revision 236195)
> +++ tree-sra.c  (working copy)
> @@ -2680,6 +2680,10 @@ analyze_all_variable_accesses (void)
>    EXECUTE_IF_SET_IN_BITMAP (tmp, 0, i, bi)
>      {
>        tree var = candidate (i);
> +      if (constant_decl_p (var)
> +         && (!bitmap_bit_p (should_scalarize_away_bitmap, i)
> +             || bitmap_bit_p (cannot_scalarize_away_bitmap, i)))
> +       continue;
>        struct access *access;
>
>        access = sort_and_splice_var_accesses (var);
>
> but I have no idea whether this is correct or not.
>
>
> Martin, are we sure to disable scalarization of constant_decl_p variables not
> covered by initialize_constant_pool_replacements that way?

Does the above "work"?  Aka, not cause testsuite regressions?  I remember
the original patch was mostly for ARM (where we don't scalarize sth at
gimplification time
for some "cost" reason).

Thanks,
Richard.

> --
> Eric Botcazou