* [PATCH] i386: Fix up cond_{and,ior,xor,mul}* [PR104779]
@ 2022-03-05 8:05 Jakub Jelinek
2022-03-07 2:15 ` Hongtao Liu
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2022-03-05 8:05 UTC (permalink / raw)
To: Hongtao Liu, Uros Bizjak; +Cc: gcc-patches
Hi!
The following testcase ICEs, because the cond_andv* expander
has vector_operand predicates in both of the commutative inputs
and calls gen_andv*_mask which calls ix86_binary_operator_ok
in its condition, but nothing calls ix86_fixup_binary_operands_no_copy
during the expansion, which means cond_* accepts even operands
like 2 MEMs which then can't be matched.
The following patch handles it like most other insns that the other
cond_* patterns use - by having a separate define_expand that calls
ix86_fixup_binary_operands_no_copy and define_ins with
ix86_binary_operator_ok.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Note, the predicates on cond_fma* and other FMA variants look all wrong to
me, usually the fma instructions require nonimmediate_operand operands,
but the cond_* patterns use vector_operand. Besides what this patch
fixes and the unfixed fma which I don't have spare cycles for right
now I went through all the other cond_* patterns checking for predicate
mismatches or similar missing ix86_fixup_binary_operands* issues and
didn't find other problems.
2022-03-05 Jakub Jelinek <jakub@redhat.com>
PR target/104779
* config/i386/sse.md (avx512dq_mul<mode>3<mask_name>): New
define_expand pattern. Rename define_insn to ...
(*avx512dq_mul<mode>3<mask_name>): ... this.
(<code><mode>3_mask): New any_logic define_expand pattern.
(<mask_codefor><code><mode>3<mask_name>): Rename to ...
(*<code><mode>3<mask_name>): ... this.
* gcc.target/i386/pr104779.c: New test.
--- gcc/config/i386/sse.md.jj 2022-02-24 15:27:14.722743984 +0100
+++ gcc/config/i386/sse.md 2022-03-04 13:56:34.863572916 +0100
@@ -15210,7 +15210,15 @@ (define_expand "cond_mul<mode>"
DONE;
})
-(define_insn "avx512dq_mul<mode>3<mask_name>"
+(define_expand "avx512dq_mul<mode>3<mask_name>"
+ [(set (match_operand:VI8_AVX512VL 0 "register_operand")
+ (mult:VI8_AVX512VL
+ (match_operand:VI8_AVX512VL 1 "bcst_vector_operand")
+ (match_operand:VI8_AVX512VL 2 "bcst_vector_operand")))]
+ "TARGET_AVX512DQ && <mask_mode512bit_condition>"
+ "ix86_fixup_binary_operands_no_copy (MULT, <MODE>mode, operands);")
+
+(define_insn "*avx512dq_mul<mode>3<mask_name>"
[(set (match_operand:VI8_AVX512VL 0 "register_operand" "=v")
(mult:VI8_AVX512VL
(match_operand:VI8_AVX512VL 1 "bcst_vector_operand" "%v")
@@ -16824,7 +16832,18 @@ (define_expand "cond_<code><mode>"
DONE;
})
-(define_insn "<mask_codefor><code><mode>3<mask_name>"
+(define_expand "<code><mode>3_mask"
+ [(set (match_operand:VI48_AVX512VL 0 "register_operand")
+ (vec_merge:VI48_AVX512VL
+ (any_logic:VI48_AVX512VL
+ (match_operand:VI48_AVX512VL 1 "bcst_vector_operand")
+ (match_operand:VI48_AVX512VL 2 "bcst_vector_operand"))
+ (match_operand:VI48_AVX512VL 3 "nonimm_or_0_operand")
+ (match_operand:<avx512fmaskmode> 4 "register_operand")))]
+ "TARGET_AVX512F"
+ "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
+
+(define_insn "*<code><mode>3<mask_name>"
[(set (match_operand:VI48_AVX_AVX512F 0 "register_operand" "=x,x,v")
(any_logic:VI48_AVX_AVX512F
(match_operand:VI48_AVX_AVX512F 1 "bcst_vector_operand" "%0,x,v")
--- gcc/testsuite/gcc.target/i386/pr104779.c.jj 2022-03-04 14:09:03.278961269 +0100
+++ gcc/testsuite/gcc.target/i386/pr104779.c 2022-03-04 14:08:38.063318794 +0100
@@ -0,0 +1,27 @@
+/* PR target/104779 */
+/* { dg-do compile } */
+/* { dg-options "-O1 --param sccvn-max-alias-queries-per-access=0" } */
+
+__attribute__ ((simd)) int
+foo (int x, int y, int z)
+{
+ return (x & y) * !!z;
+}
+
+__attribute__ ((simd)) int
+bar (int x, int y, int z)
+{
+ return (x | y) * !!z;
+}
+
+__attribute__ ((simd)) int
+baz (int x, int y, int z)
+{
+ return (x ^ y) * !!z;
+}
+
+__attribute__ ((simd, target ("avx512dq"))) long
+qux (long x, long y, long z)
+{
+ return (x * y) * !!z;
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] i386: Fix up cond_{and,ior,xor,mul}* [PR104779]
2022-03-05 8:05 [PATCH] i386: Fix up cond_{and,ior,xor,mul}* [PR104779] Jakub Jelinek
@ 2022-03-07 2:15 ` Hongtao Liu
2022-03-07 8:51 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Hongtao Liu @ 2022-03-07 2:15 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Uros Bizjak, GCC Patches
On Sat, Mar 5, 2022 at 4:05 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The following testcase ICEs, because the cond_andv* expander
> has vector_operand predicates in both of the commutative inputs
> and calls gen_andv*_mask which calls ix86_binary_operator_ok
> in its condition, but nothing calls ix86_fixup_binary_operands_no_copy
> during the expansion, which means cond_* accepts even operands
> like 2 MEMs which then can't be matched.
>
> The following patch handles it like most other insns that the other
> cond_* patterns use - by having a separate define_expand that calls
> ix86_fixup_binary_operands_no_copy and define_ins with
> ix86_binary_operator_ok.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
>
> Note, the predicates on cond_fma* and other FMA variants look all wrong to
> me, usually the fma instructions require nonimmediate_operand operands,
> but the cond_* patterns use vector_operand. Besides what this patch
--------cut from predicate.md---------
1142; Return true when OP is operand acceptable for vector memory operand.
1143; Only AVX can have misaligned memory operand.
1144(define_predicate "vector_memory_operand"
1145 (and (match_operand 0 "memory_operand")
1146 (ior (match_test "TARGET_AVX")
1147 (match_test "MEM_ALIGN (op) >= GET_MODE_ALIGNMENT (mode)"))))
1148
1149; Return true when OP is register_operand or vector_memory_operand.
1150(define_predicate "vector_operand"
1151 (ior (match_operand 0 "register_operand")
1152 (match_operand 0 "vector_memory_operand")))
--------cut end------------------------
vector_operand is a subset of nonimmediate_operands, so it's more like
a potential optimization issue rather than a correctness one?
> fixes and the unfixed fma which I don't have spare cycles for right
> now I went through all the other cond_* patterns checking for predicate
> mismatches or similar missing ix86_fixup_binary_operands* issues and
> didn't find other problems.
>
> 2022-03-05 Jakub Jelinek <jakub@redhat.com>
>
> PR target/104779
> * config/i386/sse.md (avx512dq_mul<mode>3<mask_name>): New
> define_expand pattern. Rename define_insn to ...
> (*avx512dq_mul<mode>3<mask_name>): ... this.
> (<code><mode>3_mask): New any_logic define_expand pattern.
> (<mask_codefor><code><mode>3<mask_name>): Rename to ...
> (*<code><mode>3<mask_name>): ... this.
>
> * gcc.target/i386/pr104779.c: New test.
>
> --- gcc/config/i386/sse.md.jj 2022-02-24 15:27:14.722743984 +0100
> +++ gcc/config/i386/sse.md 2022-03-04 13:56:34.863572916 +0100
> @@ -15210,7 +15210,15 @@ (define_expand "cond_mul<mode>"
> DONE;
> })
>
> -(define_insn "avx512dq_mul<mode>3<mask_name>"
> +(define_expand "avx512dq_mul<mode>3<mask_name>"
> + [(set (match_operand:VI8_AVX512VL 0 "register_operand")
> + (mult:VI8_AVX512VL
> + (match_operand:VI8_AVX512VL 1 "bcst_vector_operand")
> + (match_operand:VI8_AVX512VL 2 "bcst_vector_operand")))]
> + "TARGET_AVX512DQ && <mask_mode512bit_condition>"
> + "ix86_fixup_binary_operands_no_copy (MULT, <MODE>mode, operands);")
> +
> +(define_insn "*avx512dq_mul<mode>3<mask_name>"
> [(set (match_operand:VI8_AVX512VL 0 "register_operand" "=v")
> (mult:VI8_AVX512VL
> (match_operand:VI8_AVX512VL 1 "bcst_vector_operand" "%v")
> @@ -16824,7 +16832,18 @@ (define_expand "cond_<code><mode>"
> DONE;
> })
>
> -(define_insn "<mask_codefor><code><mode>3<mask_name>"
> +(define_expand "<code><mode>3_mask"
> + [(set (match_operand:VI48_AVX512VL 0 "register_operand")
> + (vec_merge:VI48_AVX512VL
> + (any_logic:VI48_AVX512VL
> + (match_operand:VI48_AVX512VL 1 "bcst_vector_operand")
> + (match_operand:VI48_AVX512VL 2 "bcst_vector_operand"))
> + (match_operand:VI48_AVX512VL 3 "nonimm_or_0_operand")
> + (match_operand:<avx512fmaskmode> 4 "register_operand")))]
> + "TARGET_AVX512F"
> + "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
> +
> +(define_insn "*<code><mode>3<mask_name>"
> [(set (match_operand:VI48_AVX_AVX512F 0 "register_operand" "=x,x,v")
> (any_logic:VI48_AVX_AVX512F
> (match_operand:VI48_AVX_AVX512F 1 "bcst_vector_operand" "%0,x,v")
> --- gcc/testsuite/gcc.target/i386/pr104779.c.jj 2022-03-04 14:09:03.278961269 +0100
> +++ gcc/testsuite/gcc.target/i386/pr104779.c 2022-03-04 14:08:38.063318794 +0100
> @@ -0,0 +1,27 @@
> +/* PR target/104779 */
> +/* { dg-do compile } */
> +/* { dg-options "-O1 --param sccvn-max-alias-queries-per-access=0" } */
> +
> +__attribute__ ((simd)) int
> +foo (int x, int y, int z)
> +{
> + return (x & y) * !!z;
> +}
> +
> +__attribute__ ((simd)) int
> +bar (int x, int y, int z)
> +{
> + return (x | y) * !!z;
> +}
> +
> +__attribute__ ((simd)) int
> +baz (int x, int y, int z)
> +{
> + return (x ^ y) * !!z;
> +}
> +
> +__attribute__ ((simd, target ("avx512dq"))) long
> +qux (long x, long y, long z)
> +{
> + return (x * y) * !!z;
> +}
>
> Jakub
>
--
BR,
Hongtao
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] i386: Fix up cond_{and,ior,xor,mul}* [PR104779]
2022-03-07 2:15 ` Hongtao Liu
@ 2022-03-07 8:51 ` Jakub Jelinek
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2022-03-07 8:51 UTC (permalink / raw)
To: Hongtao Liu; +Cc: Uros Bizjak, GCC Patches
On Mon, Mar 07, 2022 at 10:15:48AM +0800, Hongtao Liu wrote:
> > Note, the predicates on cond_fma* and other FMA variants look all wrong to
> > me, usually the fma instructions require nonimmediate_operand operands,
> > but the cond_* patterns use vector_operand. Besides what this patch
> --------cut from predicate.md---------
> 1142; Return true when OP is operand acceptable for vector memory operand.
> 1143; Only AVX can have misaligned memory operand.
> 1144(define_predicate "vector_memory_operand"
> 1145 (and (match_operand 0 "memory_operand")
> 1146 (ior (match_test "TARGET_AVX")
> 1147 (match_test "MEM_ALIGN (op) >= GET_MODE_ALIGNMENT (mode)"))))
> 1148
> 1149; Return true when OP is register_operand or vector_memory_operand.
> 1150(define_predicate "vector_operand"
> 1151 (ior (match_operand 0 "register_operand")
> 1152 (match_operand 0 "vector_memory_operand")))
> --------cut end------------------------
>
> vector_operand is a subset of nonimmediate_operands, so it's more like
> a potential optimization issue rather than a correctness one?
You're right.
And fma apparently doesn't use something like ix86_fixup_binary_operands*,
allows all 3 inputs as MEMs and only deals with it at RA time.
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-03-07 8:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-05 8:05 [PATCH] i386: Fix up cond_{and,ior,xor,mul}* [PR104779] Jakub Jelinek
2022-03-07 2:15 ` Hongtao Liu
2022-03-07 8:51 ` Jakub Jelinek
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).