On 28/06/2021 10:40, Kyrylo Tkachov via Gcc-patches wrote:
-----Original Message-----
From: Prathamesh Kulkarni <prathamesh.kulka...@linaro.org>
Sent: 28 June 2021 09:38
To: Kyrylo Tkachov <kyrylo.tkac...@arm.com>
Cc: Christophe Lyon <christophe.l...@linaro.org>; gcc Patches <gcc-
patc...@gcc.gnu.org>
Subject: Re: [ARM] PR98435: Missed optimization in expanding vector
constructor
On Thu, 24 Jun 2021 at 22:01, Kyrylo Tkachov <kyrylo.tkac...@arm.com>
wrote:
-----Original Message-----
From: Prathamesh Kulkarni <prathamesh.kulka...@linaro.org>
Sent: 14 June 2021 09:02
To: Christophe Lyon <christophe.l...@linaro.org>
Cc: gcc Patches <gcc-patches@gcc.gnu.org>; Kyrylo Tkachov
<kyrylo.tkac...@arm.com>
Subject: Re: [ARM] PR98435: Missed optimization in expanding vector
constructor
On Wed, 9 Jun 2021 at 15:58, Prathamesh Kulkarni
<prathamesh.kulka...@linaro.org> wrote:
On Fri, 4 Jun 2021 at 13:15, Christophe Lyon
<christophe.l...@linaro.org>
wrote:
On Fri, 4 Jun 2021 at 09:27, Prathamesh Kulkarni via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
Hi,
As mentioned in PR, for the following test-case:
#include <arm_neon.h>
bfloat16x4_t f1 (bfloat16_t a)
{
return vdup_n_bf16 (a);
}
bfloat16x4_t f2 (bfloat16_t a)
{
return (bfloat16x4_t) {a, a, a, a};
}
Compiling with arm-linux-gnueabi -O3 -mfpu=neon -mfloat-
abi=softfp
-march=armv8.2-a+bf16+fp16 results in f2 not being vectorized:
f1:
vdup.16 d16, r0
vmov r0, r1, d16 @ v4bf
bx lr
f2:
mov r3, r0 @ __bf16
adr r1, .L4
ldrd r0, [r1]
mov r2, r3 @ __bf16
mov ip, r3 @ __bf16
bfi r1, r2, #0, #16
bfi r0, ip, #0, #16
bfi r1, r3, #16, #16
bfi r0, r2, #16, #16
bx lr
This seems to happen because vec_init pattern in neon.md has VDQ
mode
iterator, which doesn't include V4BF. In attached patch, I changed
mode
to VDQX which seems to work for the test-case, and the compiler
now
generates:
f2:
vdup.16 d16, r0
vmov r0, r1, d16 @ v4bf
bx lr
However, the pattern is also gated on TARGET_HAVE_MVE and I am
not
sure if either VDQ or VDQX are correct modes for MVE since MVE
has
only 128-bit vectors ?
I think patterns common to both Neon and MVE should be moved to
vec-common.md, I don't know why such patterns were left in
neon.md.
Since we end up calling neon_expand_vector_init for both NEON and
MVE,
I am not sure if we should separate the pattern ?
Would it make sense to FAIL if the mode size isn't 16 bytes for MVE as
in attached patch so
it will call neon_expand_vector_init only for 128-bit vectors ?
Altho hard-coding 16 in the pattern doesn't seem a good idea to me
either.
ping https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572342.html
(attaching patch as text).
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -459,10 +459,12 @@
)
(define_expand "vec_init<mode><V_elem_l>"
- [(match_operand:VDQ 0 "s_register_operand")
+ [(match_operand:VDQX 0 "s_register_operand")
(match_operand 1 "" "")]
"TARGET_NEON || TARGET_HAVE_MVE"
{
+ if (TARGET_HAVE_MVE && GET_MODE_SIZE (GET_MODE
(operands[0])) != 16)
+ FAIL;
neon_expand_vector_init (operands[0], operands[1]);
DONE;
})
I think we should move this to vec-common.md like Christophe said.
Perhaps rather than making it FAIL for non-16 MVE sizes we just disable it in
the expander condition?
"TARGET_NEON || (TARGET_HAVE_MVE && GET_MODE_SIZE (<
VDQ>mode) != 16)"
Is it OK to use <MODE>mode ? Because using <VDQ>mode resulted in lot
of build errors.
Also, I think the comparison should be inverted, ie, GET_MODE_SIZE
(<MODE>mode) == 16 since
we want to make the pattern pass if target is MVE and vector size is 16 bytes ?
Do these changes in attached patch look OK ?
Yes, you're right.
Can't this be ARM_HAVE_<MODE>_ARITH like in most expanders in vec-common.md?
(maybe with a && !TARGET_REALLY_IWMMXT if needed)
Christophe
Ok.
Thanks,
Kyrill
Thanks,
Prathamesh
Thanks,
Kyrill
Thanks,
Prathamesh
Thanks,
Prathamesh
That being said, I suggest you look at other similar patterns in
vec-common.md, most of which are gated on
ARM_HAVE_<MODE>_ARITH
and possibly beware of issues with iwmmxt :-)
Christophe
Thanks,
Prathamesh
