public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] x86: {,v}psadbw have commutative source operands
@ 2022-05-27  8:13 Jan Beulich
  2022-05-27  9:05 ` Uros Bizjak
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2022-05-27  8:13 UTC (permalink / raw)
  To: gcc-patches; +Cc: hubicka, ubizjak, Kirill Yukhin, Hongtao Liu

Like noticed for gas as well (binutils-gdb commit c8cad9d389b7), the
"absolute difference" aspect of the insns makes their source operands
commutative.

gcc/
2022-05-XX  Jan Beulich  <jbeulich@suse.com>

	* config/i386/mmx.md (mmx_psadbw): Mark as commutative.
	* config/i386/sse.md (<sse2_avx2>_psadbw): Likewise.

--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -4407,7 +4407,7 @@
 
 (define_insn "mmx_psadbw"
   [(set (match_operand:V1DI 0 "register_operand" "=y,x,Yw")
-        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "0,0,Yw")
+        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "%0,0,Yw")
 		      (match_operand:V8QI 2 "register_mmxmem_operand" "ym,x,Yw")]
 		     UNSPEC_PSADBW))]
   "(TARGET_MMX || TARGET_MMX_WITH_SSE)
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -19983,7 +19983,7 @@
 (define_insn "<sse2_avx2>_psadbw"
   [(set (match_operand:VI8_AVX2_AVX512BW 0 "register_operand" "=x,YW")
 	(unspec:VI8_AVX2_AVX512BW
-	  [(match_operand:<ssebytemode> 1 "register_operand" "0,YW")
+	  [(match_operand:<ssebytemode> 1 "register_operand" "%0,YW")
 	   (match_operand:<ssebytemode> 2 "vector_operand" "xBm,YWm")]
 	  UNSPEC_PSADBW))]
   "TARGET_SSE2"


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: {,v}psadbw have commutative source operands
  2022-05-27  8:13 [PATCH] x86: {,v}psadbw have commutative source operands Jan Beulich
@ 2022-05-27  9:05 ` Uros Bizjak
  2022-05-30  7:59   ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Uros Bizjak @ 2022-05-27  9:05 UTC (permalink / raw)
  To: Jan Beulich; +Cc: gcc-patches, hubicka, Kirill Yukhin, Hongtao Liu

On Fri, May 27, 2022 at 10:13 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Like noticed for gas as well (binutils-gdb commit c8cad9d389b7), the
> "absolute difference" aspect of the insns makes their source operands
> commutative.

You will need to expand via ix86_fixup_binary_operands_no_copy, use
register_mmxmem_operand on both input operands and use
ix86_binary_operator insn constraint. Please see many examples w/
commutative operands throughout .md files.

Uros.

