Tamar Christina <tamar.christ...@arm.com> writes:
> Hi,
>
>> > --- a/gcc/config/aarch64/aarch64-simd.md
>> > +++ b/gcc/config/aarch64/aarch64-simd.md
>> > @@ -4259,7 +4259,7 @@ (define_insn
>> "*aarch64_get_lane_zero_extend<GPI:mode><VDQV_L:mode>"
>> > ;; Extracting lane zero is split into a simple move when it is
>> > between SIMD ;; registers or a store.
>> > (define_insn_and_split "aarch64_get_lane<mode>"
>> > - [(set (match_operand:<VEL> 0 "aarch64_simd_nonimmediate_operand"
>> > "=?r, w, Utv")
>> > + [(set (match_operand:<VEL> 0 "aarch64_simd_nonimmediate_operand"
>> > + "=r, w, Utv")
>> > (vec_select:<VEL>
>> > (match_operand:VALL_F16_FULL 1 "register_operand" "w, w, w")
>> > (parallel [(match_operand:SI 2 "immediate_operand" "i, i, i")])))]
>>
>> Which testcase does this help with? It didn't look like the new tests do any
>> vector stuff.
>>
>
> Right, sorry about that, splitting up my patches resulted in this sneaking in
> from a different series.
> Moved now.
>
>> > -(define_insn "*<ANY_EXTEND:optab><GPI:mode>_ashl<SHORT:mode>"
>> > +(define_insn "*<ANY_EXTEND:optab><GPI:mode>_ashl<ALLX:mode>"
>> > [(set (match_operand:GPI 0 "register_operand" "=r")
>> > (ANY_EXTEND:GPI
>> > - (ashift:SHORT (match_operand:SHORT 1 "register_operand" "r")
>> > + (ashift:ALLX (match_operand:ALLX 1 "register_operand" "r")
>> > (match_operand 2 "const_int_operand" "n"))))]
>> > - "UINTVAL (operands[2]) < GET_MODE_BITSIZE (<SHORT:MODE>mode)"
>> > + "UINTVAL (operands[2]) < GET_MODE_BITSIZE (<ALLX:MODE>mode)"
>>
>> It'd be better to avoid even defining si<-si or si<-di "extensions"
>> (even though nothing should try to match them), so how about adding:
>>
>> <GPI:sizen> > <ALLX:sizen> &&
>>
>> or similar to the beginning of the condition? The conditions for the invalid
>> combos will then be provably false at compile time and the patterns will be
>> compiled out.
>>
>
> Done.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.md
> (*<ANY_EXTEND:optab><GPI:mode>_ashl<SHORT:mode>): Renamed to...
> (*<ANY_EXTEND:optab><GPI:mode>_ashl<ALLX:mode>): ...this.
> (*zero_extend<GPI:mode>_lshr<SHORT:mode>): Renamed to...
> (*zero_extend<GPI:mode>_lshr<ALLX:mode>): ...this.
> (*extend<GPI:mode>_ashr<SHORT:mode>): Rename to...
> (*extend<GPI:mode>_ashr<ALLX:mode>): ...this.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/bitmove_1.c: New test.
> * gcc.target/aarch64/bitmove_2.c: New test.
>
> --- inline copy of patch ---
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index
> d7684c93fba5b717d568e1a4fd712bde55c7c72e..d230bbb833f97813c8371aa07b587bd8b0292cee
> 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -5711,40 +5711,43 @@ (define_insn "*extrsi5_insn_di"
> [(set_attr "type" "rotate_imm")]
> )
>
> -(define_insn "*<ANY_EXTEND:optab><GPI:mode>_ashl<SHORT:mode>"
> +(define_insn "*<ANY_EXTEND:optab><GPI:mode>_ashl<ALLX:mode>"
> [(set (match_operand:GPI 0 "register_operand" "=r")
> (ANY_EXTEND:GPI
> - (ashift:SHORT (match_operand:SHORT 1 "register_operand" "r")
> + (ashift:ALLX (match_operand:ALLX 1 "register_operand" "r")
> (match_operand 2 "const_int_operand" "n"))))]
> - "UINTVAL (operands[2]) < GET_MODE_BITSIZE (<SHORT:MODE>mode)"
> + "<GPI:sizen> > <ALLX:sizen>
> + && UINTVAL (operands[2]) < GET_MODE_BITSIZE (<ALLX:MODE>mode)"
> {
> - operands[3] = GEN_INT (<SHORT:sizen> - UINTVAL (operands[2]));
> + operands[3] = GEN_INT (<ALLX:sizen> - UINTVAL (operands[2]));
> return "<su>bfiz\t%<GPI:w>0, %<GPI:w>1, %2, %3";
> }
> [(set_attr "type" "bfx")]
> )
>
> -(define_insn "*zero_extend<GPI:mode>_lshr<SHORT:mode>"
> +(define_insn "*zero_extend<GPI:mode>_lshr<ALLX:mode>"
> [(set (match_operand:GPI 0 "register_operand" "=r")
> (zero_extend:GPI
> - (lshiftrt:SHORT (match_operand:SHORT 1 "register_operand" "r")
> - (match_operand 2 "const_int_operand" "n"))))]
> - "UINTVAL (operands[2]) < GET_MODE_BITSIZE (<SHORT:MODE>mode)"
> + (lshiftrt:ALLX (match_operand:ALLX 1 "register_operand" "r")
> + (match_operand 2 "const_int_operand" "n"))))]
> + "<GPI:sizen> > <ALLX:sizen>
> + && UINTVAL (operands[2]) < GET_MODE_BITSIZE (<ALLX:MODE>mode)"
> {
> - operands[3] = GEN_INT (<SHORT:sizen> - UINTVAL (operands[2]));
> + operands[3] = GEN_INT (<ALLX:sizen> - UINTVAL (operands[2]));
> return "ubfx\t%<GPI:w>0, %<GPI:w>1, %2, %3";
> }
> [(set_attr "type" "bfx")]
> )
>
> -(define_insn "*extend<GPI:mode>_ashr<SHORT:mode>"
> +(define_insn "*extend<GPI:mode>_ashr<ALLX:mode>"
> [(set (match_operand:GPI 0 "register_operand" "=r")
> (sign_extend:GPI
> - (ashiftrt:SHORT (match_operand:SHORT 1 "register_operand" "r")
> - (match_operand 2 "const_int_operand" "n"))))]
> - "UINTVAL (operands[2]) < GET_MODE_BITSIZE (<SHORT:MODE>mode)"
> + (ashiftrt:ALLX (match_operand:ALLX 1 "register_operand" "r")
> + (match_operand 2 "const_int_operand" "n"))))]
> + "<GPI:sizen> > <ALLX:sizen>
> + && UINTVAL (operands[2]) < GET_MODE_BITSIZE (<ALLX:MODE>mode)"
> {
> - operands[3] = GEN_INT (<SHORT:sizen> - UINTVAL (operands[2]));
> + operands[3] = GEN_INT (<ALLX:sizen> - UINTVAL (operands[2]));
> return "sbfx\\t%<GPI:w>0, %<GPI:w>1, %2, %3";
> }
> [(set_attr "type" "bfx")]
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitmove_1.c
> b/gcc/testsuite/gcc.target/aarch64/bitmove_1.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..5ea4265f55213d7e7e5193a3a3681c9350867b50
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/bitmove_1.c
> @@ -0,0 +1,76 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O3 -std=c99" } */
> +/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
> +
> +#include <stdint.h>
> +
> +/*
> +** sfoo6:
> +** asr x0, x0, 16
> +** ret
> +*/
> +int64_t sfoo6 (int32_t x)
> +{
> + return x >> 16;
> +}
> +
> +/*
> +** ufoo6:
> +** lsr w0, w0, 30
> +** ret
> +*/
> +uint64_t ufoo6 (uint32_t x)
> +{
> + return x >> 30;
> +}
> +
> +/*
> +** ufoo6s:
> +** ubfx w0, w0, 7, 9
> +** ret
> +*/
> +uint32_t ufoo6s (uint16_t x)
> +{
> + return x >> 7;
> +}
> +
> +/*
> +** ufoo6h:
> +** ubfx w0, w0, 4, 4
> +** ret
> +*/
> +uint16_t ufoo6h (uint8_t x)
> +{
> + return x >> 4;
> +}
> +
> +/*
> +** sfoo62:
> +** sbfx x0, x0, 10, 22
> +** ret
> +*/
> +int64_t sfoo62 (int32_t x)
> +{
> + return x >> 10;
> +}
> +
> +/*
> +** ufoo62:
> +** lsr w0, w0, 10
> +** ret
> +*/
> +uint64_t ufoo62 (uint32_t x)
> +{
> + return x >> 10;
> +}
> +
> +/*
> +** sfoo63:
> +** sbfx x0, x0, 10, 22
> +** ret
> +*/
> +int64_t sfoo63 (int32_t x)
> +{
> + return x >> 10;
> +}
This is the same as sfoo62, not sure if that's intentional.
> +
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitmove_2.c
> b/gcc/testsuite/gcc.target/aarch64/bitmove_2.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..329600cb3dbecf4cdfed994f6cfdf98ab77e8a01
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/bitmove_2.c
> @@ -0,0 +1,76 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O3 -std=c99" } */
> +/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
> +
> +#include <stdint.h>
> +
> +/*
> +** sfoo6:
> +** sbfiz x0, x0, 16, 16
> +** ret
> +*/
> +int64_t sfoo6 (int32_t x)
> +{
> + return x << 16;
> +}
> +
> +/*
> +** ufoo6:
> +** lsl w0, w0, 30
> +** ret
> +*/
> +uint64_t ufoo6 (uint32_t x)
> +{
> + return x << 30;
> +}
> +
> +/*
> +** ufoo6s:
> +** ubfiz w0, w0, 7, 16
> +** ret
> +*/
> +uint32_t ufoo6s (uint16_t x)
> +{
> + return x << 7;
> +}
> +
> +/*
> +** ufoo6h:
> +** ...
> +** ubfiz w0, w0, 4, 12
> +** ret
> +*/
> +uint16_t ufoo6h (uint8_t x)
> +{
> + return x << 4;
> +}
This looks odd without the ... filled in. It raises the question why
the width is 12 bits when the original type was only 8.
> +
> +/*
> +** sfoo62:
> +** sbfiz x0, x0, 10, 22
> +** ret
> +*/
> +int64_t sfoo62 (int32_t x)
> +{
> + return x << 10;
> +}
> +
> +/*
> +** ufoo62:
> +** lsl w0, w0, 10
> +** ret
> +*/
> +uint64_t ufoo62 (uint32_t x)
> +{
> + return x << 10;
> +}
> +
> +/*
> +** sfoo63:
> +** sbfiz x0, x0, 10, 22
> +** ret
> +*/
> +int64_t sfoo63 (int32_t x)
> +{
> + return x << 10;
> +}
Similarly a dup of sfoo62.
OK with those things fixed, thanks.
Richard
