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: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.


On 11 December 2014 at 11:16, David Sherwood <david.sherwood@arm.com> wrote:
> Hi Christophe,
>
> Sorry to bother you again. After my clarification email below are you now
> happy for these patches to go in?
>
> Kind Regards,
> David Sherwood.
>
>> -----Original Message-----
>> From: David Sherwood [mailto:david.sherwood@arm.com]
>> Sent: 27 November 2014 14:53
>> To: 'Christophe Lyon'
>> Cc: gcc-patches@gcc.gnu.org; Marcus Shawcroft; Alan Hayward; 'Tejas Belagod'; Richard Sandiford
>> Subject: RE: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
>>
>> > On 18 November 2014 10:14, David Sherwood <david.sherwood@arm.com> wrote:
>> > > Hi Christophe,
>> > >
>> > > Ah sorry. My mistake - it fixes this in bugzilla:
>> > >
>> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810
>> >
>> > I did look at that PR, but since it has no testcase attached, I was unsure.
>> > And I am still not :-)
>> > PR 59810 is "[AArch64] LDn/STn implementations are not ABI-conformant
>> > for bigendian."
>> > but the advsimd-intrinsics/vldX.c and vldX_lane.c now PASS with Alan's
>> > patches on aarch64_be, so I thought Alan's patches solve PR59810.
>> >
>> > What am I missing?
>>
>> Hi Christophe,
>>
>> I think probably this is our fault for making our lives way too difficult and
>> artificially splitting all these patches up. :)
>>
>> Alan's patch:
>>
>> https://gcc.gnu.org/ml/gcc-patches/2014-10/msg00952.html
>>
>> fixes some issues on aarch64_be, but also causes regressions. For example,
>>
>> ====
>> Tests that now fail, but worked before:
>>
>> aarch64_be-elf-aem: gcc.dg/vect/slp-perm-8.c -flto -ffat-lto-objects execution test
>> aarch64_be-elf-aem: gcc.dg/vect/slp-perm-8.c execution test
>> aarch64_be-elf-aem: gcc.dg/vect/vect-over-widen-1-big-array.c -flto -ffat-lto-objects execution test
>> ...
>>
>> Tests that now work, but didn't before:
>>
>> aarch64_be-elf-aem: gcc.dg/vect/fast-math-vect-complex-3.c execution test
>> aarch64_be-elf-aem: gcc.dg/vect/if-cvt-stores-vect-ifcvt-18.c execution test
>> aarch64_be-elf-aem: gcc.dg/vect/no-scevccp-outer-10a.c execution test
>> ...
>> ====
I didn't notice that because I tested Alan's patch only against the
advsimd-intrinsics tests.
In this respect, I don't understand why your ChangeLog entry says
       * config/aarch64/aarch64-simd.md (vec_store_lanes(o/c/x)i,
        vec_load_lanes(o/c/x)i): Fixed to work for Big Endian.
since the existing advsimd-intrinsics tests already pass with Alan's patch alone
or is vld1_lane still broken (for which I haven't posted a test yet)?

>> His patch is only half of the story and must be applied at the same time as the
>> "[AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe."
>> patch. With both patches applied the result looks much healthier:
>>
>> ====
>> # Comparing 1 common sum files
>> ## /bin/sh ./src/gcc/contrib/compare_tests  /tmp/gxx-sum1.10051 /tmp/gxx-sum2.10051
>> Tests that now work, but didn't before:
>>
>> aarch64_be-elf-aem: gcc.dg/torture/pr52028.c   -O3 -fomit-frame-pointer  execution test
>> aarch64_be-elf-aem: gcc.dg/torture/pr52028.c   -O3 -fomit-frame-pointer -funroll-all-loops -finline-
>> functions  execution test
>> aarch64_be-elf-aem: gcc.dg/torture/pr52028.c   -O3 -fomit-frame-pointer -funroll-loops  execution test
>> ...
>> ====
>>
>> with no new regressions. After applying both patches the aarch64_be gcc testsuite is
>> on a parity with the aarch64 testsuite. Furthermore, after applying both of these patches:
>>
>> "[AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe"
>> "[AArch64] [BE] Fix vector load/stores to not use ld1/st1"
>>
>> it then becomes safe for us to remove the CCMC macro, which is the cause of
>> unnecessary spills to the stack for certain auto-vectorised code. So really I
>> suppose when I posted my second patch
>>
>> "[AArch64] [BE] [2/2] Make large opaque integer modes endianness-safe"
>>
>> I should have really just called this
>>
>> "[AArch64] [BE] Remove CCMC for aarch64"
>>
>> in order to make it clear exactly what the purpose of these patches is.
well, not yet since this very does not remove it :-)

>>
>> Kind Regards,
>> David Sherwood.
>
>
>
>


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