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: [PR 83141] Prevent SRA from removing type changing assignment


On Tue, Dec 5, 2017 at 4:00 AM, Martin Jambor <mjambor@suse.cz> wrote:
> On Tue, Dec 05 2017, Martin Jambor wrote:
>> On Tue, Dec 05 2017, Martin Jambor wrote:
>> Hi,
>>
>>> Hi,
>>>
>>> this is a followup to Richi's
>>> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02396.html to fix PR
>>> 83141.  The basic idea is simple, be just as conservative about type
>>> changing MEM_REFs as we are about actual VCEs.
>>>
>>> I have checked how that would affect compilation of SPEC 2006 and (non
>>> LTO) Mozilla Firefox and am happy to report that the difference was
>>> tiny.  However, I had to make the test less strict, otherwise testcase
>>> gcc.dg/guality/pr54970.c kept failing because it contains folded memcpy
>>> and expects us to track values accross:
>>>
>>>   int a[] = { 1, 2, 3 };
>>>   /* ... */
>>>   __builtin_memcpy (&a, (int [3]) { 4, 5, 6 }, sizeof (a));
>>>                              /* { dg-final { gdb-test 31 "a\[0\]" "4" } } */
>>>                              /* { dg-final { gdb-test 31 "a\[1\]" "5" } } */
>>>                              /* { dg-final { gdb-test 31 "a\[2\]" "6" } } */
>>>
>>> SRA is able to load replacement of a[0] directly from the temporary
>>> array which is apparently necessary to generate proper debug info.  I
>>> have therefore allowed the current transformation to go forward if the
>>> source does not contain any padding or if it is a read-only declaration.
>>
>> Ah, the read-only test is of course bogus, it was a last minute addition
>> when I was apparently already too tired to think it through.  Please
>> disregard that line in the patch (it has passed bootstrap and testing
>> without it).
>>
>> Sorry for the noise,
>>
>> Martin
>>
>
> And for the record, below is the actual patch, after a fresh round of
> re-testing to double check I did not mess up anything else.  As before,
> I'd like to ask for review, especially of the type_contains_padding_p
> predicate and then would like to commit it to trunk.
>
> Thanks,
>
> Martin
>
>
> 2017-12-05  Martin Jambor  <mjambor@suse.cz>
>
>         PR tree-optimization/83141
>         * tree-sra.c (type_contains_padding_p): New function.
>         (contains_vce_or_bfcref_p): Move up in the file, also test for
>         MEM_REFs implicitely changing types with padding.  Remove inline
>         keyword.
>         (build_accesses_from_assign): Check contains_vce_or_bfcref_p
>         before setting bit in should_scalarize_away_bitmap.
>

This caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86763

H.J.


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