This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR 83141] Prevent SRA from removing type changing assignment
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Martin Jambor <mjambor at suse dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenther at suse dot de>
- Date: Tue, 31 Jul 2018 18:29:10 -0700
- Subject: Re: [PR 83141] Prevent SRA from removing type changing assignment
- References: <ri6indmgk7q.fsf@suse.cz> <ri6fu8qghrg.fsf@suse.cz> <ri6vahlpeop.fsf@suse.cz>
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.