AArch64 Extend umov and sbfx patterns.

  Hi All,

Our zero and sign extend and extract patterns are currently very limited and
only work for the original register size of the instructions. i.e. limited by
GPI patterns.  However these instructions extract bits and extend.  This means
that any register size can be used as an input as long as the extraction makes
logical sense.

The majority of the attached testcases fail currently to optimize.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	* config/aarch64/aarch64-simd.md (aarch64_get_lane<mode>): Drop reload
	penalty.
	* 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>_<optab><ALLX:mode>): ...this.
	(*extend<GPI:mode>_ashr<SHORT:mode>): Rename to...
	(*extend<GPI:mode>_<optab><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-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 8bcc9e76b1cad4a2591fb176175db72d7a190d57..23909c62638b49722568da4555b33c71fd21337e 100644




--
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 8bcc9e76b1cad4a2591fb176175db72d7a190d57..23909c62638b49722568da4555b33c71fd21337e 100644
--- 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")])))]
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 85b400489cb382a01b0c469eff2b600a93805e31..3116feda4fe54e2a21dc3f990b6976d216874260 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -5629,13 +5629,13 @@ (define_insn "*<optab>si3_insn2_uxtw"
 )
 
 (define_insn "*<optab><mode>3_insn"
-  [(set (match_operand:SHORT 0 "register_operand" "=r")
-	(ASHIFT:SHORT (match_operand:SHORT 1 "register_operand" "r")
+  [(set (match_operand:ALLI 0 "register_operand" "=r")
+	(ASHIFT:ALLI (match_operand:ALLI 1 "register_operand" "r")
 		      (match_operand 2 "const_int_operand" "n")))]
   "UINTVAL (operands[2]) < GET_MODE_BITSIZE (<MODE>mode)"
 {
   operands[3] = GEN_INT (<sizen> - UINTVAL (operands[2]));
-  return "<bfshift>\t%w0, %w1, %2, %3";
+  return "<bfshift>\t%<w>0, %<w>1, %2, %3";
 }
   [(set_attr "type" "bfx")]
 )
@@ -5710,40 +5710,40 @@ (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)"
+  "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>_<optab><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_ONLY:ALLX (match_operand:ALLX 1 "register_operand" "r")
+			     (match_operand 2 "const_int_operand" "n"))))]
+  "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>_<optab><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_ONLY:ALLX (match_operand:ALLX 1 "register_operand" "r")
+			     (match_operand 2 "const_int_operand" "n"))))]
+  "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..8b0aa8af49cd070928bacc4995a321c7bfde58a6
--- /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:
+** 	asr	x0, x0, 10
+** 	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:
+** 	asr	x0, x0, 10
+** 	ret
+*/
+int64_t sfoo63 (int32_t x)
+{
+  return x >> 10;
+}
+
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..54b3071a3b4e2001f83337837e712c381683d23a
--- /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:
+** 	uxtb	w0, w0
+** 	ubfiz	w0, w0, 4, 12
+** 	ret
+*/
+uint16_t ufoo6h (uint8_t x)
+{
+  return x << 4;
+}
+
+/*
+** 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;
+}
  

Tamar Christina <tamar.christina@arm.com> writes:
> Hi All,
>
> Our zero and sign extend and extract patterns are currently very limited and
> only work for the original register size of the instructions. i.e. limited by
> GPI patterns.  However these instructions extract bits and extend.  This means
> that any register size can be used as an input as long as the extraction makes
> logical sense.
>
> The majority of the attached testcases fail currently to optimize.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> 	* config/aarch64/aarch64-simd.md (aarch64_get_lane<mode>): Drop reload
> 	penalty.
> 	* 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>_<optab><ALLX:mode>): ...this.
> 	(*extend<GPI:mode>_ashr<SHORT:mode>): Rename to...
> 	(*extend<GPI:mode>_<optab><ALLX:mode>): ...this.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/aarch64/bitmove_1.c: New test.
> 	* gcc.target/aarch64/bitmove_2.c: New test.

Looks like a nice change, but some comments below.

>
> --- inline copy of patch -- 
> diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
> index 8bcc9e76b1cad4a2591fb176175db72d7a190d57..23909c62638b49722568da4555b33c71fd21337e 100644
> --- 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.

> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 85b400489cb382a01b0c469eff2b600a93805e31..3116feda4fe54e2a21dc3f990b6976d216874260 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -5629,13 +5629,13 @@ (define_insn "*<optab>si3_insn2_uxtw"
>  )
>  
>  (define_insn "*<optab><mode>3_insn"
> -  [(set (match_operand:SHORT 0 "register_operand" "=r")
> -	(ASHIFT:SHORT (match_operand:SHORT 1 "register_operand" "r")
> +  [(set (match_operand:ALLI 0 "register_operand" "=r")
> +	(ASHIFT:ALLI (match_operand:ALLI 1 "register_operand" "r")
>  		      (match_operand 2 "const_int_operand" "n")))]
>    "UINTVAL (operands[2]) < GET_MODE_BITSIZE (<MODE>mode)"
>  {
>    operands[3] = GEN_INT (<sizen> - UINTVAL (operands[2]));
> -  return "<bfshift>\t%w0, %w1, %2, %3";
> +  return "<bfshift>\t%<w>0, %<w>1, %2, %3";
>  }
>    [(set_attr "type" "bfx")]
>  )

Similar question here I guess.  There's a separate pattern for SI and DI
shifts, so I wouldn't have expected this to be necessary.

> @@ -5710,40 +5710,40 @@ (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)"
> +  "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.

Same comment for the others.

>  {
> -  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>_<optab><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_ONLY:ALLX (match_operand:ALLX 1 "register_operand" "r")
> +			     (match_operand 2 "const_int_operand" "n"))))]
> +  "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")]
>  )

I think it'd better to stick to the hard-coded lshiftrt, since nothing
in the asm template is parameterised by the operation.  Using single-item
iterators is only really useful for "@" patterns.

Likewise for the ashiftrt pattern.

Thanks,
Richard

> -(define_insn "*extend<GPI:mode>_ashr<SHORT:mode>"
> +(define_insn "*extend<GPI:mode>_<optab><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_ONLY:ALLX (match_operand:ALLX 1 "register_operand" "r")
> +			     (match_operand 2 "const_int_operand" "n"))))]
> +  "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..8b0aa8af49cd070928bacc4995a321c7bfde58a6
> --- /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:
> +** 	asr	x0, x0, 10
> +** 	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:
> +** 	asr	x0, x0, 10
> +** 	ret
> +*/
> +int64_t sfoo63 (int32_t x)
> +{
> +  return x >> 10;
> +}
> +
> 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..54b3071a3b4e2001f83337837e712c381683d23a
> --- /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:
> +** 	uxtb	w0, w0
> +** 	ubfiz	w0, w0, 4, 12
> +** 	ret
> +*/
> +uint16_t ufoo6h (uint8_t x)
> +{
> +  return x << 4;
> +}
> +
> +/*
> +** 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;
> +}
  

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;
+}
+
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;
+}
+
+/*
+** 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;
+}
  

Tamar Christina <Tamar.Christina@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