On 11 December 2014 at 11:16, David Sherwood <david.sherw...@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.sherw...@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.sherw...@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. > > > >
