Hi Elen
Thank you for doing this. You will need a maintainer's approval but I
would like to add a couple of comments. Please find them inline.
On 08/05/2019 14:36, Elen Kalda wrote:
> Hi,
>
> This patch adds a pattern to support the FADDP (scalar) instruction.
>
> Before the patch, the C code
>
> typedef double v2df __attribute__((vector_size (16)));
>
> double
> foo (v2df x)
> {
> return x[1] + x[0];
> }
>
> generated:
> foo:
> dup d1, v0.d[0]
> dup d0, v0.d[1]
> fadd d0, d1, d0
> ret
>
> After patch:
> foo:
> faddp d0, v0.2d
> ret
>
>
> Bootstrapped and done regression tests on aarch64-none-linux-gnu -
> no issues found.
>
> Best wishes,
> Elen
>
>
> gcc/ChangeLog:
>
> 2019-04-24 Elen Kalda <elen.ka...@arm.com>
>
> * config/aarch64/aarch64-simd.md (*aarch64_faddp<mode>): New.
>
> gcc/testsuite/ChangeLog:
>
> 2019-04-24 Elen Kalda <elen.ka...@arm.com>
>
> * gcc.target/aarch64/simd/scalar_faddp.c: New test.
>
> diff --git a/gcc/config/aarch64/aarch64-simd.md
b/gcc/config/aarch64/aarch64-simd.md
> index
e3852c5d182b70978d7603225fce55c0b8ee2894..89fedc6cb3f0c6eb74c6f8d0b21cedb5ae20a095
100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -2372,6 +2372,21 @@
> [(set_attr "type" "neon_fp_reduc_add_<stype><q>")]
> )
>
> +(define_insn "*aarch64_faddp<mode>"
> + [(set (match_operand:<VEL> 0 "register_operand" "=w")
> + (plus:<VEL>
> + (vec_select:<VEL> (match_operand:VHSDF 1 "register_operand" "w")
I do not think the VHSDF mode should be used here. I believe you may
have taken this from the vector form of this instruction but that seems
to be different than the scalar one. Someone with more floating point
instruction experience can chime in here.
> + (parallel[(match_operand 2 "const_int_operand" "n")]))
> + (vec_select:<VEL> (match_dup:VHSDF 1)
> + (parallel[(match_operand 3 "const_int_operand" "n")]))))]
> + "TARGET_SIMD
> + && ((INTVAL (operands[2]) == 0 && INTVAL (operands[3]) == 1)
Just some minor indentation issue. The && should be below T
> + || (INTVAL (operands[2]) == 1 && INTVAL (operands[3]) == 0))"
Likewise this should be below the second opening brace '('
...
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/simd/scalar_faddp.c
> @@ -0,0 +1,31 @@
> +/* { dg-do assemble } */
This can be dg-do compile since you only want an assembly file
> +/* { dg-require-effective-target arm_v8_2a_fp16_scalar_ok } */
> +/* { dg-add-options arm_v8_2a_fp16_scalar } */
> +/* { dg-additional-options "-save-temps -O1" } */
The --save-temps can then be removed as the dg-do compile will produce
the .s file for you
> +/* { dg-final { scan-assembler-not "dup" } } */
...
Thanks
Sudi
