Hi, As reported in https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01213.html , the new gcc.target/aarch64/store_v2vec_lanes.c fails when using -abi=ilp32. Kyrill said: My patch does indeed fail to generate the optimised sequence for -mabi=ilp32. During combine it fails to match: Failed to match this instruction: (set (mem:V2DF (plus:DI (reg/v/f:DI 79 [ z ]) (const_int 32 [0x20])) [1 MEM[(v2df *)z_8(D) + 32B]+0 S16 A128]) (vec_concat:V2DF (reg:DF 81 [ y0 ]) (reg:DF 84 [ y1 ]))) but without the -mabi=ilp32 it does successfully match the equivalent (set (mem:V2DF (plus:DI (reg:DI 1 x1 [ z ]) (const_int 32 [0x20])) [1 MEM[(v2df *)z_8(D) + 32B]+0 S16 A128]) (vec_concat:V2DF (reg:DF 81 [ y0 ]) (reg:DF 84 [ y1 ]))) The only difference is the index register being the hard reg x1. There's probably some subtlety in aarch64_classify_address that I'll need to dig into.
Thanks, confirmed.
New test so the regression is a testsuite issue (should be XFAILed for ilp32). Please do so ASAP, then we can drop the regression marker. Improving code-gen on ilp32 is an enhancement.
Author: ktkachov Date: Tue Mar 27 16:52:10 2018 New Revision: 258894 URL: https://gcc.gnu.org/viewcvs?rev=258894&root=gcc&view=rev Log: [AArch64] XFAIL gcc.target/aarch64/store_v2vec_lanes.c for ILP32 The test in question fails for ilp32. The initial analysis I did in the PR for it is that for ILP32 we generate somewhat different address forms that we'd need to adjust aarch64_classify_address to catch. Given the optimisation this test checks for was added for GCC 8 it is not a regression, and improving the codegen on ILP32 would be an enhancement rather than a fix. So Richi has asked for it to be marked as XFAIL on ILP32, which is what this patch does. Checked that the test still passes on LP64 and appears as XFAIL on -mabi=ilp32. PR target/83009 * gcc.target/aarch64/store_v2vec_lanes.c: XFAIL for ilp32. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/aarch64/store_v2vec_lanes.c
Test is XFAILed. Codegen improvement for ILP32 is not a blocker for GCC 8
I have been looking at this and the problem does indeed lie with the register not being a hard reg because aarch64_mem_pair_lanes_operand invokes aarch64_legitimate_address_p with 1 for the strict_p argument. Changing that to a 0 yields the desired results for this testcase. Also good to note that this is not an ilp32 issue only, because of this we would also miss cases where the argument hard-register would not be successfully combined into the load/store. Say if for instance the argument in the test function were a pointer to the pointer we are addressing. I will proceed to run tests now, if someone knows why that "strict_p" was being set to 1 please let me know, I am unfamiliar with this code and fear this might be too big a hammer.
(In reply to avieira from comment #5) > I have been looking at this and the problem does indeed lie with the > register not being a hard reg because aarch64_mem_pair_lanes_operand invokes > aarch64_legitimate_address_p with 1 for the strict_p argument. > > Changing that to a 0 yields the desired results for this testcase. Also good > to note that this is not an ilp32 issue only, because of this we would also > miss cases where the argument hard-register would not be successfully > combined into the load/store. Say if for instance the argument in the test > function were a pointer to the pointer we are addressing. > > I will proceed to run tests now, if someone knows why that "strict_p" was > being set to 1 please let me know, I am unfamiliar with this code and fear > this might be too big a hammer. I think requiring strict address checking in aarch64_mem_pair_lanes_operand is overly conservative indeed. The store_pair_lanes<mode> pattern that uses it is supposed to be created at combine-time at which point we want to allow any type of register. Then during reg-alloc the Uml constraint enforces the strict checking. So I think relaxing aarch64_mem_pair_lanes_operand is the right way forward, though of course testing will show.
Bootstrap and regression testing looks good. Ill put the patch up on the ML when we enter stage 1 again.
Author: avieira Date: Thu May 24 08:53:39 2018 New Revision: 260635 URL: https://gcc.gnu.org/viewcvs?rev=260635&root=gcc&view=rev Log: PR target/83009: Relax strict address checking for store pair lanes The operand constraint for the memory address of store/load pair lanes was enforcing strictly hardware registers be allowed as memory addresses. We want to relax that such that these patterns can be used by combine. During register allocation the register constraint will enforce the correct register is chosen. gcc 2018-05-24 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/83009 * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): Make address check not strict. gcc/testsuite 2018-05-24 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/83009 * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests. Modified: trunk/gcc/ChangeLog trunk/gcc/config/aarch64/predicates.md trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/aarch64/store_v2vec_lanes.c
I believe my patch fixes this.
Author: avieira Date: Wed May 30 15:59:14 2018 New Revision: 260957 URL: https://gcc.gnu.org/viewcvs?rev=260957&root=gcc&view=rev Log: Reverting r260635 gcc 2018-05-30 Andre Vieira <andre.simoesdiasvieira@arm.com> 2018-05-24 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/83009 Revert: * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): Make address check not strict. gcc/testsuite 2018-05-30 Andre Vieira <andre.simoesdiasvieira@arm.com> 2018-05-24 Andre Vieira <andre.simoesdiasvieira@arm.com> Revert PR target/83009 * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@260635 138bc75d-0d04-0410-961f-82ee72b054a4 Modified: trunk/gcc/config/aarch64/predicates.md trunk/gcc/testsuite/gcc.target/aarch64/store_v2vec_lanes.c
Author: avieira Date: Thu Jul 19 14:03:21 2018 New Revision: 262881 URL: https://gcc.gnu.org/viewcvs?rev=262881&root=gcc&view=rev Log: [AArch64][PATCH 2/2] PR target/83009: Relax strict address checking for store pair lanes gcc/ChangeLog 2018-07-19 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/83009 * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): Make address check not strict. gcc/testsuite/ChangeLog 2018-07-19 Andre Vieira <andre.simoesdiasvieira@arm.com> PR target/83009 * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests. Modified: trunk/gcc/ChangeLog trunk/gcc/config/aarch64/predicates.md trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/aarch64/store_v2vec_lanes.c