> gcc/
> 2022-05-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/i386/mmx.md (mmx_psadbw): Mark as commutative.
>         * config/i386/sse.md (<sse2_avx2>_psadbw): Likewise.
>
> --- a/gcc/config/i386/mmx.md
> +++ b/gcc/config/i386/mmx.md
> @@ -4407,7 +4407,7 @@
>
>  (define_insn "mmx_psadbw"
>    [(set (match_operand:V1DI 0 "register_operand" "=y,x,Yw")
> -        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "0,0,Yw")
> +        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "%0,0,Yw")
>                       (match_operand:V8QI 2 "register_mmxmem_operand" "ym,x,Yw")]
>                      UNSPEC_PSADBW))]
>    "(TARGET_MMX || TARGET_MMX_WITH_SSE)
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -19983,7 +19983,7 @@
>  (define_insn "<sse2_avx2>_psadbw"
>    [(set (match_operand:VI8_AVX2_AVX512BW 0 "register_operand" "=x,YW")
>         (unspec:VI8_AVX2_AVX512BW
> -         [(match_operand:<ssebytemode> 1 "register_operand" "0,YW")
> +         [(match_operand:<ssebytemode> 1 "register_operand" "%0,YW")
>            (match_operand:<ssebytemode> 2 "vector_operand" "xBm,YWm")]
>           UNSPEC_PSADBW))]
>    "TARGET_SSE2"
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: {,v}psadbw have commutative source operands
  2022-05-27  9:05 ` Uros Bizjak
@ 2022-05-30  7:59   ` Jan Beulich
  2022-05-30  8:28     ` Uros Bizjak
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2022-05-30  7:59 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches, hubicka, Kirill Yukhin, Hongtao Liu

On 27.05.2022 11:05, Uros Bizjak wrote:
> On Fri, May 27, 2022 at 10:13 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> Like noticed for gas as well (binutils-gdb commit c8cad9d389b7), the
>> "absolute difference" aspect of the insns makes their source operands
>> commutative.
> 
> You will need to expand via ix86_fixup_binary_operands_no_copy, use
> register_mmxmem_operand on both input operands and use
> ix86_binary_operator insn constraint. Please see many examples w/
> commutative operands throughout .md files.

Hmm, yes, I see. As to the use of ix86_binary_operator_ok(): In
particular in sse.md I see many uses of
ix86_fixup_binary_operands_no_copy() in expanders where the
corresponding insns don't use ix86_binary_operator_ok(), e.g. the
immediately preceding uavg. Is there a(n) (anti-)pattern?

My simplistic initial version was based on observations while
putting together the inverse change for
vgf2p8affine{,inv}qb_<mode><mask_name> (commit c0569d342ca4), which
aren't commutative. Are you suggesting that the remaining (for indeed
being commutative) vgf2p8mulb_<mode><mask_name> also is incomplete,
requiring an expander as well? And maybe the same then in
<code>v1ti3 for any_logic:V1TI, avx512bw_umulhrswv32hi3<mask_name>,
or <sse4_1>_dp<ssemodesuffix><avxsizesuffix> (and likely a few more)?

At least a few pmadd* appear to lack commutativity marking altogether.

Jan

>> --- a/gcc/config/i386/mmx.md
>> +++ b/gcc/config/i386/mmx.md
>> @@ -4407,7 +4407,7 @@
>>
>>  (define_insn "mmx_psadbw"
>>    [(set (match_operand:V1DI 0 "register_operand" "=y,x,Yw")
>> -        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "0,0,Yw")
>> +        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "%0,0,Yw")
>>                       (match_operand:V8QI 2 "register_mmxmem_operand" "ym,x,Yw")]
>>                      UNSPEC_PSADBW))]
>>    "(TARGET_MMX || TARGET_MMX_WITH_SSE)
>> --- a/gcc/config/i386/sse.md
>> +++ b/gcc/config/i386/sse.md
>> @@ -19983,7 +19983,7 @@
>>  (define_insn "<sse2_avx2>_psadbw"
>>    [(set (match_operand:VI8_AVX2_AVX512BW 0 "register_operand" "=x,YW")
>>         (unspec:VI8_AVX2_AVX512BW
>> -         [(match_operand:<ssebytemode> 1 "register_operand" "0,YW")
>> +         [(match_operand:<ssebytemode> 1 "register_operand" "%0,YW")
>>            (match_operand:<ssebytemode> 2 "vector_operand" "xBm,YWm")]
>>           UNSPEC_PSADBW))]
>>    "TARGET_SSE2"
>>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: {,v}psadbw have commutative source operands
  2022-05-30  7:59   ` Jan Beulich
@ 2022-05-30  8:28     ` Uros Bizjak
  0 siblings, 0 replies; 4+ messages in thread
From: Uros Bizjak @ 2022-05-30  8:28 UTC (permalink / raw)
  To: Jan Beulich; +Cc: gcc-patches, hubicka, Kirill Yukhin, Hongtao Liu

On Mon, May 30, 2022 at 9:59 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 27.05.2022 11:05, Uros Bizjak wrote:
> > On Fri, May 27, 2022 at 10:13 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> Like noticed for gas as well (binutils-gdb commit c8cad9d389b7), the
> >> "absolute difference" aspect of the insns makes their source operands
> >> commutative.
> >
> > You will need to expand via ix86_fixup_binary_operands_no_copy, use
> > register_mmxmem_operand on both input operands and use
> > ix86_binary_operator insn constraint. Please see many examples w/
> > commutative operands throughout .md files.
>
> Hmm, yes, I see. As to the use of ix86_binary_operator_ok(): In
> particular in sse.md I see many uses of
> ix86_fixup_binary_operands_no_copy() in expanders where the
> corresponding insns don't use ix86_binary_operator_ok(), e.g. the
> immediately preceding uavg. Is there a(n) (anti-)pattern?

ix86_fixup_binary_operands was historically used with destructive
two-operand instructions (where one input operand is tied with output
operand). It fixed up expansion to help register allocator a bit, and
brought expansion to the form of two-operand instruction, especially
with memory and immediate operands.

Please note that nowadays RA can fix up the operands by itself, but it
is still beneficial to have e.g. memory operation in the right place
from the beginning.

> My simplistic initial version was based on observations while
> putting together the inverse change for
> vgf2p8affine{,inv}qb_<mode><mask_name> (commit c0569d342ca4), which
> aren't commutative. Are you suggesting that the remaining (for indeed
> being commutative) vgf2p8mulb_<mode><mask_name> also is incomplete,
> requiring an expander as well? And maybe the same then in
> <code>v1ti3 for any_logic:V1TI, avx512bw_umulhrswv32hi3<mask_name>,
> or <sse4_1>_dp<ssemodesuffix><avxsizesuffix> (and likely a few more)?
>
> At least a few pmadd* appear to lack commutativity marking altogether.

Yes, sse.md could use some TLC in this area. I remember doing a review
of these patterns in i386.md a while ago.

Uros.

> Jan
>
> >> --- a/gcc/config/i386/mmx.md
> >> +++ b/gcc/config/i386/mmx.md
> >> @@ -4407,7 +4407,7 @@
> >>
> >>  (define_insn "mmx_psadbw"
> >>    [(set (match_operand:V1DI 0 "register_operand" "=y,x,Yw")
> >> -        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "0,0,Yw")
> >> +        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "%0,0,Yw")
> >>                       (match_operand:V8QI 2 "register_mmxmem_operand" "ym,x,Yw")]
> >>                      UNSPEC_PSADBW))]
> >>    "(TARGET_MMX || TARGET_MMX_WITH_SSE)
> >> --- a/gcc/config/i386/sse.md
> >> +++ b/gcc/config/i386/sse.md
> >> @@ -19983,7 +19983,7 @@
> >>  (define_insn "<sse2_avx2>_psadbw"
> >>    [(set (match_operand:VI8_AVX2_AVX512BW 0 "register_operand" "=x,YW")
> >>         (unspec:VI8_AVX2_AVX512BW
> >> -         [(match_operand:<ssebytemode> 1 "register_operand" "0,YW")
> >> +         [(match_operand:<ssebytemode> 1 "register_operand" "%0,YW")
> >>            (match_operand:<ssebytemode> 2 "vector_operand" "xBm,YWm")]
> >>           UNSPEC_PSADBW))]
> >>    "TARGET_SSE2"
> >>
> >
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-05-30  8:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27  8:13 [PATCH] x86: {,v}psadbw have commutative source operands Jan Beulich
2022-05-27  9:05 ` Uros Bizjak
2022-05-30  7:59   ` Jan Beulich
2022-05-30  8:28     ` Uros Bizjak

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).