* [PATCH][AArch64] vec_pack_trunc_<mode> should split after register allocator
@ 2017-04-27 7:37 Hurugalawadi, Naveen
2017-05-11 5:43 ` [PING] [PATCH] [AArch64] " Hurugalawadi, Naveen
2017-07-21 16:42 ` [PATCH][AArch64] " James Greenhalgh
0 siblings, 2 replies; 11+ messages in thread
From: Hurugalawadi, Naveen @ 2017-04-27 7:37 UTC (permalink / raw)
To: gcc-patches; +Cc: James Greenhalgh, Richard Earnshaw, Marcus Shawcroft
[-- Attachment #1: Type: text/plain, Size: 690 bytes --]
Hi,
The instruction "vec_pack_trunc_<mode>" should be split after register
allocator for scheduling reasons. Currently the instruction is marked as type
multiple which means it will scheduled as single issued. However, nothing can
be scheduled with either xtn/xtn2 which is a problem in some cases.
The patch splits the instruction and fixes the issue.
Please review the patch and let me know if its okay.
Bootstrapped and Regression tested on aarch64-thunder-linux.
2017-04-27 Naveen H.S <Naveen.Hurugalawadi@cavium.com>
* config/aarch64/aarch64-simd.md
(aarch64_simd_vec_pack_trunc_hi_<mode>): New pattern.
(vec_pack_trunc_<mode>): Split the instruction pattern.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vec_pack_trunc.patch --]
[-- Type: text/x-patch; name="vec_pack_trunc.patch", Size: 2429 bytes --]
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index c462164..9b5135c 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -1278,6 +1278,18 @@
[(set_attr "type" "neon_shift_imm_narrow_q")]
)
+(define_insn "aarch64_simd_vec_pack_trunc_hi_<mode>"
+ [(set (match_operand:<VNARROWQ2> 0 "register_operand" "=w")
+ (vec_concat:<VNARROWQ2>
+ (truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
+ (vec_select:<VNARROWQ>
+ (match_operand:<VNARROWQ2> 3 "register_operand" "0")
+ (match_operand:<VNARROWQ2> 2 "vect_par_cnst_hi_half" ""))))]
+ "TARGET_SIMD"
+ "xtn2\\t%0.<V2ntype>, %1.<Vtype>"
+ [(set_attr "type" "neon_shift_imm_narrow_q")]
+)
+
(define_expand "vec_pack_trunc_<mode>"
[(match_operand:<VNARROWD> 0 "register_operand" "")
(match_operand:VDN 1 "register_operand" "")
@@ -1296,17 +1308,41 @@
;; For quads.
-(define_insn "vec_pack_trunc_<mode>"
+(define_insn_and_split "vec_pack_trunc_<mode>"
[(set (match_operand:<VNARROWQ2> 0 "register_operand" "=&w")
(vec_concat:<VNARROWQ2>
(truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
(truncate:<VNARROWQ> (match_operand:VQN 2 "register_operand" "w"))))]
"TARGET_SIMD"
+ "#"
+ "&& reload_completed"
+ [(const_int 0)]
{
if (BYTES_BIG_ENDIAN)
- return "xtn\\t%0.<Vntype>, %2.<Vtype>\;xtn2\\t%0.<V2ntype>, %1.<Vtype>";
+ {
+ rtx low_part = gen_lowpart (<VNARROWQ>mode, operands[0]);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_<mode> (low_part,
+ operands[2]));
+ rtx high_part = aarch64_simd_vect_par_cnst_half (<VNARROWQ2>mode,
+ true);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_hi_<mode> (operands[0],
+ operands[1],
+ high_part,
+ operands[0]));
+ }
else
- return "xtn\\t%0.<Vntype>, %1.<Vtype>\;xtn2\\t%0.<V2ntype>, %2.<Vtype>";
+ {
+ rtx low_part = gen_lowpart (<VNARROWQ>mode, operands[0]);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_<mode> (low_part,
+ operands[1]));
+ rtx high_part = aarch64_simd_vect_par_cnst_half (<VNARROWQ2>mode,
+ true);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_hi_<mode> (operands[0],
+ operands[2],
+ high_part,
+ operands[0]));
+ }
+ DONE;
}
[(set_attr "type" "multiple")
(set_attr "length" "8")]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PING] [PATCH] [AArch64] vec_pack_trunc_<mode> should split after register allocator
2017-04-27 7:37 [PATCH][AArch64] vec_pack_trunc_<mode> should split after register allocator Hurugalawadi, Naveen
@ 2017-05-11 5:43 ` Hurugalawadi, Naveen
2017-05-26 6:27 ` [PING 2] " Hurugalawadi, Naveen
2017-07-21 16:42 ` [PATCH][AArch64] " James Greenhalgh
1 sibling, 1 reply; 11+ messages in thread
From: Hurugalawadi, Naveen @ 2017-05-11 5:43 UTC (permalink / raw)
To: gcc-patches; +Cc: James Greenhalgh, Richard Earnshaw, Marcus Shawcroft
Hi,
Please consider this as a personal reminder to review the patch
at following link and let me know your comments on the same.
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01334.html
Thanks,
Naveen
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][AArch64] vec_pack_trunc_<mode> should split after register allocator
2017-04-27 7:37 [PATCH][AArch64] vec_pack_trunc_<mode> should split after register allocator Hurugalawadi, Naveen
2017-05-11 5:43 ` [PING] [PATCH] [AArch64] " Hurugalawadi, Naveen
@ 2017-07-21 16:42 ` James Greenhalgh
2017-07-25 7:30 ` Hurugalawadi, Naveen
1 sibling, 1 reply; 11+ messages in thread
From: James Greenhalgh @ 2017-07-21 16:42 UTC (permalink / raw)
To: Hurugalawadi, Naveen; +Cc: gcc-patches, Richard Earnshaw, Marcus Shawcroft, nd
On Thu, Apr 27, 2017 at 05:08:38AM +0000, Hurugalawadi, Naveen wrote:
> Hi,
>
> The instruction "vec_pack_trunc_<mode>" should be split after register
> allocator for scheduling reasons. Currently the instruction is marked as type
> multiple which means it will scheduled as single issued. However, nothing can
> be scheduled with either xtn/xtn2 which is a problem in some cases.
What's the reason for splitting this only after reload? I think we can
split this whenever we like, and that there isn't any benefit in keeping
the pair together?
Am I missing something?
Thanks,
James
>
> The patch splits the instruction and fixes the issue.
>
> Please review the patch and let me know if its okay.
> Bootstrapped and Regression tested on aarch64-thunder-linux.
>
> 2017-04-27 Naveen H.S <Naveen.Hurugalawadi@cavium.com>
>
> * config/aarch64/aarch64-simd.md
> (aarch64_simd_vec_pack_trunc_hi_<mode>): New pattern.
> (vec_pack_trunc_<mode>): Split the instruction pattern.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][AArch64] vec_pack_trunc_<mode> should split after register allocator
2017-07-21 16:42 ` [PATCH][AArch64] " James Greenhalgh
@ 2017-07-25 7:30 ` Hurugalawadi, Naveen
2017-07-25 8:57 ` James Greenhalgh
0 siblings, 1 reply; 11+ messages in thread
From: Hurugalawadi, Naveen @ 2017-07-25 7:30 UTC (permalink / raw)
To: James Greenhalgh
Cc: gcc-patches, Richard Earnshaw, Marcus Shawcroft, nd, Pinski, Andrew
[-- Attachment #1: Type: text/plain, Size: 350 bytes --]
Hi,
>> I think we can split this whenever we like, and
>> that there isn't any benefit in keeping the pair together?
Thanks for the review and your views.
The patch is modified as per your suggestion.
Please review the patch and let me know if its okay?
Bootstrapped and Regression done on AArch64-Thunder-Linux.
Thanks,
Naveen
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vec_pack-1.patch --]
[-- Type: text/x-diff; name="vec_pack-1.patch", Size: 2604 bytes --]
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 1cb6eeb..a41edad 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -1291,6 +1291,18 @@
[(set_attr "type" "neon_shift_imm_narrow_q")]
)
+(define_insn "aarch64_simd_vec_pack_trunc_hi_<mode>"
+ [(set (match_operand:<VNARROWQ2> 0 "register_operand" "=w")
+ (vec_concat:<VNARROWQ2>
+ (truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
+ (vec_select:<VNARROWQ>
+ (match_operand:<VNARROWQ2> 3 "register_operand" "0")
+ (match_operand:<VNARROWQ2> 2 "vect_par_cnst_hi_half" ""))))]
+ "TARGET_SIMD"
+ "xtn2\\t%0.<V2ntype>, %1.<Vtype>"
+ [(set_attr "type" "neon_shift_imm_narrow_q")]
+)
+
(define_expand "vec_pack_trunc_<mode>"
[(match_operand:<VNARROWD> 0 "register_operand" "")
(match_operand:VDN 1 "register_operand" "")
@@ -1309,20 +1321,39 @@
;; For quads.
-(define_insn "vec_pack_trunc_<mode>"
- [(set (match_operand:<VNARROWQ2> 0 "register_operand" "=&w")
+(define_expand "vec_pack_trunc_<mode>"
+ [(set (match_operand:<VNARROWQ2> 0 "register_operand" "")
(vec_concat:<VNARROWQ2>
- (truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
- (truncate:<VNARROWQ> (match_operand:VQN 2 "register_operand" "w"))))]
+ (truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" ""))
+ (truncate:<VNARROWQ> (match_operand:VQN 2 "register_operand" ""))))]
"TARGET_SIMD"
{
if (BYTES_BIG_ENDIAN)
- return "xtn\\t%0.<Vntype>, %2.<Vtype>\;xtn2\\t%0.<V2ntype>, %1.<Vtype>";
+ {
+ rtx low_part = gen_lowpart (<VNARROWQ>mode, operands[0]);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_<mode> (low_part,
+ operands[2]));
+ rtx high_part = aarch64_simd_vect_par_cnst_half (<VNARROWQ2>mode,
+ true);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_hi_<mode> (operands[0],
+ operands[1],
+ high_part,
+ operands[0]));
+ }
else
- return "xtn\\t%0.<Vntype>, %1.<Vtype>\;xtn2\\t%0.<V2ntype>, %2.<Vtype>";
+ {
+ rtx low_part = gen_lowpart (<VNARROWQ>mode, operands[0]);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_<mode> (low_part,
+ operands[1]));
+ rtx high_part = aarch64_simd_vect_par_cnst_half (<VNARROWQ2>mode,
+ true);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_hi_<mode> (operands[0],
+ operands[2],
+ high_part,
+ operands[0]));
+ }
+ DONE;
}
- [(set_attr "type" "multiple")
- (set_attr "length" "8")]
)
;; Widening operations.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH][AArch64] vec_pack_trunc_<mode> should split after register allocator
2017-07-25 7:30 ` Hurugalawadi, Naveen
@ 2017-07-25 8:57 ` James Greenhalgh
2017-07-25 11:40 ` Hurugalawadi, Naveen
0 siblings, 1 reply; 11+ messages in thread
From: James Greenhalgh @ 2017-07-25 8:57 UTC (permalink / raw)
To: Hurugalawadi, Naveen
Cc: gcc-patches, Richard Earnshaw, Marcus Shawcroft, nd, Pinski, Andrew
On Tue, Jul 25, 2017 at 07:30:49AM +0000, Hurugalawadi, Naveen wrote:
> Hi,
>
> >> I think we can split this whenever we like, and
> >> that there isn't any benefit in keeping the pair together?
>
> Thanks for the review and your views.
Thanks for the updated patch, and sorry that I haven't been clear in what
I was asking for.
I was wondering why we could not use an insn_and_split without the
reload_completed guard - there is probably still value in allowing other
optimisation passes to see that we can support:
[(set (match_operand:<VNARROWQ2> 0 "register_operand" "")
(vec_concat:<VNARROWQ2>
(truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
(truncate:<VNARROWQ> (match_operand:VQN 2 "register_operand" "w"))))]
but an expand pattern is not going to permit that.
Could you switch this back to an insn_and_split as it was in the previous
patch, and just drop the && reload_completed ?
Thanks,
James
>
> The patch is modified as per your suggestion.
>
> Please review the patch and let me know if its okay?
>
> Bootstrapped and Regression done on AArch64-Thunder-Linux.
>
> Thanks,
> Naveen
> diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
> index 1cb6eeb..a41edad 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -1291,6 +1291,18 @@
> [(set_attr "type" "neon_shift_imm_narrow_q")]
> )
>
> +(define_insn "aarch64_simd_vec_pack_trunc_hi_<mode>"
> + [(set (match_operand:<VNARROWQ2> 0 "register_operand" "=w")
> + (vec_concat:<VNARROWQ2>
> + (truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
> + (vec_select:<VNARROWQ>
> + (match_operand:<VNARROWQ2> 3 "register_operand" "0")
> + (match_operand:<VNARROWQ2> 2 "vect_par_cnst_hi_half" ""))))]
> + "TARGET_SIMD"
> + "xtn2\\t%0.<V2ntype>, %1.<Vtype>"
> + [(set_attr "type" "neon_shift_imm_narrow_q")]
> +)
> +
> (define_expand "vec_pack_trunc_<mode>"
> [(match_operand:<VNARROWD> 0 "register_operand" "")
> (match_operand:VDN 1 "register_operand" "")
> @@ -1309,20 +1321,39 @@
>
> ;; For quads.
>
> -(define_insn "vec_pack_trunc_<mode>"
> - [(set (match_operand:<VNARROWQ2> 0 "register_operand" "=&w")
> +(define_expand "vec_pack_trunc_<mode>"
> + [(set (match_operand:<VNARROWQ2> 0 "register_operand" "")
> (vec_concat:<VNARROWQ2>
> - (truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
> - (truncate:<VNARROWQ> (match_operand:VQN 2 "register_operand" "w"))))]
> + (truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" ""))
> + (truncate:<VNARROWQ> (match_operand:VQN 2 "register_operand" ""))))]
> "TARGET_SIMD"
> {
> if (BYTES_BIG_ENDIAN)
> - return "xtn\\t%0.<Vntype>, %2.<Vtype>\;xtn2\\t%0.<V2ntype>, %1.<Vtype>";
> + {
> + rtx low_part = gen_lowpart (<VNARROWQ>mode, operands[0]);
> + emit_insn (gen_aarch64_simd_vec_pack_trunc_<mode> (low_part,
> + operands[2]));
> + rtx high_part = aarch64_simd_vect_par_cnst_half (<VNARROWQ2>mode,
> + true);
> + emit_insn (gen_aarch64_simd_vec_pack_trunc_hi_<mode> (operands[0],
> + operands[1],
> + high_part,
> + operands[0]));
> + }
> else
> - return "xtn\\t%0.<Vntype>, %1.<Vtype>\;xtn2\\t%0.<V2ntype>, %2.<Vtype>";
> + {
> + rtx low_part = gen_lowpart (<VNARROWQ>mode, operands[0]);
> + emit_insn (gen_aarch64_simd_vec_pack_trunc_<mode> (low_part,
> + operands[1]));
> + rtx high_part = aarch64_simd_vect_par_cnst_half (<VNARROWQ2>mode,
> + true);
> + emit_insn (gen_aarch64_simd_vec_pack_trunc_hi_<mode> (operands[0],
> + operands[2],
> + high_part,
> + operands[0]));
> + }
> + DONE;
> }
> - [(set_attr "type" "multiple")
> - (set_attr "length" "8")]
> )
>
> ;; Widening operations.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH][AArch64] vec_pack_trunc_<mode> should split after register allocator
2017-07-25 8:57 ` James Greenhalgh
@ 2017-07-25 11:40 ` Hurugalawadi, Naveen
[not found] ` <CO2PR07MB26932740F6783B49FB886B7983890@CO2PR07MB2693.namprd07.prod.outlook.com>
0 siblings, 1 reply; 11+ messages in thread
From: Hurugalawadi, Naveen @ 2017-07-25 11:40 UTC (permalink / raw)
To: James Greenhalgh
Cc: gcc-patches, Richard Earnshaw, Marcus Shawcroft, nd, Pinski, Andrew
[-- Attachment #1: Type: text/plain, Size: 433 bytes --]
Hi,
>> I haven't been clear in what I was asking for
Sorry. We understood right with the first comment but the second
part confused us a bit :).
>> Could you switch this back to an insn_and_split as it was in the previous
>> patch, and just drop the && reload_completed ?
Done.
Bootstrapped and Regression done on AArch64-Thunder-Linux.
Please review the patch and let me know if its okay?
Thanks,
Naveen
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vec_pack-2.patch --]
[-- Type: text/x-diff; name="vec_pack-2.patch", Size: 2410 bytes --]
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 1cb6eeb..0011040 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -1291,6 +1291,18 @@
[(set_attr "type" "neon_shift_imm_narrow_q")]
)
+(define_insn "aarch64_simd_vec_pack_trunc_hi_<mode>"
+ [(set (match_operand:<VNARROWQ2> 0 "register_operand" "=w")
+ (vec_concat:<VNARROWQ2>
+ (truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
+ (vec_select:<VNARROWQ>
+ (match_operand:<VNARROWQ2> 3 "register_operand" "0")
+ (match_operand:<VNARROWQ2> 2 "vect_par_cnst_hi_half" ""))))]
+ "TARGET_SIMD"
+ "xtn2\\t%0.<V2ntype>, %1.<Vtype>"
+ [(set_attr "type" "neon_shift_imm_narrow_q")]
+)
+
(define_expand "vec_pack_trunc_<mode>"
[(match_operand:<VNARROWD> 0 "register_operand" "")
(match_operand:VDN 1 "register_operand" "")
@@ -1309,17 +1321,41 @@
;; For quads.
-(define_insn "vec_pack_trunc_<mode>"
+(define_insn_and_split "vec_pack_trunc_<mode>"
[(set (match_operand:<VNARROWQ2> 0 "register_operand" "=&w")
(vec_concat:<VNARROWQ2>
(truncate:<VNARROWQ> (match_operand:VQN 1 "register_operand" "w"))
(truncate:<VNARROWQ> (match_operand:VQN 2 "register_operand" "w"))))]
"TARGET_SIMD"
+ "#"
+ ""
+ [(const_int 0)]
{
if (BYTES_BIG_ENDIAN)
- return "xtn\\t%0.<Vntype>, %2.<Vtype>\;xtn2\\t%0.<V2ntype>, %1.<Vtype>";
+ {
+ rtx low_part = gen_lowpart (<VNARROWQ>mode, operands[0]);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_<mode> (low_part,
+ operands[2]));
+ rtx high_part = aarch64_simd_vect_par_cnst_half (<VNARROWQ2>mode,
+ true);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_hi_<mode> (operands[0],
+ operands[1],
+ high_part,
+ operands[0]));
+ }
else
- return "xtn\\t%0.<Vntype>, %1.<Vtype>\;xtn2\\t%0.<V2ntype>, %2.<Vtype>";
+ {
+ rtx low_part = gen_lowpart (<VNARROWQ>mode, operands[0]);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_<mode> (low_part,
+ operands[1]));
+ rtx high_part = aarch64_simd_vect_par_cnst_half (<VNARROWQ2>mode,
+ true);
+ emit_insn (gen_aarch64_simd_vec_pack_trunc_hi_<mode> (operands[0],
+ operands[2],
+ high_part,
+ operands[0]));
+ }
+ DONE;
}
[(set_attr "type" "multiple")
(set_attr "length" "8")]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-08-11 4:56 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-27 7:37 [PATCH][AArch64] vec_pack_trunc_<mode> should split after register allocator Hurugalawadi, Naveen
2017-05-11 5:43 ` [PING] [PATCH] [AArch64] " Hurugalawadi, Naveen
2017-05-26 6:27 ` [PING 2] " Hurugalawadi, Naveen
2017-06-15 3:49 ` [PING 3] " Hurugalawadi, Naveen
2017-06-30 6:29 ` [PING 4] " Hurugalawadi, Naveen
2017-07-19 2:57 ` [PING 5] " Hurugalawadi, Naveen
2017-07-21 16:42 ` [PATCH][AArch64] " James Greenhalgh
2017-07-25 7:30 ` Hurugalawadi, Naveen
2017-07-25 8:57 ` James Greenhalgh
2017-07-25 11:40 ` Hurugalawadi, Naveen
[not found] ` <CO2PR07MB26932740F6783B49FB886B7983890@CO2PR07MB2693.namprd07.prod.outlook.com>
2017-08-11 6:09 ` [PING] [PATCH] [AArch64] " Hurugalawadi, Naveen
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).