Re: [AArch64][PATCH 2/2] PR target/83009: Relax strict address checking for store pair lanes
On Mon, Jun 25, 2018 at 03:48:43AM -0500, Andre Simoes Dias Vieira wrote: > On 14/06/18 12:47, Richard Sandiford wrote: > > Kyrill Tkachov writes: > >> Hi Andre, > >> On 07/06/18 18:02, Andre Simoes Dias Vieira wrote: > >>> Hi, > >>> > >>> See below a patch to address PR 83009. > >>> > >>> Tested with aarch64-linux-gnu bootstrap and regtests for c, c++ and > >>> fortran. > >>> Ran the adjusted testcase for -mabi=ilp32. > >>> > >>> Is this OK for trunk? > >>> > >>> Cheers, > >>> Andre > >>> > >>> 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, prior > >>> to reload. > >>> During register allocation the register constraint will enforce the > >>> correct > >>> register is chosen. > >>> > >>> gcc > >>> 2018-06-07 Andre Vieira > >>> > >>> PR target/83009 > >>> * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): > >>> Make > >>> address check not strict prior to reload. > >>> > >>> gcc/testsuite > >>> 2018-06-07 Andre Vieira > >>> > >>> PR target/83009 > >>> * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests. > >> > >> diff --git a/gcc/config/aarch64/predicates.md > >> b/gcc/config/aarch64/predicates.md > >> index > >> f0917af8b4cec945ba4e38e4dc670200f8812983..30aa88838671bf343a883077c2b606a035c030dd > >> 100644 > >> --- a/gcc/config/aarch64/predicates.md > >> +++ b/gcc/config/aarch64/predicates.md > >> @@ -227,7 +227,7 @@ > >> (define_predicate "aarch64_mem_pair_lanes_operand" > >> (and (match_code "mem") > >> (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP > >> (op, 0), > >> -true, > >> +reload_completed, > >> ADDR_QUERY_LDP_STP_N)"))) > >> > >> > >> If you want to enforce strict checking during reload and later then I > >> think you need to use reload_in_progress || reload_completed ? > > > > That was the old way, but it would be lra_in_progress now. > > However... > > > >> I guess that would be equivalent to !can_create_pseudo (). > > > > We should never see pseudos when reload_completed, so the choice > > shouldn't really matter then. And I don't think we should use > > lra_in_progress either, since that would make the checks stricter > > before RA has actually happened, which would likely lead to an > > unrecognisable insn ICE if recog is called during one of the LRA > > subpasses. > > > > So unless we know a reason otherwise, I think this should just > > be "false" (like it already is for aarch64_mem_pair_operand). > > > > Thanks, > > Richard > > > Changed it to false. > > Bootstrapped and regression testing for aarch64-none-linux-gnu. > > Is this OK for trunk? OK. Thanks, James > gcc > 2018-06-25 Andre Vieira > > PR target/83009 > * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): > Make > address check not strict. > > gcc/testsuite > 2018-06-25 Andre Vieira > > PR target/83009 > * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests.
Re: [AArch64][PATCH 2/2] PR target/83009: Relax strict address checking for store pair lanes
On 14/06/18 12:47, Richard Sandiford wrote:
> Kyrill Tkachov writes:
>> Hi Andre,
>> On 07/06/18 18:02, Andre Simoes Dias Vieira wrote:
>>> Hi,
>>>
>>> See below a patch to address PR 83009.
>>>
>>> Tested with aarch64-linux-gnu bootstrap and regtests for c, c++ and fortran.
>>> Ran the adjusted testcase for -mabi=ilp32.
>>>
>>> Is this OK for trunk?
>>>
>>> Cheers,
>>> Andre
>>>
>>> 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, prior
>>> to reload.
>>> During register allocation the register constraint will enforce the correct
>>> register is chosen.
>>>
>>> gcc
>>> 2018-06-07 Andre Vieira
>>>
>>> PR target/83009
>>> * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand):
>>> Make
>>> address check not strict prior to reload.
>>>
>>> gcc/testsuite
>>> 2018-06-07 Andre Vieira
>>>
>>> PR target/83009
>>> * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests.
>>
>> diff --git a/gcc/config/aarch64/predicates.md
>> b/gcc/config/aarch64/predicates.md
>> index
>> f0917af8b4cec945ba4e38e4dc670200f8812983..30aa88838671bf343a883077c2b606a035c030dd
>> 100644
>> --- a/gcc/config/aarch64/predicates.md
>> +++ b/gcc/config/aarch64/predicates.md
>> @@ -227,7 +227,7 @@
>> (define_predicate "aarch64_mem_pair_lanes_operand"
>> (and (match_code "mem")
>> (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op,
>> 0),
>> - true,
>> + reload_completed,
>>ADDR_QUERY_LDP_STP_N)")))
>>
>>
>> If you want to enforce strict checking during reload and later then I think
>> you need to use reload_in_progress || reload_completed ?
>
> That was the old way, but it would be lra_in_progress now.
> However...
>
>> I guess that would be equivalent to !can_create_pseudo ().
>
> We should never see pseudos when reload_completed, so the choice
> shouldn't really matter then. And I don't think we should use
> lra_in_progress either, since that would make the checks stricter
> before RA has actually happened, which would likely lead to an
> unrecognisable insn ICE if recog is called during one of the LRA
> subpasses.
>
> So unless we know a reason otherwise, I think this should just
> be "false" (like it already is for aarch64_mem_pair_operand).
>
> Thanks,
> Richard
>
Changed it to false.
Bootstrapped and regression testing for aarch64-none-linux-gnu.
Is this OK for trunk?
Cheers,
Andre
gcc
2018-06-25 Andre Vieira
PR target/83009
* config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand):
Make
address check not strict.
gcc/testsuite
2018-06-25 Andre Vieira
PR target/83009
* gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests.
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index f0917af8b4cec945ba4e38e4dc670200f8812983..e1a022b5c5a371230c71cd1dd944f5b0d4f4dc4c 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -227,7 +227,7 @@
(define_predicate "aarch64_mem_pair_lanes_operand"
(and (match_code "mem")
(match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0),
- true,
+ false,
ADDR_QUERY_LDP_STP_N)")))
(define_predicate "aarch64_prefetch_operand"
diff --git a/gcc/testsuite/gcc.target/aarch64/store_v2vec_lanes.c b/gcc/testsuite/gcc.target/aarch64/store_v2vec_lanes.c
index 990aea32de6f8239effa95a081950684c6e11386..3296d04da14149d26d19da785663b87bd5ad8994 100644
--- a/gcc/testsuite/gcc.target/aarch64/store_v2vec_lanes.c
+++ b/gcc/testsuite/gcc.target/aarch64/store_v2vec_lanes.c
@@ -22,10 +22,32 @@ construct_lane_2 (long long *y, v2di *z)
z[2] = x;
}
+void
+construct_lane_3 (double **py, v2df **pz)
+{
+ double *y = *py;
+ v2df *z = *pz;
+ double y0 = y[0] + 1;
+ double y1 = y[1] + 2;
+ v2df x = {y0, y1};
+ z[2] = x;
+}
+
+void
+construct_lane_4 (long long **py, v2di **pz)
+{
+ long long *y = *py;
+ v2di *z = *pz;
+ long long y0 = y[0] + 1;
+ long long y1 = y[1] + 2;
+ v2di x = {y0, y1};
+ z[2] = x;
+}
+
/* We can use the load_pair_lanes pattern to vec_concat two DI/DF
values from consecutive memory into a 2-element vector by using
a Q-reg LDR. */
-/* { dg-final { scan-assembler-times "stp\td\[0-9\]+, d\[0-9\]+" 1 { xfail ilp32 } } } */
-/* { dg-final { scan-assembler-times "stp\tx\[0-9\]+, x\[0-9\]+" 1 { xfail ilp32 } } } */
-/* { dg-final { scan-assembler-not "ins\t" { xfail ilp32 } } } */
+/* { dg-final { scan-assembler-times "stp\td\[0-9\]+, d\[0-9\]+" 2 } } */
+/* { dg-final { scan-assembler-times "stp\tx\[0-9\]+, x\[0-9\]+" 2 } } */
+/* { dg-final
Re: [AArch64][PATCH 2/2] PR target/83009: Relax strict address checking for store pair lanes
Kyrill Tkachov writes: > Hi Andre, > On 07/06/18 18:02, Andre Simoes Dias Vieira wrote: >> Hi, >> >> See below a patch to address PR 83009. >> >> Tested with aarch64-linux-gnu bootstrap and regtests for c, c++ and fortran. >> Ran the adjusted testcase for -mabi=ilp32. >> >> Is this OK for trunk? >> >> Cheers, >> Andre >> >> 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, prior >> to reload. >> During register allocation the register constraint will enforce the correct >> register is chosen. >> >> gcc >> 2018-06-07 Andre Vieira >> >> PR target/83009 >> * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): Make >> address check not strict prior to reload. >> >> gcc/testsuite >> 2018-06-07 Andre Vieira >> >> PR target/83009 >> * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests. > > diff --git a/gcc/config/aarch64/predicates.md > b/gcc/config/aarch64/predicates.md > index > f0917af8b4cec945ba4e38e4dc670200f8812983..30aa88838671bf343a883077c2b606a035c030dd > 100644 > --- a/gcc/config/aarch64/predicates.md > +++ b/gcc/config/aarch64/predicates.md > @@ -227,7 +227,7 @@ > (define_predicate "aarch64_mem_pair_lanes_operand" > (and (match_code "mem") > (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, > 0), > - true, > + reload_completed, > ADDR_QUERY_LDP_STP_N)"))) > > > If you want to enforce strict checking during reload and later then I think > you need to use reload_in_progress || reload_completed ? That was the old way, but it would be lra_in_progress now. However... > I guess that would be equivalent to !can_create_pseudo (). We should never see pseudos when reload_completed, so the choice shouldn't really matter then. And I don't think we should use lra_in_progress either, since that would make the checks stricter before RA has actually happened, which would likely lead to an unrecognisable insn ICE if recog is called during one of the LRA subpasses. So unless we know a reason otherwise, I think this should just be "false" (like it already is for aarch64_mem_pair_operand). Thanks, Richard
Re: [AArch64][PATCH 2/2] PR target/83009: Relax strict address checking for store pair lanes
Hi Andre, On 07/06/18 18:02, Andre Simoes Dias Vieira wrote: Hi, See below a patch to address PR 83009. Tested with aarch64-linux-gnu bootstrap and regtests for c, c++ and fortran. Ran the adjusted testcase for -mabi=ilp32. Is this OK for trunk? Cheers, Andre 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, prior to reload. During register allocation the register constraint will enforce the correct register is chosen. gcc 2018-06-07 Andre Vieira PR target/83009 * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): Make address check not strict prior to reload. gcc/testsuite 2018-06-07 Andre Vieira PR target/83009 * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests. diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md index f0917af8b4cec945ba4e38e4dc670200f8812983..30aa88838671bf343a883077c2b606a035c030dd 100644 --- a/gcc/config/aarch64/predicates.md +++ b/gcc/config/aarch64/predicates.md @@ -227,7 +227,7 @@ (define_predicate "aarch64_mem_pair_lanes_operand" (and (match_code "mem") (match_test "aarch64_legitimate_address_p (GET_MODE (op), XEXP (op, 0), - true, + reload_completed, ADDR_QUERY_LDP_STP_N)"))) If you want to enforce strict checking during reload and later then I think you need to use reload_in_progress || reload_completed ? I guess that would be equivalent to !can_create_pseudo (). Thanks, Kyrill
[AArch64][PATCH 2/2] PR target/83009: Relax strict address checking for store pair lanes
Hi, See below a patch to address PR 83009. Tested with aarch64-linux-gnu bootstrap and regtests for c, c++ and fortran. Ran the adjusted testcase for -mabi=ilp32. Is this OK for trunk? Cheers, Andre 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, prior to reload. During register allocation the register constraint will enforce the correct register is chosen. gcc 2018-06-07 Andre Vieira PR target/83009 * config/aarch64/predicates.md (aarch64_mem_pair_lanes_operand): Make address check not strict prior to reload. gcc/testsuite 2018-06-07 Andre Vieira PR target/83009 * gcc/target/aarch64/store_v2vec_lanes.c: Add extra tests. stp-2.patch Description: stp-2.patch
