Re: [PATCH][AArch64] Remove constraint strings from define_expand constructs in the back end

[Richard Sandiford]

Dennis Zhang  writes:
> Hi Richard, Thanks for the tips.
>
> The special exceptions according to TARGET_SECONDARY_RELOAD hook are 
> revised. Some related patterns still need constraints in order to work 
> in an expected way in the TARGET_SECONDARY_RELOAD function.
>
> The updated patch is tested for targets: aarch64_be-linux-gnu,
> aarch64_be-none-linux-gnu, aarch64-linux-gnu, and 
> aarch64-none-linux-gnu. It survives in testsuite regression.
>
> gcc/ChangeLog:
>
> 2019-07-03  Dennis Zhang  
>
>   * config/aarch64/aarch64.md: Remove redundant constraints from
>   define_expand but keep some patterns untouched if they are
>   specially selected by TARGET_SECONDARY_RELOAD hook.
>   * config/aarch64/aarch64-sve.md: Likewise.
>   * config/aarch64/atomics.md: Remove redundant constraints from
>   define_expand.
>   * config/aarch64/aarch64-simd.md: Likewise.

Thanks, applied as r273021.

Richard

Re: [PATCH][AArch64] Remove constraint strings from define_expand constructs in the back end

[Dennis Zhang]

Hi Richard, Thanks for the tips.

The special exceptions according to TARGET_SECONDARY_RELOAD hook are 
revised. Some related patterns still need constraints in order to work 
in an expected way in the TARGET_SECONDARY_RELOAD function.

The updated patch is tested for targets: aarch64_be-linux-gnu,
aarch64_be-none-linux-gnu, aarch64-linux-gnu, and 
aarch64-none-linux-gnu. It survives in testsuite regression.

gcc/ChangeLog:

2019-07-03  Dennis Zhang  

* config/aarch64/aarch64.md: Remove redundant constraints from
define_expand but keep some patterns untouched if they are
specially selected by TARGET_SECONDARY_RELOAD hook.
* config/aarch64/aarch64-sve.md: Likewise.
* config/aarch64/atomics.md: Remove redundant constraints from
define_expand.
* config/aarch64/aarch64-simd.md: Likewise.

