This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC 4/5] Handle constant-pool entries


On Tue, Aug 25, 2015 at 10:13 PM, Jeff Law <law@redhat.com> wrote:
> On 08/25/2015 05:06 AM, Alan Lawrence wrote:
>>
>> This makes SRA replace loads of records/arrays from constant pool entries,
>> with elementwise assignments of the constant values, hence, overcoming the
>> fundamental problem in PR/63679.
>>
>> As a first pass, the approach I took was to look for constant-pool loads
>> as
>> we scanned through other accesses, and add them as candidates there; to
>> build a
>> constant replacement_decl for any such accesses in completely_scalarize;
>> and to
>> use any existing replacement_decl rather than creating a variable in
>> create_access_replacement. (I did try using CONSTANT_CLASS_P in the
>> latter, but
>> that does not allow addresses of labels, which can still end up in the
>> constant
>> pool.)
>>
>> Feedback as to the approach or how it might be better structured / fitted
>> into
>> SRA, is solicited ;).
>>
>> Bootstrapped + check-gcc on x86-none-linux-gnu, aarch64-none-linux-gnu and
>> arm-none-linux-gnueabihf, including with the next patch (rfc), which
>> greatly increases the number of testcases in which this code is exercised!
>>
>> Have also verified that the ssa-dom-cse-2.c scan-tree-dump test passes
>> (using a stage 1 compiler only, without execution) on alpha, hppa, powerpc,
>> sparc, avr, and sh.
>>
>> gcc/ChangeLog:
>>
>>         * tree-sra.c (create_access): Scan for uses of constant pool and
>> add
>>         to candidates.
>>         (subst_initial): New.
>>         (scalarize_elem): Build replacement_decl using subst_initial.
>>         (create_access_replacement): Use replacement_decl if set.
>>
>> gcc/testsuite/ChangeLog:
>>
>>         * gcc.dg/tree-ssa/ssa-dom-cse-2.c: Remove xfail, add --param
>>         sra-max-scalarization-size-Ospeed.
>> ---
>>   gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-2.c |  7 +---
>>   gcc/tree-sra.c                                | 56
>> +++++++++++++++++++++++++--
>>   2 files changed, 55 insertions(+), 8 deletions(-)
>>
>> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
>> index af35fcc..a3ff2df 100644
>> --- a/gcc/tree-sra.c
>> +++ b/gcc/tree-sra.c
>> @@ -865,6 +865,17 @@ create_access (tree expr, gimple stmt, bool write)
>>     else
>>       ptr = false;
>>
>> +  /* FORNOW: scan for uses of constant pool as we go along.  */
>
> I'm not sure why you have this marked as FORNOW.  If I'm reading all this
> code correctly, you're lazily adding items from the constant pool into the
> candidates table when you find they're used.  That seems better than walking
> the entire constant pool adding them all to the candidates.
>
> I don't see this as fundamentally wrong or unclean.
>
> The question I have is why this differs from the effects of patch #5. That
> would seem to indicate that there's things we're not getting into the
> candidate tables with this approach?!?
>
>
>
>> @@ -1025,6 +1036,37 @@ completely_scalarize (tree base, tree decl_type,
>> HOST_WIDE_INT offset, tree ref)
>>       }
>>   }
>>
>> +static tree
>> +subst_initial (tree expr, tree var)
>
> Function comment.
>
> I think this patch is fine with the function comment added and removing the
> FORNOW part of the comment in create_access.  It may be worth noting in
> create_access's comment that it can add new items to the candidates tables
> for constant pool entries.

I'm happy seeing this code in SRA as I never liked that we already decide
at gimplification time which initializers to expand and which to init from
a constant pool entry.  So ... can we now "remove" gimplify_init_constructor
by _always_ emitting a constant pool entry and an assignment from it
(obviously only if the constructor can be put into the constant pool)?  Defering
the expansion decision to SRA makes it possible to better estimate whether
the code is hot/cold or whether the initialized variable can be replaced by
the constant pool entry completely (variable ends up readonly).

Oh, and we'd no longer create the awful split code at -O0 ...

So can you explore that a bit once this series is settled?  This is probably
also related to 5/5 as this makes all the target dependent decisions in SRA
now and thus the initial IL from gimplification should be the same for all
targets (that's always a nice thing to have IMHO).

Thanks,
Richard.

>
> Jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]