From: Matthew Malcomson <matthew.malcomson@arm.com>
To: Sudakshina Das <sudi.das@arm.com>,
gcc-patches@gcc.gnu.org, richard.earnshaw@arm.com,
james.greenhalgh@arm.com, marcus.shawcroft@arm.com,
richard.sandiford@arm.com
Cc: nd@arm.com
Subject: Re: [PATCH][GCC][AARCH64] Canonicalize aarch64 widening simd plus insns
Date: Thu, 19 Jul 2018 12:35:00 -0000 [thread overview]
Message-ID: <22844196-5cd4-d456-4194-14ced88f642e@arm.com> (raw)
In-Reply-To: <030aafb9-ad3c-422f-c20d-1e407068a786@arm.com>
[-- Attachment #1: Type: text/plain, Size: 3292 bytes --]
Hi again.
Providing an updated patch to include the formatting suggestions.
Thanks,
Matthew
On 12/07/18 11:39, Sudakshina Das wrote:
> Hi Matthew
>
> On 12/07/18 11:18, Richard Sandiford wrote:
>> Looks good to me FWIW (not a maintainer), just a minor formatting thing:
>>
>> Matthew Malcomson <matthew.malcomson@arm.com> writes:
>>> diff --git a/gcc/config/aarch64/aarch64-simd.md
>>> b/gcc/config/aarch64/aarch64-simd.md
>>> index
>>> aac5fa146ed8dde4507a0eb4ad6a07ce78d2f0cd..67b29cbe2cad91e031ee23be656ec61a403f2cf9
>>> 100644
>>> --- a/gcc/config/aarch64/aarch64-simd.md
>>> +++ b/gcc/config/aarch64/aarch64-simd.md
>>> @@ -3302,38 +3302,78 @@
>>> Â Â Â DONE;
>>> Â })
>>> Â -(define_insn "aarch64_<ANY_EXTEND:su><ADDSUB:optab>w<mode>"
>>> +(define_insn "aarch64_<ANY_EXTEND:su>subw<mode>"
>>> Â Â Â [(set (match_operand:<VWIDE> 0 "register_operand" "=w")
>>> -Â Â Â Â Â Â Â (ADDSUB:<VWIDE> (match_operand:<VWIDE> 1 "register_operand"
>>> "w")
>>> -Â Â Â Â Â Â Â Â Â Â Â (ANY_EXTEND:<VWIDE>
>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:VD_BHSI 2 "register_operand" "w"))))]
>>> +Â Â Â Â Â Â Â Â Â Â Â (minus:<VWIDE>
>>> +Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:<VWIDE> 1 "register_operand" "w")
>>> +Â Â Â Â Â Â Â Â Â Â Â Â (ANY_EXTEND:<VWIDE>
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:VD_BHSI 2 "register_operand" "w"))))]
>>
>> The (minus should be under the "(match_operand":
>>
>> (define_insn "aarch64_<ANY_EXTEND:su>subw<mode>"
>> Â Â [(set (match_operand:<VWIDE> 0 "register_operand" "=w")
>> Â Â Â Â (minus:<VWIDE> (match_operand:<VWIDE> 1 "register_operand" "w")
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â (ANY_EXTEND:<VWIDE>
>> Â Â Â Â Â Â Â Â Â Â Â Â (match_operand:VD_BHSI 2 "register_operand" "w"))))]
>>
>> Same for the other patterns.
>>
>> Thanks,
>> Richard
>>
>
> You will need a maintainer's approval but this looks good to me.
> Thanks for doing this. I would only point out one other nit which you
> can choose to ignore:
>
> +/* Ensure
> +Â Â saddw2 and one saddw for the function add()
> +Â Â ssubw2 and one ssubw for the function subtract()
> +Â Â uaddw2 and one uaddw for the function uadd()
> +Â Â usubw2 and one usubw for the function usubtract() */
> +
> +/* { dg-final { scan-assembler-times "\[ \t\]ssubw2\[ \t\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "\[ \t\]ssubw\[ \t\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "\[ \t\]saddw2\[ \t\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "\[ \t\]saddw\[ \t\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "\[ \t\]usubw2\[ \t\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "\[ \t\]usubw\[ \t\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "\[ \t\]uaddw2\[ \t\]+" 1 } } */
> +/* { dg-final { scan-assembler-times "\[ \t\]uaddw\[ \t\]+" 1 } } */
>
> The scan-assembly directives for the different
> functions can be placed right below each of them and that would
> make it easier to read the expected results in the test and you
> can get rid of the comments saying the same.
>
> Thanks
> Sudi
[-- Attachment #2: canonicalize-aarch64-widening-plus-simd.patch --]
[-- Type: text/x-patch, Size: 5728 bytes --]
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index b5c551ad650e1a83416d5fbbbdd38e3fa3beb532..1f356d04d5b3542ac9ce51bc315c81d1fff91f21 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -3303,38 +3303,74 @@
DONE;
})
-(define_insn "aarch64_<ANY_EXTEND:su><ADDSUB:optab>w<mode>"
+(define_insn "aarch64_<ANY_EXTEND:su>subw<mode>"
[(set (match_operand:<VWIDE> 0 "register_operand" "=w")
- (ADDSUB:<VWIDE> (match_operand:<VWIDE> 1 "register_operand" "w")
- (ANY_EXTEND:<VWIDE>
- (match_operand:VD_BHSI 2 "register_operand" "w"))))]
+ (minus:<VWIDE> (match_operand:<VWIDE> 1 "register_operand" "w")
+ (ANY_EXTEND:<VWIDE>
+ (match_operand:VD_BHSI 2 "register_operand" "w"))))]
"TARGET_SIMD"
- "<ANY_EXTEND:su><ADDSUB:optab>w\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vtype>"
- [(set_attr "type" "neon_<ADDSUB:optab>_widen")]
+ "<ANY_EXTEND:su>subw\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vtype>"
+ [(set_attr "type" "neon_sub_widen")]
)
-(define_insn "aarch64_<ANY_EXTEND:su><ADDSUB:optab>w<mode>_internal"
+(define_insn "aarch64_<ANY_EXTEND:su>subw<mode>_internal"
[(set (match_operand:<VWIDE> 0 "register_operand" "=w")
- (ADDSUB:<VWIDE> (match_operand:<VWIDE> 1 "register_operand" "w")
- (ANY_EXTEND:<VWIDE>
- (vec_select:<VHALF>
- (match_operand:VQW 2 "register_operand" "w")
- (match_operand:VQW 3 "vect_par_cnst_lo_half" "")))))]
+ (minus:<VWIDE> (match_operand:<VWIDE> 1 "register_operand" "w")
+ (ANY_EXTEND:<VWIDE>
+ (vec_select:<VHALF>
+ (match_operand:VQW 2 "register_operand" "w")
+ (match_operand:VQW 3 "vect_par_cnst_lo_half" "")))))]
"TARGET_SIMD"
- "<ANY_EXTEND:su><ADDSUB:optab>w\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vhalftype>"
- [(set_attr "type" "neon_<ADDSUB:optab>_widen")]
+ "<ANY_EXTEND:su>subw\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vhalftype>"
+ [(set_attr "type" "neon_sub_widen")]
)
-(define_insn "aarch64_<ANY_EXTEND:su><ADDSUB:optab>w2<mode>_internal"
+(define_insn "aarch64_<ANY_EXTEND:su>subw2<mode>_internal"
[(set (match_operand:<VWIDE> 0 "register_operand" "=w")
- (ADDSUB:<VWIDE> (match_operand:<VWIDE> 1 "register_operand" "w")
- (ANY_EXTEND:<VWIDE>
- (vec_select:<VHALF>
- (match_operand:VQW 2 "register_operand" "w")
- (match_operand:VQW 3 "vect_par_cnst_hi_half" "")))))]
+ (minus:<VWIDE> (match_operand:<VWIDE> 1 "register_operand" "w")
+ (ANY_EXTEND:<VWIDE>
+ (vec_select:<VHALF>
+ (match_operand:VQW 2 "register_operand" "w")
+ (match_operand:VQW 3 "vect_par_cnst_hi_half" "")))))]
"TARGET_SIMD"
- "<ANY_EXTEND:su><ADDSUB:optab>w2\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vtype>"
- [(set_attr "type" "neon_<ADDSUB:optab>_widen")]
+ "<ANY_EXTEND:su>subw2\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vtype>"
+ [(set_attr "type" "neon_sub_widen")]
+)
+
+(define_insn "aarch64_<ANY_EXTEND:su>addw<mode>"
+ [(set (match_operand:<VWIDE> 0 "register_operand" "=w")
+ (plus:<VWIDE>
+ (ANY_EXTEND:<VWIDE> (match_operand:VD_BHSI 2 "register_operand" "w"))
+ (match_operand:<VWIDE> 1 "register_operand" "w")))]
+ "TARGET_SIMD"
+ "<ANY_EXTEND:su>addw\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vtype>"
+ [(set_attr "type" "neon_add_widen")]
+)
+
+(define_insn "aarch64_<ANY_EXTEND:su>addw<mode>_internal"
+ [(set (match_operand:<VWIDE> 0 "register_operand" "=w")
+ (plus:<VWIDE>
+ (ANY_EXTEND:<VWIDE>
+ (vec_select:<VHALF>
+ (match_operand:VQW 2 "register_operand" "w")
+ (match_operand:VQW 3 "vect_par_cnst_lo_half" "")))
+ (match_operand:<VWIDE> 1 "register_operand" "w")))]
+ "TARGET_SIMD"
+ "<ANY_EXTEND:su>addw\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vhalftype>"
+ [(set_attr "type" "neon_add_widen")]
+)
+
+(define_insn "aarch64_<ANY_EXTEND:su>addw2<mode>_internal"
+ [(set (match_operand:<VWIDE> 0 "register_operand" "=w")
+ (plus:<VWIDE>
+ (ANY_EXTEND:<VWIDE>
+ (vec_select:<VHALF>
+ (match_operand:VQW 2 "register_operand" "w")
+ (match_operand:VQW 3 "vect_par_cnst_hi_half" "")))
+ (match_operand:<VWIDE> 1 "register_operand" "w")))]
+ "TARGET_SIMD"
+ "<ANY_EXTEND:su>addw2\\t%0.<Vwtype>, %1.<Vwtype>, %2.<Vtype>"
+ [(set_attr "type" "neon_add_widen")]
)
(define_expand "aarch64_saddw2<mode>"
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vect_su_add_sub.c b/gcc/testsuite/gcc.target/aarch64/simd/vect_su_add_sub.c
new file mode 100644
index 0000000000000000000000000000000000000000..338da54f6281c90e1c6b1c59fa50d9b719005c77
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/vect_su_add_sub.c
@@ -0,0 +1,49 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+/* Ensure we use the signed/unsigned extend vectorized add and sub
+ instructions. */
+#define N 1024
+
+int a[N];
+long c[N];
+long d[N];
+unsigned int ua[N];
+unsigned long uc[N];
+unsigned long ud[N];
+
+void
+add ()
+{
+ for (int i = 0; i < N; i++)
+ d[i] = a[i] + c[i];
+}
+/* { dg-final { scan-assembler-times "\[ \t\]saddw2\[ \t\]+" 1 } } */
+/* { dg-final { scan-assembler-times "\[ \t\]saddw\[ \t\]+" 1 } } */
+
+void
+subtract ()
+{
+ for (int i = 0; i < N; i++)
+ d[i] = c[i] - a[i];
+}
+/* { dg-final { scan-assembler-times "\[ \t\]ssubw2\[ \t\]+" 1 } } */
+/* { dg-final { scan-assembler-times "\[ \t\]ssubw\[ \t\]+" 1 } } */
+
+void
+uadd ()
+{
+ for (int i = 0; i < N; i++)
+ ud[i] = ua[i] + uc[i];
+}
+/* { dg-final { scan-assembler-times "\[ \t\]uaddw2\[ \t\]+" 1 } } */
+/* { dg-final { scan-assembler-times "\[ \t\]uaddw\[ \t\]+" 1 } } */
+
+void
+usubtract ()
+{
+ for (int i = 0; i < N; i++)
+ ud[i] = uc[i] - ua[i];
+}
+/* { dg-final { scan-assembler-times "\[ \t\]usubw2\[ \t\]+" 1 } } */
+/* { dg-final { scan-assembler-times "\[ \t\]usubw\[ \t\]+" 1 } } */
next prev parent reply other threads:[~2018-07-19 12:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-11 10:36 Matthew Malcomson
2018-07-12 10:18 ` Richard Sandiford
2018-07-12 10:39 ` Sudakshina Das
2018-07-19 12:35 ` Matthew Malcomson [this message]
2018-07-24 15:12 ` James Greenhalgh
2018-07-24 15:39 ` Kyrill Tkachov
2018-07-30 13:30 ` Christophe Lyon
2018-07-30 13:58 ` Kyrill Tkachov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=22844196-5cd4-d456-4194-14ced88f642e@arm.com \
--to=matthew.malcomson@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=james.greenhalgh@arm.com \
--cc=marcus.shawcroft@arm.com \
--cc=nd@arm.com \
--cc=richard.earnshaw@arm.com \
--cc=richard.sandiford@arm.com \
--cc=sudi.das@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).