On 7/2/19 8:05 AM, Richard Sandiford wrote:
> James Greenhalgh  writes:
>> On Mon, Jun 24, 2019 at 04:33:40PM +0100, Dennis Zhang wrote:
>>> Hi,
>>>
>>> A number of AArch64 define_expand patterns have specified constraints
>>> for their operands. But the constraint strings are ignored at expand
>>> time and are therefore redundant/useless. We now avoid specifying
>>> constraints in new define_expands, but we should clean up the existing
>>> define_expand definitions.
>>>
>>> For example, the constraint "=w" is removed in the following case:
>>> (define_expand "sqrt2"
>>> [(set (match_operand:GPF_F16 0 "register_operand" "=w")
>>> The "" marks with an empty constraint in define_expand are removed as well.
>>>
>>> The patch is tested with the build configuration of
>>> --target=aarch64-none-linux-gnu, and it passes gcc/testsuite.
>>
>> This is OK for trunk.
> 
> My fault, sorry, but... Kyrill pointed out when the corresponding arm
> patch was posted that it removes constraints from reload expanders that
> actually need them.  This patch has the same problem and so shouldn't
> go in as-is.
> 
> I'd thought at the time that Kyrill's comment applied to both patches,
> but I see now that it really was specific to arm.
> 
> Thanks,
> Richard
> 
>>
>> Thanks,
>> James
>>
>>> gcc/ChangeLog:
>>>
>>> 2019-06-21  Dennis Zhang  
>>>
>>> * config/aarch64/aarch64-simd.md: Remove redundant constraints
>>> from define_expand.
>>> * config/aarch64/aarch64-sve.md: Likewise.
>>> * config/aarch64/aarch64.md: Likewise.
>>> * config/aarch64/atomics.md: Likewise.
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index df8bf1d9778..837242c7e56 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -19,8 +19,8 @@
 ;; .
 
 (define_expand "mov"
-  [(set (match_operand:VALL_F16 0 "nonimmediate_operand" "")
-	(match_operand:VALL_F16 1 "general_operand" ""))]
+  [(set (match_operand:VALL_F16 0 "nonimmediate_operand")
+	(match_operand:VALL_F16 1 "general_operand"))]
   "TARGET_SIMD"
   "
   /* Force the operand into a register if it is not an
@@ -39,8 +39,8 @@
 )
 
 (define_expand "movmisalign"
-  [(set (match_operand:VALL 0 "nonimmediate_operand" "")
-(match_operand:VALL 1 "general_operand" ""))]
+  [(set (match_operand:VALL 0 "nonimmediate_operand")
+(match_operand:VALL 1 "general_operand"))]
   "TARGET_SIMD"
 {
   /* This pattern is not permitted to fail during expansion: if both arguments
@@ -652,8 +652,8 @@
   [(set_attr "type" "neon_fp_rsqrts_")])
 
 (define_expand "rsqrt2"
-  [(set (match_operand:VALLF 0 "register_operand" "=w")
-	(unspec:VALLF [(match_operand:VALLF 1 "register_operand" "w")]
+  [(set (match_operand:VALLF 0 "register_operand")
+	(unspec:VALLF [(match_operand:VALLF 1 "register_operand")]
 		 UNSPEC_RSQRT))]
   "TARGET_SIMD"
 {
@@ -1025,9 +1025,9 @@
 )
 
 (define_expand "ashl3"
-  [(match_operand:VDQ_I 0 "register_operand" "")
-   (match_operand:VDQ_I 1 "register_operand" "")
-   (match_operand:SI  2 "general_operand" "")]
+  [(match_operand:VDQ_I 0 "register_operand")
+   (match_operand:VDQ_I 1 "register_operand")
+   (match_operand:SI  2 "general_operand")]
  "TARGET_SIMD"
 {
   int bit_width = GET_MODE_UNIT_SIZE (mode) * BITS_PER_UNIT;
@@ -1072,9 +1072,9 @@
 )
 
 (define_expand "lshr3"
-  [(match_operand:VDQ_I 0 "register_operand" "")
-   (match_operand:VDQ_I 1 "register_operand" "")
-   (match_operand:SI  2 "general_operand" "")]
+  [(match_operand:VDQ_I 0 "register_operand")
+   (match_operand:VDQ_I 1 "register_operand")
+   (match_operand:SI  2 "general_operand")]
  "TARGET_SIMD"
 {
   int bit_width = GET_MODE_UNIT_SIZE (mode) * BITS_PER_UNIT;
@@ -1119,9 +1119,9 @@
 )
 
 (define_expand "ashr3"
-  [(match_operand:VDQ_I 0 "register_operand" "")
-   (match_operand:VDQ_I 1 "register_operand" "")
-   (match_operand:SI  2 "general_operand" "")]
+  [(match_operand:VDQ_I 0 "register_operand")
+   (match_operand:VDQ_I 1 "register_operand")
+   (match_operand:SI  2 "general_operand")]
  "TARGET_SIMD"
 {
   int bit_width = 

Re: [PATCH][AArch64] Remove constraint strings from define_expand constructs in the back end

[Richard Sandiford]

James Greenhalgh  writes:
> On Mon, Jun 24, 2019 at 04:33:40PM +0100, Dennis Zhang wrote:
>> Hi,
>> 
>> A number of AArch64 define_expand patterns have specified constraints 
>> for their operands. But the constraint strings are ignored at expand 
>> time and are therefore redundant/useless. We now avoid specifying 
>> constraints in new define_expands, but we should clean up the existing 
>> define_expand definitions.
>> 
>> For example, the constraint "=w" is removed in the following case:
>> (define_expand "sqrt2"
>>[(set (match_operand:GPF_F16 0 "register_operand" "=w")
>> The "" marks with an empty constraint in define_expand are removed as well.
>> 
>> The patch is tested with the build configuration of 
>> --target=aarch64-none-linux-gnu, and it passes gcc/testsuite.
>
> This is OK for trunk.

My fault, sorry, but... Kyrill pointed out when the corresponding arm
patch was posted that it removes constraints from reload expanders that
actually need them.  This patch has the same problem and so shouldn't
go in as-is.

I'd thought at the time that Kyrill's comment applied to both patches,
but I see now that it really was specific to arm.

Thanks,
Richard

>
> Thanks,
> James
>
>> gcc/ChangeLog:
>> 
>> 2019-06-21  Dennis Zhang  
>> 
>>  * config/aarch64/aarch64-simd.md: Remove redundant constraints
>>  from define_expand.
>>  * config/aarch64/aarch64-sve.md: Likewise.
>>  * config/aarch64/aarch64.md: Likewise.
>>  * config/aarch64/atomics.md: Likewise.

Re: [PATCH][AArch64] Remove constraint strings from define_expand constructs in the back end

[James Greenhalgh]

On Mon, Jun 24, 2019 at 04:33:40PM +0100, Dennis Zhang wrote:
> Hi,
> 
> A number of AArch64 define_expand patterns have specified constraints 
> for their operands. But the constraint strings are ignored at expand 
> time and are therefore redundant/useless. We now avoid specifying 
> constraints in new define_expands, but we should clean up the existing 
> define_expand definitions.
> 
> For example, the constraint "=w" is removed in the following case:
> (define_expand "sqrt2"
>[(set (match_operand:GPF_F16 0 "register_operand" "=w")
> The "" marks with an empty constraint in define_expand are removed as well.
> 
> The patch is tested with the build configuration of 
> --target=aarch64-none-linux-gnu, and it passes gcc/testsuite.

This is OK for trunk.

Thanks,
James

> gcc/ChangeLog:
> 
> 2019-06-21  Dennis Zhang  
> 
>   * config/aarch64/aarch64-simd.md: Remove redundant constraints
>   from define_expand.
>   * config/aarch64/aarch64-sve.md: Likewise.
>   * config/aarch64/aarch64.md: Likewise.
>   * config/aarch64/atomics.md: Likewise.

[PATCH][AArch64] Remove constraint strings from define_expand constructs in the back end

[Dennis Zhang]

Hi,

A number of AArch64 define_expand patterns have specified constraints 
for their operands. But the constraint strings are ignored at expand 
time and are therefore redundant/useless. We now avoid specifying 
constraints in new define_expands, but we should clean up the existing 
define_expand definitions.

For example, the constraint "=w" is removed in the following case:
(define_expand "sqrt2"
   [(set (match_operand:GPF_F16 0 "register_operand" "=w")
The "" marks with an empty constraint in define_expand are removed as well.

The patch is tested with the build configuration of 
--target=aarch64-none-linux-gnu, and it passes gcc/testsuite.

Thanks
Dennis

gcc/ChangeLog:

2019-06-21  Dennis Zhang  

* config/aarch64/aarch64-simd.md: Remove redundant constraints
from define_expand.
* config/aarch64/aarch64-sve.md: Likewise.
* config/aarch64/aarch64.md: Likewise.
* config/aarch64/atomics.md: Likewise.
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index df8bf1d9778..837242c7e56 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -19,8 +19,8 @@
 ;; .
 
 (define_expand "mov"
-  [(set (match_operand:VALL_F16 0 "nonimmediate_operand" "")
-	(match_operand:VALL_F16 1 "general_operand" ""))]
+  [(set (match_operand:VALL_F16 0 "nonimmediate_operand")
+	(match_operand:VALL_F16 1 "general_operand"))]
   "TARGET_SIMD"
   "
   /* Force the operand into a register if it is not an
@@ -39,8 +39,8 @@
 )
 
 (define_expand "movmisalign"
-  [(set (match_operand:VALL 0 "nonimmediate_operand" "")
-(match_operand:VALL 1 "general_operand" ""))]
+  [(set (match_operand:VALL 0 "nonimmediate_operand")
+(match_operand:VALL 1 "general_operand"))]
   "TARGET_SIMD"
 {
   /* This pattern is not permitted to fail during expansion: if both arguments
@@ -652,8 +652,8 @@
   [(set_attr "type" "neon_fp_rsqrts_")])
 
 (define_expand "rsqrt2"
-  [(set (match_operand:VALLF 0 "register_operand" "=w")
-	(unspec:VALLF [(match_operand:VALLF 1 "register_operand" "w")]
+  [(set (match_operand:VALLF 0 "register_operand")
+	(unspec:VALLF [(match_operand:VALLF 1 "register_operand")]
 		 UNSPEC_RSQRT))]
   "TARGET_SIMD"
 {
@@ -1025,9 +1025,9 @@
 )
 
 (define_expand "ashl3"
-  [(match_operand:VDQ_I 0 "register_operand" "")
-   (match_operand:VDQ_I 1 "register_operand" "")
-   (match_operand:SI  2 "general_operand" "")]
+  [(match_operand:VDQ_I 0 "register_operand")
+   (match_operand:VDQ_I 1 "register_operand")
+   (match_operand:SI  2 "general_operand")]
  "TARGET_SIMD"
 {
   int bit_width = GET_MODE_UNIT_SIZE (mode) * BITS_PER_UNIT;
@@ -1072,9 +1072,9 @@
 )
 
 (define_expand "lshr3"
-  [(match_operand:VDQ_I 0 "register_operand" "")
-   (match_operand:VDQ_I 1 "register_operand" "")
-   (match_operand:SI  2 "general_operand" "")]
+  [(match_operand:VDQ_I 0 "register_operand")
+   (match_operand:VDQ_I 1 "register_operand")
+   (match_operand:SI  2 "general_operand")]
  "TARGET_SIMD"
 {
   int bit_width = GET_MODE_UNIT_SIZE (mode) * BITS_PER_UNIT;
@@ -1119,9 +1119,9 @@
 )
 
 (define_expand "ashr3"
-  [(match_operand:VDQ_I 0 "register_operand" "")
-   (match_operand:VDQ_I 1 "register_operand" "")
-   (match_operand:SI  2 "general_operand" "")]
+  [(match_operand:VDQ_I 0 "register_operand")
+   (match_operand:VDQ_I 1 "register_operand")
+   (match_operand:SI  2 "general_operand")]
  "TARGET_SIMD"
 {
   int bit_width = GET_MODE_UNIT_SIZE (mode) * BITS_PER_UNIT;
@@ -1166,9 +1166,9 @@
 )
 
 (define_expand "vashl3"
- [(match_operand:VDQ_I 0 "register_operand" "")
-  (match_operand:VDQ_I 1 "register_operand" "")
-  (match_operand:VDQ_I 2 "register_operand" "")]
+ [(match_operand:VDQ_I 0 "register_operand")
+  (match_operand:VDQ_I 1 "register_operand")
+  (match_operand:VDQ_I 2 "register_operand")]
  "TARGET_SIMD"
 {
   emit_insn (gen_aarch64_simd_reg_sshl (operands[0], operands[1],
@@ -1180,9 +1180,9 @@
 ;; Negating individual lanes most certainly offsets the
 ;; gain from vectorization.
 (define_expand "vashr3"
- [(match_operand:VDQ_BHSI 0 "register_operand" "")
-  (match_operand:VDQ_BHSI 1 "register_operand" "")
-  (match_operand:VDQ_BHSI 2 "register_operand" "")]
+ [(match_operand:VDQ_BHSI 0 "register_operand")
+  (match_operand:VDQ_BHSI 1 "register_operand")
+  (match_operand:VDQ_BHSI 2 "register_operand")]
  "TARGET_SIMD"
 {
   rtx neg = gen_reg_rtx (mode);
@@ -1194,9 +1194,9 @@
 
 ;; DI vector shift
 (define_expand "aarch64_ashr_simddi"
-  [(match_operand:DI 0 "register_operand" "=w")
-   (match_operand:DI 1 "register_operand" "w")
-   (match_operand:SI 2 "aarch64_shift_imm64_di" "")]
+  [(match_operand:DI 0 "register_operand")
+   (match_operand:DI 1 "register_operand")
+   (match_operand:SI 2 "aarch64_shift_imm64_di")]
   "TARGET_SIMD"
   {
 /* An arithmetic shift right by 64 fills the result with copies of the sign
@@ -1210,9 +1210,9 @@
 )
 
 (define_expand "vlshr3"
-