This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] Fix PR tree-optimization/70884
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Eric Botcazou <ebotcazou at adacore dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, alan dot lawrence at arm dot com, Martin Jambor <mjambor at suse dot cz>
- Date: Fri, 13 May 2016 13:52:29 +0200
- Subject: Re: [patch] Fix PR tree-optimization/70884
- Authentication-results: sourceware.org; auth=none
- References: <6226269 dot FBBDIS7nhY at polaris> <CAFiYyc0bLjfH4c2r1xXJDqKQm43WA1kKeBQHUBdGgDA8qoREqQ at mail dot gmail dot com> <20609673 dot FxQTIz62nb at polaris>
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