* [PATCH][ARM] Remove movdi_vfp_cortexa8
@ 2016-11-29 11:06 Wilco Dijkstra
2016-12-06 15:03 ` Wilco Dijkstra
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Wilco Dijkstra @ 2016-11-29 11:06 UTC (permalink / raw)
To: GCC Patches; +Cc: nd
Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
unnecessary duplication and repeating bugs like PR78439 due to changes being
applied only to one of the duplicates.
Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
ChangeLog:
2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
* config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
* (movdi_vfp_cortexa8): Remove pattern.
--
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -304,9 +304,9 @@
;; DImode moves
(define_insn "*movdi_vfp"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
+ [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
(match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
+ "TARGET_32BIT && TARGET_HARD_FLOAT
&& ( register_operand (operands[0], DImode)
|| register_operand (operands[1], DImode))
&& !(TARGET_NEON && CONST_INT_P (operands[1])
@@ -339,71 +339,25 @@
}
"
[(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
+ (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
(eq_attr "alternative" "2") (const_int 12)
(eq_attr "alternative" "3") (const_int 16)
+ (eq_attr "alternative" "4,5,6")
+ (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
(eq_attr "alternative" "9")
(if_then_else
(match_test "TARGET_VFP_SINGLE")
(const_int 8)
(const_int 4))]
(const_int 4)))
+ (set_attr "predicable" "yes")
(set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
(set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
(set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
+ (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
(set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
)
-(define_insn "*movdi_vfp_cortexa8"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
- (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
- && ( register_operand (operands[0], DImode)
- || register_operand (operands[1], DImode))
- && !(TARGET_NEON && CONST_INT_P (operands[1])
- && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
- "*
- switch (which_alternative)
- {
- case 0:
- case 1:
- case 2:
- case 3:
- return \"#\";
- case 4:
- case 5:
- case 6:
- return output_move_double (operands, true, NULL);
- case 7:
- return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
- case 8:
- return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
- case 9:
- return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
- case 10: case 11:
- return output_move_vfp (operands);
- default:
- gcc_unreachable ();
- }
- "
- [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
- (eq_attr "alternative" "2") (const_int 12)
- (eq_attr "alternative" "3") (const_int 16)
- (eq_attr "alternative" "4,5,6")
- (symbol_ref
- "arm_count_output_move_double_insns (operands) \
- * 4")]
- (const_int 4)))
- (set_attr "predicable" "yes")
- (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
- (set (attr "ce_count")
- (symbol_ref "get_attr_length (insn) / 4"))
- (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
- )
-
;; HFmode moves
(define_insn "*movhf_vfp_fp16"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-11-29 11:06 [PATCH][ARM] Remove movdi_vfp_cortexa8 Wilco Dijkstra
@ 2016-12-06 15:03 ` Wilco Dijkstra
2016-12-14 16:38 ` Wilco Dijkstra
2017-02-02 14:48 ` Wilco Dijkstra
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Wilco Dijkstra @ 2016-12-06 15:03 UTC (permalink / raw)
To: GCC Patches, Kyrylo Tkachov; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 29 November 2016 11:05
To: GCC Patches
Cc: nd
Subject: [PATCH][ARM] Remove movdi_vfp_cortexa8
Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
unnecessary duplication and repeating bugs like PR78439 due to changes being
applied only to one of the duplicates.
Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
ChangeLog:
2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
* config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
* (movdi_vfp_cortexa8): Remove pattern.
--
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -304,9 +304,9 @@
;; DImode moves
(define_insn "*movdi_vfp"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
+ [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
(match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
+ "TARGET_32BIT && TARGET_HARD_FLOAT
&& ( register_operand (operands[0], DImode)
|| register_operand (operands[1], DImode))
&& !(TARGET_NEON && CONST_INT_P (operands[1])
@@ -339,71 +339,25 @@
}
"
[(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
+ (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
(eq_attr "alternative" "2") (const_int 12)
(eq_attr "alternative" "3") (const_int 16)
+ (eq_attr "alternative" "4,5,6")
+ (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
(eq_attr "alternative" "9")
(if_then_else
(match_test "TARGET_VFP_SINGLE")
(const_int 8)
(const_int 4))]
(const_int 4)))
+ (set_attr "predicable" "yes")
(set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
(set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
(set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
+ (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
(set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
)
-(define_insn "*movdi_vfp_cortexa8"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
- (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
- && ( register_operand (operands[0], DImode)
- || register_operand (operands[1], DImode))
- && !(TARGET_NEON && CONST_INT_P (operands[1])
- && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
- "*
- switch (which_alternative)
- {
- case 0:
- case 1:
- case 2:
- case 3:
- return \"#\";
- case 4:
- case 5:
- case 6:
- return output_move_double (operands, true, NULL);
- case 7:
- return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
- case 8:
- return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
- case 9:
- return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
- case 10: case 11:
- return output_move_vfp (operands);
- default:
- gcc_unreachable ();
- }
- "
- [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
- (eq_attr "alternative" "2") (const_int 12)
- (eq_attr "alternative" "3") (const_int 16)
- (eq_attr "alternative" "4,5,6")
- (symbol_ref
- "arm_count_output_move_double_insns (operands) \
- * 4")]
- (const_int 4)))
- (set_attr "predicable" "yes")
- (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
- (set (attr "ce_count")
- (symbol_ref "get_attr_length (insn) / 4"))
- (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
- )
-
;; HFmode moves
(define_insn "*movhf_vfp_fp16"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-12-06 15:03 ` Wilco Dijkstra
@ 2016-12-14 16:38 ` Wilco Dijkstra
2016-12-14 16:45 ` Kyrill Tkachov
0 siblings, 1 reply; 17+ messages in thread
From: Wilco Dijkstra @ 2016-12-14 16:38 UTC (permalink / raw)
To: GCC Patches, Kyrylo Tkachov; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 29 November 2016 11:05
To: GCC Patches
Cc: nd
Subject: [PATCH][ARM] Remove movdi_vfp_cortexa8
Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
unnecessary duplication and repeating bugs like PR78439 due to changes being
applied only to one of the duplicates.
Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
ChangeLog:
2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
* config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
* (movdi_vfp_cortexa8): Remove pattern.
--
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -304,9 +304,9 @@
;; DImode moves
(define_insn "*movdi_vfp"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
+ [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
(match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
+ "TARGET_32BIT && TARGET_HARD_FLOAT
&& ( register_operand (operands[0], DImode)
|| register_operand (operands[1], DImode))
&& !(TARGET_NEON && CONST_INT_P (operands[1])
@@ -339,71 +339,25 @@
}
"
[(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
+ (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
(eq_attr "alternative" "2") (const_int 12)
(eq_attr "alternative" "3") (const_int 16)
+ (eq_attr "alternative" "4,5,6")
+ (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
(eq_attr "alternative" "9")
(if_then_else
(match_test "TARGET_VFP_SINGLE")
(const_int 8)
(const_int 4))]
(const_int 4)))
+ (set_attr "predicable" "yes")
(set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
(set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
(set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
+ (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
(set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
)
-(define_insn "*movdi_vfp_cortexa8"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
- (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
- && ( register_operand (operands[0], DImode)
- || register_operand (operands[1], DImode))
- && !(TARGET_NEON && CONST_INT_P (operands[1])
- && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
- "*
- switch (which_alternative)
- {
- case 0:
- case 1:
- case 2:
- case 3:
- return \"#\";
- case 4:
- case 5:
- case 6:
- return output_move_double (operands, true, NULL);
- case 7:
- return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
- case 8:
- return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
- case 9:
- return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
- case 10: case 11:
- return output_move_vfp (operands);
- default:
- gcc_unreachable ();
- }
- "
- [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
- (eq_attr "alternative" "2") (const_int 12)
- (eq_attr "alternative" "3") (const_int 16)
- (eq_attr "alternative" "4,5,6")
- (symbol_ref
- "arm_count_output_move_double_insns (operands) \
- * 4")]
- (const_int 4)))
- (set_attr "predicable" "yes")
- (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
- (set (attr "ce_count")
- (symbol_ref "get_attr_length (insn) / 4"))
- (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
- )
-
;; HFmode moves
(define_insn "*movhf_vfp_fp16"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-12-14 16:38 ` Wilco Dijkstra
@ 2016-12-14 16:45 ` Kyrill Tkachov
[not found] ` <AM5PR0802MB2610BA9D4EEAF7BB81A4FA26839A0@AM5PR0802MB2610.eurprd08.prod.outlook.com>
0 siblings, 1 reply; 17+ messages in thread
From: Kyrill Tkachov @ 2016-12-14 16:45 UTC (permalink / raw)
To: Wilco Dijkstra, GCC Patches
Hi Wilco,
On 14/12/16 16:37, Wilco Dijkstra wrote:
>
>
> ping
>
>
> From: Wilco Dijkstra
> Sent: 29 November 2016 11:05
> To: GCC Patches
> Cc: nd
> Subject: [PATCH][ARM] Remove movdi_vfp_cortexa8
>
> Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
> unnecessary duplication and repeating bugs like PR78439 due to changes being
> applied only to one of the duplicates.
When this was brought up Ramana requested some more investigations on the codegen impact [1].
Have you done the archaeology on why we had two patterns in the first place and what the codegen
effect is of remove the Cortex-A8-specific one?
Thanks,
Kyrill
[1] https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02248.html
> Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
>
> ChangeLog:
> 2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
>
> * config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
> * (movdi_vfp_cortexa8): Remove pattern.
> --
>
> diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
> index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
> --- a/gcc/config/arm/vfp.md
> +++ b/gcc/config/arm/vfp.md
> @@ -304,9 +304,9 @@
> ;; DImode moves
>
> (define_insn "*movdi_vfp"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
> (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
> + "TARGET_32BIT && TARGET_HARD_FLOAT
> && ( register_operand (operands[0], DImode)
> || register_operand (operands[1], DImode))
> && !(TARGET_NEON && CONST_INT_P (operands[1])
> @@ -339,71 +339,25 @@
> }
> "
> [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
> - (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
> + (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
> (eq_attr "alternative" "2") (const_int 12)
> (eq_attr "alternative" "3") (const_int 16)
> + (eq_attr "alternative" "4,5,6")
> + (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
> (eq_attr "alternative" "9")
> (if_then_else
> (match_test "TARGET_VFP_SINGLE")
> (const_int 8)
> (const_int 4))]
> (const_int 4)))
> + (set_attr "predicable" "yes")
> (set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
> (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
> + (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
> (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> )
>
> -(define_insn "*movdi_vfp_cortexa8"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
> - (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
> - && ( register_operand (operands[0], DImode)
> - || register_operand (operands[1], DImode))
> - && !(TARGET_NEON && CONST_INT_P (operands[1])
> - && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
> - "*
> - switch (which_alternative)
> - {
> - case 0:
> - case 1:
> - case 2:
> - case 3:
> - return \"#\";
> - case 4:
> - case 5:
> - case 6:
> - return output_move_double (operands, true, NULL);
> - case 7:
> - return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
> - case 8:
> - return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
> - case 9:
> - return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
> - case 10: case 11:
> - return output_move_vfp (operands);
> - default:
> - gcc_unreachable ();
> - }
> - "
> - [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
> - (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
> - (eq_attr "alternative" "2") (const_int 12)
> - (eq_attr "alternative" "3") (const_int 16)
> - (eq_attr "alternative" "4,5,6")
> - (symbol_ref
> - "arm_count_output_move_double_insns (operands) \
> - * 4")]
> - (const_int 4)))
> - (set_attr "predicable" "yes")
> - (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> - (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> - (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
> - (set (attr "ce_count")
> - (symbol_ref "get_attr_length (insn) / 4"))
> - (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> - )
> -
> ;; HFmode moves
>
> (define_insn "*movhf_vfp_fp16"
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
[not found] ` <AM5PR0802MB2610BA9D4EEAF7BB81A4FA26839A0@AM5PR0802MB2610.eurprd08.prod.outlook.com>
@ 2016-12-14 17:49 ` Wilco Dijkstra
2016-12-19 9:50 ` Ramana Radhakrishnan
2017-10-16 11:30 ` Wilco Dijkstra
0 siblings, 2 replies; 17+ messages in thread
From: Wilco Dijkstra @ 2016-12-14 17:49 UTC (permalink / raw)
To: Kyrill Tkachov, GCC Patches; +Cc: nd
Kyrill Tkachov wrote:
> On 14/12/16 16:37, Wilco Dijkstra wrote:
>
> > Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
> > unnecessary duplication and repeating bugs like PR78439 due to changes being
> > applied only to one of the duplicates.
>
> When this was brought up Ramana requested some more investigations on the codegen impact [1].
> Have you done the archaeology on why we had two patterns in the first place and what the codegen
> effect is of remove the Cortex-A8-specific one?
Yes, the reason to split the pattern was to introduce the '!' to discourage Neon->int moves on Cortex-A8 (https://patches.linaro.org/patch/541/). I am not removing the optimization for Cortex-A8, however I haven't been able to find an example where it makes a difference, even on high register pressure code.
Wilco
> Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
>
> ChangeLog:
> 2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
>
> * config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
> * (movdi_vfp_cortexa8): Remove pattern.
> --
>
> diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
> index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
> --- a/gcc/config/arm/vfp.md
> +++ b/gcc/config/arm/vfp.md
> @@ -304,9 +304,9 @@
> ;; DImode moves
>
> (define_insn "*movdi_vfp"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
> (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
> + "TARGET_32BIT && TARGET_HARD_FLOAT
> && ( register_operand (operands[0], DImode)
> || register_operand (operands[1], DImode))
> && !(TARGET_NEON && CONST_INT_P (operands[1])
> @@ -339,71 +339,25 @@
> }
> "
> [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
> - (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
> + (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
> (eq_attr "alternative" "2") (const_int 12)
> (eq_attr "alternative" "3") (const_int 16)
> + (eq_attr "alternative" "4,5,6")
> + (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
> (eq_attr "alternative" "9")
> (if_then_else
> (match_test "TARGET_VFP_SINGLE")
> (const_int 8)
> (const_int 4))]
> (const_int 4)))
> + (set_attr "predicable" "yes")
> (set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
> (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
> + (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
> (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> )
>
> -(define_insn "*movdi_vfp_cortexa8"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
> - (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
> - && ( register_operand (operands[0], DImode)
> - || register_operand (operands[1], DImode))
> - && !(TARGET_NEON && CONST_INT_P (operands[1])
> - && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
> - "*
> - switch (which_alternative)
> - {
> - case 0:
> - case 1:
> - case 2:
> - case 3:
> - return \"#\";
> - case 4:
> - case 5:
> - case 6:
> - return output_move_double (operands, true, NULL);
> - case 7:
> - return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
> - case 8:
> - return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
> - case 9:
> - return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
> - case 10: case 11:
> - return output_move_vfp (operands);
> - default:
> - gcc_unreachable ();
> - }
> - "
> - [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
> - (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
> - (eq_attr "alternative" "2") (const_int 12)
> - (eq_attr "alternative" "3") (const_int 16)
> - (eq_attr "alternative" "4,5,6")
> - (symbol_ref
> - "arm_count_output_move_double_insns (operands) \
> - * 4")]
> - (const_int 4)))
> - (set_attr "predicable" "yes")
> - (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> - (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> - (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
> - (set (attr "ce_count")
> - (symbol_ref "get_attr_length (insn) / 4"))
> - (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> - )
> -
> ;; HFmode moves
>
> (define_insn "*movhf_vfp_fp16"
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-12-14 17:49 ` Wilco Dijkstra
@ 2016-12-19 9:50 ` Ramana Radhakrishnan
2016-12-19 13:09 ` Wilco Dijkstra
2017-10-16 11:30 ` Wilco Dijkstra
1 sibling, 1 reply; 17+ messages in thread
From: Ramana Radhakrishnan @ 2016-12-19 9:50 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: Kyrill Tkachov, GCC Patches, nd
On Wed, Dec 14, 2016 at 5:43 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> Kyrill Tkachov wrote:
>> On 14/12/16 16:37, Wilco Dijkstra wrote:
>>
>> > Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
>> > unnecessary duplication and repeating bugs like PR78439 due to changes being
>> > applied only to one of the duplicates.
>>
>> When this was brought up Ramana requested some more investigations on the codegen impact [1].
>> Have you done the archaeology on why we had two patterns in the first place and what the codegen
>> effect is of remove the Cortex-A8-specific one?
>
> Yes, the reason to split the pattern was to introduce the '!' to discourage Neon->int moves on Cortex-A8 (https://patches.linaro.org/patch/541/). I am not removing the optimization for Cortex-A8, however I haven't been able to find an example where it makes a difference, even on high register pressure code.
>
even on crafty with -mtune=cortex-a8 ?
Ramana
> Wilco
>
>
>> Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
>>
>> ChangeLog:
>> 2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
>>
>> * config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
>> * (movdi_vfp_cortexa8): Remove pattern.
>> --
>>
>> diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
>> index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
>> --- a/gcc/config/arm/vfp.md
>> +++ b/gcc/config/arm/vfp.md
>> @@ -304,9 +304,9 @@
>> ;; DImode moves
>>
>> (define_insn "*movdi_vfp"
>> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
>> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
>> (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
>> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
>> + "TARGET_32BIT && TARGET_HARD_FLOAT
>> && ( register_operand (operands[0], DImode)
>> || register_operand (operands[1], DImode))
>> && !(TARGET_NEON && CONST_INT_P (operands[1])
>> @@ -339,71 +339,25 @@
>> }
>> "
>> [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
>> - (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
>> + (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
>> (eq_attr "alternative" "2") (const_int 12)
>> (eq_attr "alternative" "3") (const_int 16)
>> + (eq_attr "alternative" "4,5,6")
>> + (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
>> (eq_attr "alternative" "9")
>> (if_then_else
>> (match_test "TARGET_VFP_SINGLE")
>> (const_int 8)
>> (const_int 4))]
>> (const_int 4)))
>> + (set_attr "predicable" "yes")
>> (set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
>> (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
>> (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
>> + (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
>> (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
>> )
>>
>> -(define_insn "*movdi_vfp_cortexa8"
>> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
>> - (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
>> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
>> - && ( register_operand (operands[0], DImode)
>> - || register_operand (operands[1], DImode))
>> - && !(TARGET_NEON && CONST_INT_P (operands[1])
>> - && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
>> - "*
>> - switch (which_alternative)
>> - {
>> - case 0:
>> - case 1:
>> - case 2:
>> - case 3:
>> - return \"#\";
>> - case 4:
>> - case 5:
>> - case 6:
>> - return output_move_double (operands, true, NULL);
>> - case 7:
>> - return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
>> - case 8:
>> - return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
>> - case 9:
>> - return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
>> - case 10: case 11:
>> - return output_move_vfp (operands);
>> - default:
>> - gcc_unreachable ();
>> - }
>> - "
>> - [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
>> - (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
>> - (eq_attr "alternative" "2") (const_int 12)
>> - (eq_attr "alternative" "3") (const_int 16)
>> - (eq_attr "alternative" "4,5,6")
>> - (symbol_ref
>> - "arm_count_output_move_double_insns (operands) \
>> - * 4")]
>> - (const_int 4)))
>> - (set_attr "predicable" "yes")
>> - (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
>> - (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
>> - (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
>> - (set (attr "ce_count")
>> - (symbol_ref "get_attr_length (insn) / 4"))
>> - (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
>> - )
>> -
>> ;; HFmode moves
>>
>> (define_insn "*movhf_vfp_fp16"
>>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-12-19 9:50 ` Ramana Radhakrishnan
@ 2016-12-19 13:09 ` Wilco Dijkstra
2017-01-17 12:11 ` Wilco Dijkstra
0 siblings, 1 reply; 17+ messages in thread
From: Wilco Dijkstra @ 2016-12-19 13:09 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: Kyrill Tkachov, GCC Patches, nd
Ramana Radhakrishnan wrote:
> On Wed, Dec 14, 2016 at 5:43 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> > Yes, the reason to split the pattern was to introduce the '!' to discourage Neon->int moves on Cortex-A8 (https://patches.linaro.org/patch/541/). I am not removing the optimization for Cortex-A8, however I haven't been able to find an example where it makes a difference, even on high register pressure code.
>
>
> even on crafty with -mtune=cortex-a8 ?
Indeed the '!' makes no difference on crafty either with -mcpu=cortex-a8 -mfpu=neon.
Even if it did make a difference in the past, it is either totally ignored by the register
allocator or has no useful effect on deciding whether to use a Neon or integer register.
Wilco
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-12-19 13:09 ` Wilco Dijkstra
@ 2017-01-17 12:11 ` Wilco Dijkstra
0 siblings, 0 replies; 17+ messages in thread
From: Wilco Dijkstra @ 2017-01-17 12:11 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: Kyrill Tkachov, GCC Patches, nd
Wilco Dijkstra wrote:
> Ramana Radhakrishnan wrote:
>> On Wed, Dec 14, 2016 at 5:43 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>
> > > Yes, the reason to split the pattern was to introduce the '!' to discourage Neon->int moves on Cortex-A8
> (https://patches.linaro.org/patch/541/). I am not removing the optimization for Cortex-A8, however
> I haven't been able to find an example where it makes a difference, even on high register pressure code.
> >
> > even on crafty with -mtune=cortex-a8 ?
>
> Indeed the '!' makes no difference on crafty either with -mcpu=cortex-a8 -mfpu=neon.
> Even if it did make a difference in the past, it is either totally ignored by the register
> allocator or has no useful effect on deciding whether to use a Neon or integer register.
Any comments?
Wilco
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-11-29 11:06 [PATCH][ARM] Remove movdi_vfp_cortexa8 Wilco Dijkstra
2016-12-06 15:03 ` Wilco Dijkstra
@ 2017-02-02 14:48 ` Wilco Dijkstra
2017-02-23 16:57 ` Wilco Dijkstra
2017-04-20 16:37 ` Wilco Dijkstra
2017-05-05 14:27 ` Richard Earnshaw (lists)
3 siblings, 1 reply; 17+ messages in thread
From: Wilco Dijkstra @ 2017-02-02 14:48 UTC (permalink / raw)
To: GCC Patches, Ramana Radhakrishnan, Kyrylo Tkachov; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 29 November 2016 11:05
To: GCC Patches
Cc: nd
Subject: [PATCH][ARM] Remove movdi_vfp_cortexa8
Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
unnecessary duplication and repeating bugs like PR78439 due to changes being
applied only to one of the duplicates.
Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
ChangeLog:
2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
* config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
* (movdi_vfp_cortexa8): Remove pattern.
--
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -304,9 +304,9 @@
;; DImode moves
(define_insn "*movdi_vfp"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
+ [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
(match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
+ "TARGET_32BIT && TARGET_HARD_FLOAT
&& ( register_operand (operands[0], DImode)
|| register_operand (operands[1], DImode))
&& !(TARGET_NEON && CONST_INT_P (operands[1])
@@ -339,71 +339,25 @@
}
"
[(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
+ (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
(eq_attr "alternative" "2") (const_int 12)
(eq_attr "alternative" "3") (const_int 16)
+ (eq_attr "alternative" "4,5,6")
+ (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
(eq_attr "alternative" "9")
(if_then_else
(match_test "TARGET_VFP_SINGLE")
(const_int 8)
(const_int 4))]
(const_int 4)))
+ (set_attr "predicable" "yes")
(set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
(set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
(set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
+ (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
(set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
)
-(define_insn "*movdi_vfp_cortexa8"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
- (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
- && ( register_operand (operands[0], DImode)
- || register_operand (operands[1], DImode))
- && !(TARGET_NEON && CONST_INT_P (operands[1])
- && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
- "*
- switch (which_alternative)
- {
- case 0:
- case 1:
- case 2:
- case 3:
- return \"#\";
- case 4:
- case 5:
- case 6:
- return output_move_double (operands, true, NULL);
- case 7:
- return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
- case 8:
- return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
- case 9:
- return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
- case 10: case 11:
- return output_move_vfp (operands);
- default:
- gcc_unreachable ();
- }
- "
- [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
- (eq_attr "alternative" "2") (const_int 12)
- (eq_attr "alternative" "3") (const_int 16)
- (eq_attr "alternative" "4,5,6")
- (symbol_ref
- "arm_count_output_move_double_insns (operands) \
- * 4")]
- (const_int 4)))
- (set_attr "predicable" "yes")
- (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
- (set (attr "ce_count")
- (symbol_ref "get_attr_length (insn) / 4"))
- (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
- )
-
;; HFmode moves
(define_insn "*movhf_vfp_fp16"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2017-02-02 14:48 ` Wilco Dijkstra
@ 2017-02-23 16:57 ` Wilco Dijkstra
0 siblings, 0 replies; 17+ messages in thread
From: Wilco Dijkstra @ 2017-02-23 16:57 UTC (permalink / raw)
To: GCC Patches, Ramana Radhakrishnan, Kyrylo Tkachov; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 29 November 2016 11:05
To: GCC Patches
Cc: nd
Subject: [PATCH][ARM] Remove movdi_vfp_cortexa8
Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
unnecessary duplication and repeating bugs like PR78439 due to changes being
applied only to one of the duplicates.
Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
ChangeLog:
2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
* config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
* (movdi_vfp_cortexa8): Remove pattern.
--
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -304,9 +304,9 @@
;; DImode moves
(define_insn "*movdi_vfp"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
+ [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
(match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
+ "TARGET_32BIT && TARGET_HARD_FLOAT
&& ( register_operand (operands[0], DImode)
|| register_operand (operands[1], DImode))
&& !(TARGET_NEON && CONST_INT_P (operands[1])
@@ -339,71 +339,25 @@
}
"
[(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
+ (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
(eq_attr "alternative" "2") (const_int 12)
(eq_attr "alternative" "3") (const_int 16)
+ (eq_attr "alternative" "4,5,6")
+ (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
(eq_attr "alternative" "9")
(if_then_else
(match_test "TARGET_VFP_SINGLE")
(const_int 8)
(const_int 4))]
(const_int 4)))
+ (set_attr "predicable" "yes")
(set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
(set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
(set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
+ (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
(set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
)
-(define_insn "*movdi_vfp_cortexa8"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
- (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
- && ( register_operand (operands[0], DImode)
- || register_operand (operands[1], DImode))
- && !(TARGET_NEON && CONST_INT_P (operands[1])
- && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
- "*
- switch (which_alternative)
- {
- case 0:
- case 1:
- case 2:
- case 3:
- return \"#\";
- case 4:
- case 5:
- case 6:
- return output_move_double (operands, true, NULL);
- case 7:
- return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
- case 8:
- return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
- case 9:
- return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
- case 10: case 11:
- return output_move_vfp (operands);
- default:
- gcc_unreachable ();
- }
- "
- [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
- (eq_attr "alternative" "2") (const_int 12)
- (eq_attr "alternative" "3") (const_int 16)
- (eq_attr "alternative" "4,5,6")
- (symbol_ref
- "arm_count_output_move_double_insns (operands) \
- * 4")]
- (const_int 4)))
- (set_attr "predicable" "yes")
- (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
- (set (attr "ce_count")
- (symbol_ref "get_attr_length (insn) / 4"))
- (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
- )
-
;; HFmode moves
(define_insn "*movhf_vfp_fp16"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-11-29 11:06 [PATCH][ARM] Remove movdi_vfp_cortexa8 Wilco Dijkstra
2016-12-06 15:03 ` Wilco Dijkstra
2017-02-02 14:48 ` Wilco Dijkstra
@ 2017-04-20 16:37 ` Wilco Dijkstra
2017-05-05 14:27 ` Richard Earnshaw (lists)
3 siblings, 0 replies; 17+ messages in thread
From: Wilco Dijkstra @ 2017-04-20 16:37 UTC (permalink / raw)
To: GCC Patches, Kyrylo Tkachov; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 29 November 2016 11:05
To: GCC Patches
Cc: nd
Subject: [PATCH][ARM] Remove movdi_vfp_cortexa8
Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
unnecessary duplication and repeating bugs like PR78439 due to changes being
applied only to one of the duplicates.
Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
ChangeLog:
2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
* config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
* (movdi_vfp_cortexa8): Remove pattern.
--
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -304,9 +304,9 @@
;; DImode moves
(define_insn "*movdi_vfp"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
+ [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
(match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
+ "TARGET_32BIT && TARGET_HARD_FLOAT
&& ( register_operand (operands[0], DImode)
|| register_operand (operands[1], DImode))
&& !(TARGET_NEON && CONST_INT_P (operands[1])
@@ -339,71 +339,25 @@
}
"
[(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
+ (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
(eq_attr "alternative" "2") (const_int 12)
(eq_attr "alternative" "3") (const_int 16)
+ (eq_attr "alternative" "4,5,6")
+ (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
(eq_attr "alternative" "9")
(if_then_else
(match_test "TARGET_VFP_SINGLE")
(const_int 8)
(const_int 4))]
(const_int 4)))
+ (set_attr "predicable" "yes")
(set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
(set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
(set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
+ (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
(set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
)
-(define_insn "*movdi_vfp_cortexa8"
- [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
- (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
- "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
- && ( register_operand (operands[0], DImode)
- || register_operand (operands[1], DImode))
- && !(TARGET_NEON && CONST_INT_P (operands[1])
- && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
- "*
- switch (which_alternative)
- {
- case 0:
- case 1:
- case 2:
- case 3:
- return \"#\";
- case 4:
- case 5:
- case 6:
- return output_move_double (operands, true, NULL);
- case 7:
- return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
- case 8:
- return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
- case 9:
- return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
- case 10: case 11:
- return output_move_vfp (operands);
- default:
- gcc_unreachable ();
- }
- "
- [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
- (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
- (eq_attr "alternative" "2") (const_int 12)
- (eq_attr "alternative" "3") (const_int 16)
- (eq_attr "alternative" "4,5,6")
- (symbol_ref
- "arm_count_output_move_double_insns (operands) \
- * 4")]
- (const_int 4)))
- (set_attr "predicable" "yes")
- (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
- (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
- (set (attr "ce_count")
- (symbol_ref "get_attr_length (insn) / 4"))
- (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
- )
-
;; HFmode moves
(define_insn "*movhf_vfp_fp16"
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-11-29 11:06 [PATCH][ARM] Remove movdi_vfp_cortexa8 Wilco Dijkstra
` (2 preceding siblings ...)
2017-04-20 16:37 ` Wilco Dijkstra
@ 2017-05-05 14:27 ` Richard Earnshaw (lists)
2017-05-05 14:39 ` Wilco Dijkstra
3 siblings, 1 reply; 17+ messages in thread
From: Richard Earnshaw (lists) @ 2017-05-05 14:27 UTC (permalink / raw)
To: Wilco Dijkstra, GCC Patches; +Cc: nd
On 29/11/16 11:05, Wilco Dijkstra wrote:
> Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
> unnecessary duplication and repeating bugs like PR78439 due to changes being
> applied only to one of the duplicates.
>
> Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
>
> ChangeLog:
> 2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
>
> * config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
> * (movdi_vfp_cortexa8): Remove pattern.
> --
In general I'm in favour of cleanups like this, but ...
>
> diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
> index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
> --- a/gcc/config/arm/vfp.md
> +++ b/gcc/config/arm/vfp.md
> @@ -304,9 +304,9 @@
> ;; DImode moves
>
> (define_insn "*movdi_vfp"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
Why have you introduced a no-reloads block on the 9th alternative for
all variants?
R.
> (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
> + "TARGET_32BIT && TARGET_HARD_FLOAT
> && ( register_operand (operands[0], DImode)
> || register_operand (operands[1], DImode))
> && !(TARGET_NEON && CONST_INT_P (operands[1])
> @@ -339,71 +339,25 @@
> }
> "
> [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
> - (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
> + (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
> (eq_attr "alternative" "2") (const_int 12)
> (eq_attr "alternative" "3") (const_int 16)
> + (eq_attr "alternative" "4,5,6")
> + (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
> (eq_attr "alternative" "9")
> (if_then_else
> (match_test "TARGET_VFP_SINGLE")
> (const_int 8)
> (const_int 4))]
> (const_int 4)))
> + (set_attr "predicable" "yes")
> (set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
> (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
> + (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
> (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> )
>
> -(define_insn "*movdi_vfp_cortexa8"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
> - (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
> - && ( register_operand (operands[0], DImode)
> - || register_operand (operands[1], DImode))
> - && !(TARGET_NEON && CONST_INT_P (operands[1])
> - && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
> - "*
> - switch (which_alternative)
> - {
> - case 0:
> - case 1:
> - case 2:
> - case 3:
> - return \"#\";
> - case 4:
> - case 5:
> - case 6:
> - return output_move_double (operands, true, NULL);
> - case 7:
> - return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
> - case 8:
> - return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
> - case 9:
> - return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
> - case 10: case 11:
> - return output_move_vfp (operands);
> - default:
> - gcc_unreachable ();
> - }
> - "
> - [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
> - (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
> - (eq_attr "alternative" "2") (const_int 12)
> - (eq_attr "alternative" "3") (const_int 16)
> - (eq_attr "alternative" "4,5,6")
> - (symbol_ref
> - "arm_count_output_move_double_insns (operands) \
> - * 4")]
> - (const_int 4)))
> - (set_attr "predicable" "yes")
> - (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> - (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> - (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
> - (set (attr "ce_count")
> - (symbol_ref "get_attr_length (insn) / 4"))
> - (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> - )
> -
> ;; HFmode moves
>
> (define_insn "*movhf_vfp_fp16"
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2017-05-05 14:27 ` Richard Earnshaw (lists)
@ 2017-05-05 14:39 ` Wilco Dijkstra
2017-06-13 13:58 ` Wilco Dijkstra
0 siblings, 1 reply; 17+ messages in thread
From: Wilco Dijkstra @ 2017-05-05 14:39 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
Richard Earnshaw (lists) wrote:
> (define_insn "*movdi_vfp"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
> Why have you introduced a no-reloads block on the 9th alternative for
> all variants?
That is the default behaviour when you don't explicitly set a cpu, so I kept that.
See https://patches.linaro.org/patch/541/ for the original reason for adding it -
duplicating this pattern was a mistake since '!' wouldn't pessimize other cores
as int<->fp moves typically have a non-trivial cost.
However given Cortex-A8 is ancient now we could just remove the '!'.
Wilco
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2017-05-05 14:39 ` Wilco Dijkstra
@ 2017-06-13 13:58 ` Wilco Dijkstra
2017-06-27 15:40 ` Wilco Dijkstra
0 siblings, 1 reply; 17+ messages in thread
From: Wilco Dijkstra @ 2017-06-13 13:58 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
Richard Earnshaw (lists) wrote:
> (define_insn "*movdi_vfp"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
> Why have you introduced a no-reloads block on the 9th alternative for
> all variants?
That is the default behaviour when you don't explicitly set a cpu, so I kept that.
See https://patches.linaro.org/patch/541/ for the original reason for adding it -
duplicating this pattern was a mistake since '!' wouldn't pessimize other cores
as int<->fp moves typically have a non-trivial cost.
However given Cortex-A8 is ancient now we could just remove the '!'.
Wilco
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2017-06-13 13:58 ` Wilco Dijkstra
@ 2017-06-27 15:40 ` Wilco Dijkstra
0 siblings, 0 replies; 17+ messages in thread
From: Wilco Dijkstra @ 2017-06-27 15:40 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
Richard Earnshaw (lists) wrote:
> (define_insn "*movdi_vfp"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
> Why have you introduced a no-reloads block on the 9th alternative for
> all variants?
That is the default behaviour when you don't explicitly set a cpu, so I kept that.
See https://patches.linaro.org/patch/541/ for the original reason for adding it -
duplicating this pattern was a mistake since '!' wouldn't pessimize other cores
as int<->fp moves typically have a non-trivial cost.
However given Cortex-A8 is ancient now we could just remove the '!'.
Wilco
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2016-12-14 17:49 ` Wilco Dijkstra
2016-12-19 9:50 ` Ramana Radhakrishnan
@ 2017-10-16 11:30 ` Wilco Dijkstra
2017-10-27 16:53 ` Kyrill Tkachov
1 sibling, 1 reply; 17+ messages in thread
From: Wilco Dijkstra @ 2017-10-16 11:30 UTC (permalink / raw)
To: Kyrill Tkachov, GCC Patches; +Cc: nd
ping
Kyrill Tkachov wrote:
> On 14/12/16 16:37, Wilco Dijkstra wrote:
>
> > Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
> > unnecessary duplication and repeating bugs like PR78439 due to changes being
> > applied only to one of the duplicates.
>
> When this was brought up Ramana requested some more investigations on the codegen impact [1].
> Have you done the archaeology on why we had two patterns in the first place and what the codegen
> effect is of remove the Cortex-A8-specific one?
Yes, the reason to split the pattern was to introduce the '!' to discourage Neon->int moves on Cortex-A8 (https://patches.linaro.org/patch/541/). I am not removing the optimization for Cortex-A8, however I haven't been able to find an example where it makes a difference, even on high register pressure code.
Wilco
> Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
>
> ChangeLog:
> 2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
>
> * config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
> * (movdi_vfp_cortexa8): Remove pattern.
> --
>
> diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
> index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
> --- a/gcc/config/arm/vfp.md
> +++ b/gcc/config/arm/vfp.md
> @@ -304,9 +304,9 @@
> ;; DImode moves
>
> (define_insn "*movdi_vfp"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
> (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
> + "TARGET_32BIT && TARGET_HARD_FLOAT
> && ( register_operand (operands[0], DImode)
> || register_operand (operands[1], DImode))
> && !(TARGET_NEON && CONST_INT_P (operands[1])
> @@ -339,71 +339,25 @@
> }
> "
> [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
> - (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
> + (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
> (eq_attr "alternative" "2") (const_int 12)
> (eq_attr "alternative" "3") (const_int 16)
> + (eq_attr "alternative" "4,5,6")
> + (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
> (eq_attr "alternative" "9")
> (if_then_else
> (match_test "TARGET_VFP_SINGLE")
> (const_int 8)
> (const_int 4))]
> (const_int 4)))
> + (set_attr "predicable" "yes")
> (set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
> (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
> + (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
> (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> )
>
> -(define_insn "*movdi_vfp_cortexa8"
> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
> - (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
> - && ( register_operand (operands[0], DImode)
> - || register_operand (operands[1], DImode))
> - && !(TARGET_NEON && CONST_INT_P (operands[1])
> - && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
> - "*
> - switch (which_alternative)
> - {
> - case 0:
> - case 1:
> - case 2:
> - case 3:
> - return \"#\";
> - case 4:
> - case 5:
> - case 6:
> - return output_move_double (operands, true, NULL);
> - case 7:
> - return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
> - case 8:
> - return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
> - case 9:
> - return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
> - case 10: case 11:
> - return output_move_vfp (operands);
> - default:
> - gcc_unreachable ();
> - }
> - "
> - [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
> - (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
> - (eq_attr "alternative" "2") (const_int 12)
> - (eq_attr "alternative" "3") (const_int 16)
> - (eq_attr "alternative" "4,5,6")
> - (symbol_ref
> - "arm_count_output_move_double_insns (operands) \
> - * 4")]
> - (const_int 4)))
> - (set_attr "predicable" "yes")
> - (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> - (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
> - (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
> - (set (attr "ce_count")
> - (symbol_ref "get_attr_length (insn) / 4"))
> - (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
> - )
> -
> ;; HFmode moves
>
> (define_insn "*movhf_vfp_fp16"
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH][ARM] Remove movdi_vfp_cortexa8
2017-10-16 11:30 ` Wilco Dijkstra
@ 2017-10-27 16:53 ` Kyrill Tkachov
0 siblings, 0 replies; 17+ messages in thread
From: Kyrill Tkachov @ 2017-10-27 16:53 UTC (permalink / raw)
To: Wilco Dijkstra, GCC Patches; +Cc: nd
Hi Wilco,
On 16/10/17 12:29, Wilco Dijkstra wrote:
> ping
>
> Kyrill Tkachov wrote:
>> On 14/12/16 16:37, Wilco Dijkstra wrote:
>>
>>> Merge the movdi_vfp_cortexa8 pattern into movdi_vfp and remove it to avoid
>>> unnecessary duplication and repeating bugs like PR78439 due to changes being
>>> applied only to one of the duplicates.
>> When this was brought up Ramana requested some more investigations on the codegen impact [1].
>> Have you done the archaeology on why we had two patterns in the first place and what the codegen
>> effect is of remove the Cortex-A8-specific one?
> Yes, the reason to split the pattern was to introduce the '!' to discourage Neon->int moves on Cortex-A8 (https://patches.linaro.org/patch/541/). I am not removing the optimization for Cortex-A8, however I haven't been able to find an example where it makes a difference, even on high register pressure code.
Thanks, as far as I can see this doesn't impact codegen much these days
[1] and this Cortex-A8-specific pattern is a huge liability in my eyes.
So this patch is ok but please re-bootstrap and re-test it on
arm-none-linux-gnueabihf before committing.
Thank you for your patience with this,
Kyrill
[1] https://gcc.gnu.org/ml/gcc-patches/2016-12/msg01585.html
> Wilco
>
>
>> Bootstrap OK for ARM and Thumb-2 gnueabihf targets. OK for commit?
>>
>> ChangeLog:
>> 2016-11-29 Wilco Dijkstra <wdijkstr@arm.com>
>>
>> * config/arm/vfp.md (movdi_vfp): Merge changes from movdi_vfp_cortexa8.
>> * (movdi_vfp_cortexa8): Remove pattern.
>> --
>>
>> diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
>> index 2051f1018f1cbff9c5bf044e71304d78e615458e..a917aa625a7b15f6c9e2b549ab22e5219bb9b99c 100644
>> --- a/gcc/config/arm/vfp.md
>> +++ b/gcc/config/arm/vfp.md
>> @@ -304,9 +304,9 @@
>> ;; DImode moves
>>
>> (define_insn "*movdi_vfp"
>> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,r,w,w, Uv")
>> + [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,q,q,m,w,!r,w,w, Uv")
>> (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,q,r,w,w,Uvi,w"))]
>> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune != TARGET_CPU_cortexa8
>> + "TARGET_32BIT && TARGET_HARD_FLOAT
>> && ( register_operand (operands[0], DImode)
>> || register_operand (operands[1], DImode))
>> && !(TARGET_NEON && CONST_INT_P (operands[1])
>> @@ -339,71 +339,25 @@
>> }
>> "
>> [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
>> - (set (attr "length") (cond [(eq_attr "alternative" "1,4,5,6") (const_int 8)
>> + (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
>> (eq_attr "alternative" "2") (const_int 12)
>> (eq_attr "alternative" "3") (const_int 16)
>> + (eq_attr "alternative" "4,5,6")
>> + (symbol_ref "arm_count_output_move_double_insns (operands) * 4")
>> (eq_attr "alternative" "9")
>> (if_then_else
>> (match_test "TARGET_VFP_SINGLE")
>> (const_int 8)
>> (const_int 4))]
>> (const_int 4)))
>> + (set_attr "predicable" "yes")
>> (set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*")
>> (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
>> (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
>> + (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4"))
>> (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
>> )
>>
>> -(define_insn "*movdi_vfp_cortexa8"
>> - [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r,r,r,r,r,r,m,w,!r,w,w, Uv")
>> - (match_operand:DI 1 "di_operand" "r,rDa,Db,Dc,mi,mi,r,r,w,w,Uvi,w"))]
>> - "TARGET_32BIT && TARGET_HARD_FLOAT && arm_tune == TARGET_CPU_cortexa8
>> - && ( register_operand (operands[0], DImode)
>> - || register_operand (operands[1], DImode))
>> - && !(TARGET_NEON && CONST_INT_P (operands[1])
>> - && neon_immediate_valid_for_move (operands[1], DImode, NULL, NULL))"
>> - "*
>> - switch (which_alternative)
>> - {
>> - case 0:
>> - case 1:
>> - case 2:
>> - case 3:
>> - return \"#\";
>> - case 4:
>> - case 5:
>> - case 6:
>> - return output_move_double (operands, true, NULL);
>> - case 7:
>> - return \"vmov%?\\t%P0, %Q1, %R1\\t%@ int\";
>> - case 8:
>> - return \"vmov%?\\t%Q0, %R0, %P1\\t%@ int\";
>> - case 9:
>> - return \"vmov%?.f64\\t%P0, %P1\\t%@ int\";
>> - case 10: case 11:
>> - return output_move_vfp (operands);
>> - default:
>> - gcc_unreachable ();
>> - }
>> - "
>> - [(set_attr "type" "multiple,multiple,multiple,multiple,load2,load2,store2,f_mcrr,f_mrrc,ffarithd,f_loadd,f_stored")
>> - (set (attr "length") (cond [(eq_attr "alternative" "1") (const_int 8)
>> - (eq_attr "alternative" "2") (const_int 12)
>> - (eq_attr "alternative" "3") (const_int 16)
>> - (eq_attr "alternative" "4,5,6")
>> - (symbol_ref
>> - "arm_count_output_move_double_insns (operands) \
>> - * 4")]
>> - (const_int 4)))
>> - (set_attr "predicable" "yes")
>> - (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
>> - (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*")
>> - (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*")
>> - (set (attr "ce_count")
>> - (symbol_ref "get_attr_length (insn) / 4"))
>> - (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")]
>> - )
>> -
>> ;; HFmode moves
>>
>> (define_insn "*movhf_vfp_fp16"
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-10-27 16:52 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-29 11:06 [PATCH][ARM] Remove movdi_vfp_cortexa8 Wilco Dijkstra
2016-12-06 15:03 ` Wilco Dijkstra
2016-12-14 16:38 ` Wilco Dijkstra
2016-12-14 16:45 ` Kyrill Tkachov
[not found] ` <AM5PR0802MB2610BA9D4EEAF7BB81A4FA26839A0@AM5PR0802MB2610.eurprd08.prod.outlook.com>
2016-12-14 17:49 ` Wilco Dijkstra
2016-12-19 9:50 ` Ramana Radhakrishnan
2016-12-19 13:09 ` Wilco Dijkstra
2017-01-17 12:11 ` Wilco Dijkstra
2017-10-16 11:30 ` Wilco Dijkstra
2017-10-27 16:53 ` Kyrill Tkachov
2017-02-02 14:48 ` Wilco Dijkstra
2017-02-23 16:57 ` Wilco Dijkstra
2017-04-20 16:37 ` Wilco Dijkstra
2017-05-05 14:27 ` Richard Earnshaw (lists)
2017-05-05 14:39 ` Wilco Dijkstra
2017-06-13 13:58 ` Wilco Dijkstra
2017-06-27 15:40 ` Wilco Dijkstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).