Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 25 January 2015 at 22:33, Christophe Lyon christophe.l...@linaro.org wrote: On 17 December 2014 at 18:02, Tejas Belagod tejas.bela...@arm.com wrote: On 17/12/14 16:46, Marcus Shawcroft wrote: On 17 December 2014 at 15:15, Tejas Belagod tejas.bela...@arm.com wrote: It isn;t clear to me how far through the various BE patches we need to get before 59810 is actually resolved? David's 2 patches https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01431.html https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01099.html and Alan's 2 patches: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02797.html OK, thanks, my understanding is that all of the above are now blocked waiting some resolution on this patch to rtlanal.c: I believe so. Thanks, Tejas. Hi, If I'm not mistaken, this has been committed as r219959, and is causing regressions on aarch64_be for several AdvSimd intrinsic tests: vldX_lane, vtrn, vuzp, vzip, as well as vldN_1 and vstN_1 See: http://abe.tcwglab.linaro.org/logs/validations/cross-validation/gcc/trunk/219959/report-build-info.html These tests started passing at the previous commit (r219958) with the other half of this patch. I haven't looked at the details yet. I've been looking at these failures for some time. I currently have a patch which mostly reverts the one discussed in this thread, and makes the AdvSIMD tests pass. However, it creates regressions in some of the vectorizer tests. When r219959 was committed, I observed that these tests started to pass: gcc.dg/torture/pr52028.c gcc.dg/torture/pr53366-1.c gcc.dg/vect/pr37539.c gcc.dg/vect/pr40074.c gcc.dg/vect/pr51074.c gcc.dg/vect/pr59354.c gcc.dg/vect/pr64252.c gcc.dg/vect/slp-12b.c gcc.dg/vect/slp-19b.c gcc.dg/vect/slp-perm-8.c gcc.dg/vect/slp-perm-9.c gcc.dg/vect/vect-107.c gcc.dg/vect/vect-over-widen-1-big-array.c gcc.dg/vect/vect-over-widen-1.c gcc.dg/vect/vect-over-widen-2-big-array.c gcc.dg/vect/vect-over-widen-2.c gcc.dg/vect/vect-over-widen-3-big-array.c gcc.dg/vect/vect-over-widen-3.c gcc.dg/vect/vect-over-widen-4-big-array.c gcc.dg/vect/vect-over-widen-4.c gcc.dg/vect/vect-strided-store-a-u8-i2.c gcc.dg/vect/vect-strided-store-u16-i4.c gcc.dg/vect/vect-strided-store-u32-i2.c gcc.dg/vect/vect-strided-u16-i3.c I am looking at the pr59354.c testcase, which seems to be one the simplest showing a regression. The vectorizer does a pretty good at generating obfuscated code :-) and I find it quite difficult to debug, given the lack of debug environment. In fact, I still don't understand what r219959 actually fixed. I believe that https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810 is covered by the AdvSIMD tests, which r219959 actually breaks. In summary, I have the feeling that r219959 should be somewhat reverted such that the AdvSIMD tests pass on aarch64_be, but that this would expose a bug related to vectorization. Any advice appreciated. Thanks, Christophe. Christophe. https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01087.html /Marcus
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 17 December 2014 at 18:02, Tejas Belagod tejas.bela...@arm.com wrote: On 17/12/14 16:46, Marcus Shawcroft wrote: On 17 December 2014 at 15:15, Tejas Belagod tejas.bela...@arm.com wrote: It isn;t clear to me how far through the various BE patches we need to get before 59810 is actually resolved? David's 2 patches https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01431.html https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01099.html and Alan's 2 patches: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02797.html OK, thanks, my understanding is that all of the above are now blocked waiting some resolution on this patch to rtlanal.c: I believe so. Thanks, Tejas. Hi, If I'm not mistaken, this has been committed as r219959, and is causing regressions on aarch64_be for several AdvSimd intrinsic tests: vldX_lane, vtrn, vuzp, vzip, as well as vldN_1 and vstN_1 See: http://abe.tcwglab.linaro.org/logs/validations/cross-validation/gcc/trunk/219959/report-build-info.html These tests started passing at the previous commit (r219958) with the other half of this patch. I haven't looked at the details yet. Christophe. https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01087.html /Marcus
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 20/11/14 18:19, Marcus Shawcroft wrote: On 13 November 2014 10:09, David Sherwood david.sherw...@arm.com wrote: gcc/: 2014-11-13 David Sherwood david.sherw...@arm.com * config/aarch64/aarch64-protos.h (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): New decls. * config/aarch64/iterators.md (UNSPEC_REV_REGLIST): New enum. * config/aarch64/iterators.md (insn_count): New mode_attr. * 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. Spell these out in full please, some folks like to be able to grep for function names in these logs. * config/aarch64/aarch64-simd.md (aarch64_rev_reglist, aarch64_simd_(ld/st)(2/3/4)): Added. Likewise. * config/aarch64/aarch64.c (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): Added. It isn;t clear to me how far through the various BE patches we need to get before 59810 is actually resolved? David's 2 patches https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01431.html https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01099.html and Alan's 2 patches: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02797.html https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01087.html should fix 59810. Thanks, Tejas.
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 17 December 2014 at 15:15, Tejas Belagod tejas.bela...@arm.com wrote: It isn;t clear to me how far through the various BE patches we need to get before 59810 is actually resolved? David's 2 patches https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01431.html https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01099.html and Alan's 2 patches: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02797.html OK, thanks, my understanding is that all of the above are now blocked waiting some resolution on this patch to rtlanal.c: https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01087.html /Marcus
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 17/12/14 16:46, Marcus Shawcroft wrote: On 17 December 2014 at 15:15, Tejas Belagod tejas.bela...@arm.com wrote: It isn;t clear to me how far through the various BE patches we need to get before 59810 is actually resolved? David's 2 patches https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01431.html https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01099.html and Alan's 2 patches: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02797.html OK, thanks, my understanding is that all of the above are now blocked waiting some resolution on this patch to rtlanal.c: I believe so. Thanks, Tejas. https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01087.html /Marcus
RE: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
-Original Message- From: Christophe Lyon [mailto:christophe.l...@linaro.org] Sent: 11 December 2014 13:47 To: David Sherwood 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 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)? Yes, I think the change log is unclear and I will change it. The only thing that was broken was not adhering to the ABI, but we don't have any specific regression tests that prove this. 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 :-) Again, this is my fault as I made a mistake in the change log. If you look at the actual patch the CCMC macro is removed. Let me re-post corrected, more sensible change logs for both of those changes here: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe ChangeLog: gcc/: 2014-10-10 David Sherwood david.sherw...@arm.com 2014-10-10 Tejas
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 15 December 2014 at 10:56, David Sherwood david.sherw...@arm.com wrote: -Original Message- From: Christophe Lyon [mailto:christophe.l...@linaro.org] Sent: 11 December 2014 13:47 To: David Sherwood 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 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)? Yes, I think the change log is unclear and I will change it. The only thing that was broken was not adhering to the ABI, but we don't have any specific regression tests that prove this. OK thanks for the clarification. 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 :-) Again, this is my fault as I made a mistake in the change log. If you look at the actual patch the CCMC macro is removed. Let me re-post corrected, more sensible change logs for both of those changes here: [AArch64] [BE] [1/2] Make large opaque
RE: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
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 ... 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. Kind Regards, David Sherwood.
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
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.
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 ... 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. Kind Regards, David Sherwood.
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 13 November 2014 10:09, David Sherwood david.sherw...@arm.com wrote: gcc/: 2014-11-13 David Sherwood david.sherw...@arm.com * config/aarch64/aarch64-protos.h (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): New decls. * config/aarch64/iterators.md (UNSPEC_REV_REGLIST): New enum. * config/aarch64/iterators.md (insn_count): New mode_attr. * 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. Spell these out in full please, some folks like to be able to grep for function names in these logs. * config/aarch64/aarch64-simd.md (aarch64_rev_reglist, aarch64_simd_(ld/st)(2/3/4)): Added. Likewise. * config/aarch64/aarch64.c (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): Added. It isn;t clear to me how far through the various BE patches we need to get before 59810 is actually resolved? Cheers /Marcus
RE: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
Hi Christophe, Ah sorry. My mistake - it fixes this in bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810 This change is needed in order to remove the CANNOT_CHANGE_MODE_CLASS #define, which will be committed as a separate patch. Regards, David Sherwood. -Original Message- From: Christophe Lyon [mailto:christophe.l...@linaro.org] Sent: 17 November 2014 21:09 To: David Sherwood Cc: gcc-patches@gcc.gnu.org; Alan Hayward Subject: Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. On 13 November 2014 15:32, David Sherwood david.sherw...@arm.com wrote: Hi, I think that's because Alan Hayward's original patch had a bug in it, which he has fixed and should be submitting to gcc patches fairly soon. So it's probably best waiting until he gets a new patch up I think. I've applied both Alan's patches and the advsimd-intrinsics tests now all pass on aarch64_be, but this doesn't need your patch. Which testcase does your patch actually fix? Regards, David. -Original Message- From: Christophe Lyon [mailto:christophe.l...@linaro.org] Sent: 13 November 2014 14:22 To: David Sherwood Cc: gcc-patches@gcc.gnu.org Subject: Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. On 13 November 2014 11:09, David Sherwood david.sherw...@arm.com wrote: Hi All, I have successfully rebased this and tested in conjunction with a patch from Alan Hayward ([AArch64] [BE] Fix vector load/stores to not use ld1/st1), who should be submitting a new version shortly. Built and tested on: aarch64-none-elf aarch64_be-none-elf x86_64-linux-gnu I've applied both patches to recent trunk (r217483), and I still see ICEs on aarch64_be, when running the advsimd-intrinsics/vldX.c tests. I have them with Alan Hayward's patch alone, too. BTW, I thought that patch had been approved by Marcus, but it's not in trunk yet. Maybe I missed something. Christophe. Regards, David Sherwood. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 28 October 2014 08:55 To: 'gcc-patches@gcc.gnu.org' Subject: RE: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, Sorry to bother you again. Could someone take a look at this change please if they have time? Thanks! David. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 10 October 2014 15:48 To: gcc-patches@gcc.gnu.org Subject: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, I have a fix (originally written by Tejas Belagod) for the following bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810 Could someone take a look please? Thanks! David Sherwood. ChangeLog: gcc/: 2014-11-13 David Sherwood david.sherw...@arm.com * config/aarch64/aarch64-protos.h (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): New decls. * config/aarch64/iterators.md (UNSPEC_REV_REGLIST): New enum. * config/aarch64/iterators.md (insn_count): New mode_attr. * 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. * config/aarch64/aarch64-simd.md (aarch64_rev_reglist, aarch64_simd_(ld/st)(2/3/4)): Added. * config/aarch64/aarch64.c (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): Added.
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? This change is needed in order to remove the CANNOT_CHANGE_MODE_CLASS #define, which will be committed as a separate patch. Regards, David Sherwood. -Original Message- From: Christophe Lyon [mailto:christophe.l...@linaro.org] Sent: 17 November 2014 21:09 To: David Sherwood Cc: gcc-patches@gcc.gnu.org; Alan Hayward Subject: Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. On 13 November 2014 15:32, David Sherwood david.sherw...@arm.com wrote: Hi, I think that's because Alan Hayward's original patch had a bug in it, which he has fixed and should be submitting to gcc patches fairly soon. So it's probably best waiting until he gets a new patch up I think. I've applied both Alan's patches and the advsimd-intrinsics tests now all pass on aarch64_be, but this doesn't need your patch. Which testcase does your patch actually fix? Regards, David. -Original Message- From: Christophe Lyon [mailto:christophe.l...@linaro.org] Sent: 13 November 2014 14:22 To: David Sherwood Cc: gcc-patches@gcc.gnu.org Subject: Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. On 13 November 2014 11:09, David Sherwood david.sherw...@arm.com wrote: Hi All, I have successfully rebased this and tested in conjunction with a patch from Alan Hayward ([AArch64] [BE] Fix vector load/stores to not use ld1/st1), who should be submitting a new version shortly. Built and tested on: aarch64-none-elf aarch64_be-none-elf x86_64-linux-gnu I've applied both patches to recent trunk (r217483), and I still see ICEs on aarch64_be, when running the advsimd-intrinsics/vldX.c tests. I have them with Alan Hayward's patch alone, too. BTW, I thought that patch had been approved by Marcus, but it's not in trunk yet. Maybe I missed something. Christophe. Regards, David Sherwood. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 28 October 2014 08:55 To: 'gcc-patches@gcc.gnu.org' Subject: RE: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, Sorry to bother you again. Could someone take a look at this change please if they have time? Thanks! David. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 10 October 2014 15:48 To: gcc-patches@gcc.gnu.org Subject: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, I have a fix (originally written by Tejas Belagod) for the following bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810 Could someone take a look please? Thanks! David Sherwood. ChangeLog: gcc/: 2014-11-13 David Sherwood david.sherw...@arm.com * config/aarch64/aarch64-protos.h (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): New decls. * config/aarch64/iterators.md (UNSPEC_REV_REGLIST): New enum. * config/aarch64/iterators.md (insn_count): New mode_attr. * 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. * config/aarch64/aarch64-simd.md (aarch64_rev_reglist, aarch64_simd_(ld/st)(2/3/4)): Added. * config/aarch64/aarch64.c (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): Added.
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 13 November 2014 15:32, David Sherwood david.sherw...@arm.com wrote: Hi, I think that's because Alan Hayward's original patch had a bug in it, which he has fixed and should be submitting to gcc patches fairly soon. So it's probably best waiting until he gets a new patch up I think. I've applied both Alan's patches and the advsimd-intrinsics tests now all pass on aarch64_be, but this doesn't need your patch. Which testcase does your patch actually fix? Regards, David. -Original Message- From: Christophe Lyon [mailto:christophe.l...@linaro.org] Sent: 13 November 2014 14:22 To: David Sherwood Cc: gcc-patches@gcc.gnu.org Subject: Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. On 13 November 2014 11:09, David Sherwood david.sherw...@arm.com wrote: Hi All, I have successfully rebased this and tested in conjunction with a patch from Alan Hayward ([AArch64] [BE] Fix vector load/stores to not use ld1/st1), who should be submitting a new version shortly. Built and tested on: aarch64-none-elf aarch64_be-none-elf x86_64-linux-gnu I've applied both patches to recent trunk (r217483), and I still see ICEs on aarch64_be, when running the advsimd-intrinsics/vldX.c tests. I have them with Alan Hayward's patch alone, too. BTW, I thought that patch had been approved by Marcus, but it's not in trunk yet. Maybe I missed something. Christophe. Regards, David Sherwood. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 28 October 2014 08:55 To: 'gcc-patches@gcc.gnu.org' Subject: RE: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, Sorry to bother you again. Could someone take a look at this change please if they have time? Thanks! David. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 10 October 2014 15:48 To: gcc-patches@gcc.gnu.org Subject: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, I have a fix (originally written by Tejas Belagod) for the following bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810 Could someone take a look please? Thanks! David Sherwood. ChangeLog: gcc/: 2014-11-13 David Sherwood david.sherw...@arm.com * config/aarch64/aarch64-protos.h (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): New decls. * config/aarch64/iterators.md (UNSPEC_REV_REGLIST): New enum. * config/aarch64/iterators.md (insn_count): New mode_attr. * 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. * config/aarch64/aarch64-simd.md (aarch64_rev_reglist, aarch64_simd_(ld/st)(2/3/4)): Added. * config/aarch64/aarch64.c (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): Added.
Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
On 13 November 2014 11:09, David Sherwood david.sherw...@arm.com wrote: Hi All, I have successfully rebased this and tested in conjunction with a patch from Alan Hayward ([AArch64] [BE] Fix vector load/stores to not use ld1/st1), who should be submitting a new version shortly. Built and tested on: aarch64-none-elf aarch64_be-none-elf x86_64-linux-gnu I've applied both patches to recent trunk (r217483), and I still see ICEs on aarch64_be, when running the advsimd-intrinsics/vldX.c tests. I have them with Alan Hayward's patch alone, too. BTW, I thought that patch had been approved by Marcus, but it's not in trunk yet. Maybe I missed something. Christophe. Regards, David Sherwood. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 28 October 2014 08:55 To: 'gcc-patches@gcc.gnu.org' Subject: RE: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, Sorry to bother you again. Could someone take a look at this change please if they have time? Thanks! David. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 10 October 2014 15:48 To: gcc-patches@gcc.gnu.org Subject: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, I have a fix (originally written by Tejas Belagod) for the following bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810 Could someone take a look please? Thanks! David Sherwood. ChangeLog: gcc/: 2014-11-13 David Sherwood david.sherw...@arm.com * config/aarch64/aarch64-protos.h (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): New decls. * config/aarch64/iterators.md (UNSPEC_REV_REGLIST): New enum. * config/aarch64/iterators.md (insn_count): New mode_attr. * 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. * config/aarch64/aarch64-simd.md (aarch64_rev_reglist, aarch64_simd_(ld/st)(2/3/4)): Added. * config/aarch64/aarch64.c (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): Added.
RE: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe.
Hi, I think that's because Alan Hayward's original patch had a bug in it, which he has fixed and should be submitting to gcc patches fairly soon. So it's probably best waiting until he gets a new patch up I think. Regards, David. -Original Message- From: Christophe Lyon [mailto:christophe.l...@linaro.org] Sent: 13 November 2014 14:22 To: David Sherwood Cc: gcc-patches@gcc.gnu.org Subject: Re: New patch: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. On 13 November 2014 11:09, David Sherwood david.sherw...@arm.com wrote: Hi All, I have successfully rebased this and tested in conjunction with a patch from Alan Hayward ([AArch64] [BE] Fix vector load/stores to not use ld1/st1), who should be submitting a new version shortly. Built and tested on: aarch64-none-elf aarch64_be-none-elf x86_64-linux-gnu I've applied both patches to recent trunk (r217483), and I still see ICEs on aarch64_be, when running the advsimd-intrinsics/vldX.c tests. I have them with Alan Hayward's patch alone, too. BTW, I thought that patch had been approved by Marcus, but it's not in trunk yet. Maybe I missed something. Christophe. Regards, David Sherwood. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 28 October 2014 08:55 To: 'gcc-patches@gcc.gnu.org' Subject: RE: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, Sorry to bother you again. Could someone take a look at this change please if they have time? Thanks! David. -Original Message- From: David Sherwood [mailto:david.sherw...@arm.com] Sent: 10 October 2014 15:48 To: gcc-patches@gcc.gnu.org Subject: [AArch64] [BE] [1/2] Make large opaque integer modes endianness-safe. Hi, I have a fix (originally written by Tejas Belagod) for the following bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59810 Could someone take a look please? Thanks! David Sherwood. ChangeLog: gcc/: 2014-11-13 David Sherwood david.sherw...@arm.com * config/aarch64/aarch64-protos.h (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): New decls. * config/aarch64/iterators.md (UNSPEC_REV_REGLIST): New enum. * config/aarch64/iterators.md (insn_count): New mode_attr. * 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. * config/aarch64/aarch64-simd.md (aarch64_rev_reglist, aarch64_simd_(ld/st)(2/3/4)): Added. * config/aarch64/aarch64.c (aarch64_simd_attr_length_rglist, aarch64_reverse_mask): Added.
