* [PATCH][ARM] PR target/71270 fix neon_valid_immediate for big-endian
@ 2017-01-06 11:55 Kyrill Tkachov
2017-01-18 9:48 ` Kyrill Tkachov
2017-01-20 14:34 ` Richard Earnshaw (lists)
0 siblings, 2 replies; 4+ messages in thread
From: Kyrill Tkachov @ 2017-01-06 11:55 UTC (permalink / raw)
To: GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
[-- Attachment #1: Type: text/plain, Size: 1630 bytes --]
Hi all,
In this wrong-code issue the RTL tries to load a const_vector:
(const_vector:V8QI [
(const_int 1 [0x1])
(const_int 0 [0])
(const_int 1 [0x1])
(const_int 0 [0])
(const_int 1 [0x1])
(const_int 0 [0])
(const_int 1 [0x1])
(const_int 0 [0])
])
into a NEON register. The logic for that is in neon_valid_immediate which does a number of tricks
to decide what value and of what size to do a VMOV on to load the correct vector immediate into the register.
It goes wrong on big-endian. On both big and little-endian this outputs:
vmov.i16 d18, #0x1
This is wrong on big-endian. The vector layout has to be such as if loaded from memory.
I've tried various approaches of fixing neon_valid_immediate to generate the correct immediate but have been unsuccessful,
resulting in regressions in various parts of the testsuite or making a big mess of the function.
Given that armeb is not a target of major concern I believe the safest route at this stage is to only allow vector constants
that will obviously work on big-endian, that is the ones that are just a single element duplicated in all lanes.
This patch fixes the execution failures:
FAIL: gfortran.dg/intrinsic_pack_1.f90
FAIL: gfortran.dg/c_f_pointer_logical.f03
FAIL: gcc.dg/torture/pr60183.c
FAIL: gcc.dg/vect/pr51581-4.c
on armeb-none-eabi.
Ok for trunk?
Thanks,
Kyrill
2017-01-06 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR target/71270
* config/arm/arm.c (neon_valid_immediate): Reject vector constants
in big-endian mode when they are not a single duplicated value.
[-- Attachment #2: armeb-vec.patch --]
[-- Type: text/x-patch, Size: 710 bytes --]
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 45e0a3cd11fa650f456f7382ffbbcd1c932b95eb..2beee8ebe94eeadd6fb1d5b119e3b474e1ab902a 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -11542,6 +11542,12 @@ neon_valid_immediate (rtx op, machine_mode mode, int inverse,
return 18;
}
+ /* The tricks done in the code below apply for little-endian vector layout.
+ For big-endian vectors only allow vectors of the form { a, a, a..., a }.
+ FIXME: Implement logic for big-endian vectors. */
+ if (BYTES_BIG_ENDIAN && vector && !const_vec_duplicate_p (op))
+ return -1;
+
/* Splat vector constant out into a byte vector. */
for (i = 0; i < n_elts; i++)
{
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][ARM] PR target/71270 fix neon_valid_immediate for big-endian
2017-01-06 11:55 [PATCH][ARM] PR target/71270 fix neon_valid_immediate for big-endian Kyrill Tkachov
@ 2017-01-18 9:48 ` Kyrill Tkachov
2017-01-20 14:34 ` Richard Earnshaw (lists)
1 sibling, 0 replies; 4+ messages in thread
From: Kyrill Tkachov @ 2017-01-18 9:48 UTC (permalink / raw)
To: GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw
Ping.
https://gcc.gnu.org/ml/gcc-patches/2017-01/msg00381.html
Thanks,
Kyrill
On 06/01/17 11:54, Kyrill Tkachov wrote:
> Hi all,
>
> In this wrong-code issue the RTL tries to load a const_vector:
> (const_vector:V8QI [
> (const_int 1 [0x1])
> (const_int 0 [0])
> (const_int 1 [0x1])
> (const_int 0 [0])
> (const_int 1 [0x1])
> (const_int 0 [0])
> (const_int 1 [0x1])
> (const_int 0 [0])
> ])
>
> into a NEON register. The logic for that is in neon_valid_immediate which does a number of tricks
> to decide what value and of what size to do a VMOV on to load the correct vector immediate into the register.
> It goes wrong on big-endian. On both big and little-endian this outputs:
> vmov.i16 d18, #0x1
>
> This is wrong on big-endian. The vector layout has to be such as if loaded from memory.
> I've tried various approaches of fixing neon_valid_immediate to generate the correct immediate but have been unsuccessful,
> resulting in regressions in various parts of the testsuite or making a big mess of the function.
>
> Given that armeb is not a target of major concern I believe the safest route at this stage is to only allow vector constants
> that will obviously work on big-endian, that is the ones that are just a single element duplicated in all lanes.
>
> This patch fixes the execution failures:
> FAIL: gfortran.dg/intrinsic_pack_1.f90
> FAIL: gfortran.dg/c_f_pointer_logical.f03
> FAIL: gcc.dg/torture/pr60183.c
> FAIL: gcc.dg/vect/pr51581-4.c
>
> on armeb-none-eabi.
>
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> 2017-01-06 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> PR target/71270
> * config/arm/arm.c (neon_valid_immediate): Reject vector constants
> in big-endian mode when they are not a single duplicated value.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][ARM] PR target/71270 fix neon_valid_immediate for big-endian
2017-01-06 11:55 [PATCH][ARM] PR target/71270 fix neon_valid_immediate for big-endian Kyrill Tkachov
2017-01-18 9:48 ` Kyrill Tkachov
@ 2017-01-20 14:34 ` Richard Earnshaw (lists)
2017-01-20 14:47 ` Kyrill Tkachov
1 sibling, 1 reply; 4+ messages in thread
From: Richard Earnshaw (lists) @ 2017-01-20 14:34 UTC (permalink / raw)
To: Kyrill Tkachov, GCC Patches; +Cc: Ramana Radhakrishnan
On 06/01/17 11:54, Kyrill Tkachov wrote:
> Hi all,
>
> In this wrong-code issue the RTL tries to load a const_vector:
> (const_vector:V8QI [
> (const_int 1 [0x1])
> (const_int 0 [0])
> (const_int 1 [0x1])
> (const_int 0 [0])
> (const_int 1 [0x1])
> (const_int 0 [0])
> (const_int 1 [0x1])
> (const_int 0 [0])
> ])
>
> into a NEON register. The logic for that is in neon_valid_immediate
> which does a number of tricks
> to decide what value and of what size to do a VMOV on to load the
> correct vector immediate into the register.
> It goes wrong on big-endian. On both big and little-endian this outputs:
> vmov.i16 d18, #0x1
>
> This is wrong on big-endian. The vector layout has to be such as if
> loaded from memory.
> I've tried various approaches of fixing neon_valid_immediate to generate
> the correct immediate but have been unsuccessful,
> resulting in regressions in various parts of the testsuite or making a
> big mess of the function.
>
> Given that armeb is not a target of major concern I believe the safest
> route at this stage is to only allow vector constants
> that will obviously work on big-endian, that is the ones that are just a
> single element duplicated in all lanes.
>
> This patch fixes the execution failures:
> FAIL: gfortran.dg/intrinsic_pack_1.f90
> FAIL: gfortran.dg/c_f_pointer_logical.f03
> FAIL: gcc.dg/torture/pr60183.c
> FAIL: gcc.dg/vect/pr51581-4.c
>
> on armeb-none-eabi.
>
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> 2017-01-06 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> PR target/71270
> * config/arm/arm.c (neon_valid_immediate): Reject vector constants
> in big-endian mode when they are not a single duplicated value.
>
Ok, but if you plan to close the PR above, please create a new
'enhancement' PR to fix the missed optimization.
R.
> armeb-vec.patch
>
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 45e0a3cd11fa650f456f7382ffbbcd1c932b95eb..2beee8ebe94eeadd6fb1d5b119e3b474e1ab902a 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -11542,6 +11542,12 @@ neon_valid_immediate (rtx op, machine_mode mode, int inverse,
> return 18;
> }
>
> + /* The tricks done in the code below apply for little-endian vector layout.
> + For big-endian vectors only allow vectors of the form { a, a, a..., a }.
> + FIXME: Implement logic for big-endian vectors. */
> + if (BYTES_BIG_ENDIAN && vector && !const_vec_duplicate_p (op))
> + return -1;
> +
> /* Splat vector constant out into a byte vector. */
> for (i = 0; i < n_elts; i++)
> {
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][ARM] PR target/71270 fix neon_valid_immediate for big-endian
2017-01-20 14:34 ` Richard Earnshaw (lists)
@ 2017-01-20 14:47 ` Kyrill Tkachov
0 siblings, 0 replies; 4+ messages in thread
From: Kyrill Tkachov @ 2017-01-20 14:47 UTC (permalink / raw)
To: Richard Earnshaw (lists), GCC Patches; +Cc: Ramana Radhakrishnan
On 20/01/17 14:33, Richard Earnshaw (lists) wrote:
> On 06/01/17 11:54, Kyrill Tkachov wrote:
>> Hi all,
>>
>> In this wrong-code issue the RTL tries to load a const_vector:
>> (const_vector:V8QI [
>> (const_int 1 [0x1])
>> (const_int 0 [0])
>> (const_int 1 [0x1])
>> (const_int 0 [0])
>> (const_int 1 [0x1])
>> (const_int 0 [0])
>> (const_int 1 [0x1])
>> (const_int 0 [0])
>> ])
>>
>> into a NEON register. The logic for that is in neon_valid_immediate
>> which does a number of tricks
>> to decide what value and of what size to do a VMOV on to load the
>> correct vector immediate into the register.
>> It goes wrong on big-endian. On both big and little-endian this outputs:
>> vmov.i16 d18, #0x1
>>
>> This is wrong on big-endian. The vector layout has to be such as if
>> loaded from memory.
>> I've tried various approaches of fixing neon_valid_immediate to generate
>> the correct immediate but have been unsuccessful,
>> resulting in regressions in various parts of the testsuite or making a
>> big mess of the function.
>>
>> Given that armeb is not a target of major concern I believe the safest
>> route at this stage is to only allow vector constants
>> that will obviously work on big-endian, that is the ones that are just a
>> single element duplicated in all lanes.
>>
>> This patch fixes the execution failures:
>> FAIL: gfortran.dg/intrinsic_pack_1.f90
>> FAIL: gfortran.dg/c_f_pointer_logical.f03
>> FAIL: gcc.dg/torture/pr60183.c
>> FAIL: gcc.dg/vect/pr51581-4.c
>>
>> on armeb-none-eabi.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> 2017-01-06 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>
>> PR target/71270
>> * config/arm/arm.c (neon_valid_immediate): Reject vector constants
>> in big-endian mode when they are not a single duplicated value.
>>
> Ok, but if you plan to close the PR above, please create a new
> 'enhancement' PR to fix the missed optimization.
Thanks, I've committed it as r244716 and opened PR 79166 to track
the missed optimisation.
Kyrill
> R.
>
>> armeb-vec.patch
>>
>>
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index 45e0a3cd11fa650f456f7382ffbbcd1c932b95eb..2beee8ebe94eeadd6fb1d5b119e3b474e1ab902a 100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -11542,6 +11542,12 @@ neon_valid_immediate (rtx op, machine_mode mode, int inverse,
>> return 18;
>> }
>>
>> + /* The tricks done in the code below apply for little-endian vector layout.
>> + For big-endian vectors only allow vectors of the form { a, a, a..., a }.
>> + FIXME: Implement logic for big-endian vectors. */
>> + if (BYTES_BIG_ENDIAN && vector && !const_vec_duplicate_p (op))
>> + return -1;
>> +
>> /* Splat vector constant out into a byte vector. */
>> for (i = 0; i < n_elts; i++)
>> {
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-20 14:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-06 11:55 [PATCH][ARM] PR target/71270 fix neon_valid_immediate for big-endian Kyrill Tkachov
2017-01-18 9:48 ` Kyrill Tkachov
2017-01-20 14:34 ` Richard Earnshaw (lists)
2017-01-20 14:47 ` Kyrill Tkachov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).