The AVX-512 masked epilogue emits a masked load (UNSPEC_MASKLOAD under
a VEC_MERGE) feeding a masked binary op; the wrapper hides the memory
operand from combine, so it is not folded into the operation.
Add define_insn_and_split patterns that strip the wrapper. The masked
off lanes of the load are zero, so the correctness of a fold depends on
the value of the result lanes that are masked off:
- When an outer mask is present, the whole result is wrapped in
vec_merge (op (...), const0, mask), so the off lanes are forced to
zero regardless of the operator. Both a commutative and a
non-commutative pattern are provided for this case and accept every
operator.
- When there is no outer mask, the fold forces the result off lanes to
zero, which only matches the operator when 0 op x == 0. This holds
for mult, and and umin, so the unmasked commutative pattern is
restricted to those via the new zero_absorbing_operator
predicate. No unmasked non-commutative pattern is provided: for
minus x - 0 == x and for div x / 0 is undefined, so neither is safe.
Iterator V1248FH_AVX512VLBW covers all maskable modes.
gcc/ChangeLog:
PR target/123997
* config/i386/predicates.md (sub_div_operator):
New predicate.
(zero_absorbing_operator): New predicate.
* config/i386/sse.md (V1248FH_AVX512VLBW): New mode iterator.
(*comm_maskload_fold_unmasked_op): New pattern.
(*comm_maskload_fold_masked_op): New pattern.
(*noncomm_maskload_fold_masked_op): New pattern.
gcc/testsuite/ChangeLog:
PR target/123997
* gcc.target/i386/avx512-maskload-fold-1.c: New test.
* gcc.target/i386/avx512-maskload-fold-2.c: New test.
Co-authored-by: Venkataramanan Kumar <[email protected]>
Signed-off-by: Sarvesh Chandra <[email protected]>
---
Hi Hongtao,
Post our discussion at the Cauldron, where we discussed how vec_merge
(op ((mem), reg), 0, k) already has matching patterns in sse.md, doesn't
need a vec_merge (op (UNSPEC(mem), reg), 0, k) and a mem->reg conversion
would require a load so the match would always succeed, and safety is
guaranteed by the EVEX standard for these select instructions, I feel
this is the minimal patch that enforces both correctness and compactness
without an explosion of patterns. The more complex cases like vpavgb I
feel could be handled in GCC 18, through a combiner patch.
Regards,
Sarvesh
> Rather than describe what I want, let me show it. Below is a two-pattern
> demo I built and tested on trunk. It is FP add only -- deliberately
> minimal, not a patch -- but it demonstrates the structure I would like v2
> to follow: keep UNSPEC_MASKLOAD, no target hook, no op-specific pattern
> explosion.
>
> The key point is that you do not need to strip the UNSPEC to fold the
> memory operand. Combine already hands recog the folded form; all that is
> missing is a define_insn of that shape.
>
> 1. The form the PR needs.
> --------------------------
>
> The MEM sits *inside* the UNSPEC and carries the "m" constraint, exactly
> as *<avx512>_load<mode>_mask already does. That is what makes the RA
> question disappear: LRA never sees a bare MEM in an operator slot, so
> there is nothing for it to reload, and the UNSPEC still records that the
> access is masked so nothing downstream can widen it.
>
> (define_insn "*add<mode>3_maskload_fold_maskz"
> [(set (match_operand:VFH_AVX512VL 0 "register_operand" "=v")
> (vec_merge:VFH_AVX512VL
> (plus:VFH_AVX512VL
> (unspec:VFH_AVX512VL
> [(match_operand:VFH_AVX512VL 1 "memory_operand" "m")]
> UNSPEC_MASKLOAD)
> (match_operand:VFH_AVX512VL 2 "register_operand" "v"))
> (match_operand:VFH_AVX512VL 3 "const0_operand")
> (match_operand:<avx512fmaskmode> 4 "register_operand" "Yk")))]
> "TARGET_AVX512F"
> "vadd<ssemodesuffix>\t{%1, %2, %0%{%4%}%{z%}|%0%{%4%}%{z%}, %2, %1}"
> [(set_attr "type" "sseadd")
> (set_attr "prefix" "evex")
> (set_attr "mode" "<MODE>")])
>
> 2. Bridging what combine actually produces.
> --------------------------------------------
>
> Combine does not hand us the flat form above. It hands us the nested one,
> with the inner vec_merge still in place, because simplify_merge_mask
> refuses to strip it for a trapping MEM. From the -1.c dump on trunk:
>
> Failed to match this instruction:
> (set (reg:V8DF 133)
> (vec_merge:V8DF (plus:V8DF (vec_merge:V8DF (unspec:V8DF [
> (mem:V8DF (plus:DI (mult:DI (reg:DI 129)
> (const_int 8))
> (reg/v/f:DI 120 [ a ])) [1 S64 A64])
> ] UNSPEC_MASKLOAD)
> (const_vector:V8DF [(const_double 0.0) repeated x8])
> (reg:QI 118))
> (reg:V8DF 134))
> (const_vector:V8DF [(const_double 0.0) repeated x8])
> (reg:QI 118)))
>
> Since match_dup 3 forces both masks to be the same register, the outer
> mask already zeroes the off lanes and the inner merge is redundant. So
> match that shape and split to the pattern above:
>
> (define_insn_and_split "*add<mode>3_maskload_fold_nested"
> [(set (match_operand:VFH_AVX512VL 0 "register_operand")
> (vec_merge:VFH_AVX512VL
> (plus:VFH_AVX512VL
> (vec_merge:VFH_AVX512VL
> (unspec:VFH_AVX512VL
> [(match_operand:VFH_AVX512VL 1 "memory_operand")]
> UNSPEC_MASKLOAD)
> (match_operand:VFH_AVX512VL 2 "const0_operand")
> (match_operand:<avx512fmaskmode> 3 "register_operand"))
> (match_operand:VFH_AVX512VL 4 "register_operand"))
> (match_operand:VFH_AVX512VL 5 "const0_operand")
> (match_dup 3)))]
> "TARGET_AVX512F && ix86_pre_reload_split ()"
> "#"
> "&& 1"
> [(set (match_dup 0)
> (vec_merge:VFH_AVX512VL
> (plus:VFH_AVX512VL
> (unspec:VFH_AVX512VL [(match_dup 1)] UNSPEC_MASKLOAD)
> (match_dup 4))
> (match_dup 5)
> (match_dup 3)))])
>
> Note the split *keeps* the UNSPEC. That is the whole difference from your
> version: we remove the redundant inner mask, not the record that the load
> is masked.
>
> 3. Result.
> -----------
>
> 45 lines, no other changes. avx512-maskload-fold-1.c:
>
> before: vmovupd (%r9,%rax,8), %zmm2{%k1}{z}
> vmovupd (%r8,%rax,8), %zmm1{%k1}{z}
> vaddpd %zmm1, %zmm2, %zmm0{%k1}{z}
>
> after: vmovupd (%r9,%rax,8), %zmm0{%k1}{z}
> vaddpd (%r8,%rax,8), %zmm0, %zmm0{%k1}{z}
>
> pr97642-1.c still emits no k regs, and a maskz-load + maskz-add built from
> intrinsics is unaffected.
>
> And op can be generalized to other operations, similar like your masked
> patterns.
>
> But for the those unmasked cases, GCC doesn't have per-lane analysis in the
> rtl to decide if the transformation for the unmasked patterns are valid.
> Maybe in the vectorizer always generates MASKED_LOAD + COND_OP + MASKED_STORE
> instead MASKED_LOAD + OP + MASKED_STORE. Then the masked define_insn and
> define_insn_and_split can automatically matches that.
>
> BR,
> Hongtao
gcc/config/i386/predicates.md | 10 ++
gcc/config/i386/sse.md | 96 +++++++++++++++++++
.../gcc.target/i386/avx512-maskload-fold-1.c | 32 +++++++
.../gcc.target/i386/avx512-maskload-fold-2.c | 8 ++
4 files changed, 151 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/i386/avx512-maskload-fold-1.c
create mode 100644 gcc/testsuite/gcc.target/i386/avx512-maskload-fold-2.c
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index cf3a68a9fe8..c44eba44417 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -1740,6 +1740,16 @@
(define_predicate "commutative_operator"
(match_code "plus,mult,and,ior,xor,smin,smax,umin,umax"))
+;; Return true for the non-commutative ops we fold (minus, div).
+(define_predicate "sub_div_operator"
+ (match_code "minus,div"))
+
+;; Return true for commutative operators that absorb zero, where
+;; 0 op x == 0 (mult, and, umin). Used to fold a zero-masked load into
+;; the op when there is no outer mask: the masked-off lanes stay zero.
+(define_predicate "zero_absorbing_operator"
+ (match_code "mult,and,umin"))
+
;; Return true if OP is a binary operator that can be promoted to wider mode.
(define_predicate "promotable_binary_operator"
(ior (match_code "plus,minus,and,ior,xor,ashift")
diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index 51d1e9b455a..039c8428c77 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -2792,6 +2792,102 @@
(set_attr "btver2_decode" "direct,double")
(set_attr "mode" "<MODE>")])
+;; AVX-512 modes that support masked loads.
+
+(define_mode_iterator V1248FH_AVX512VLBW
+ [V16SF (V8SF "TARGET_AVX512VL") (V4SF "TARGET_AVX512VL")
+ V8DF (V4DF "TARGET_AVX512VL") (V2DF "TARGET_AVX512VL")
+ (V32HF "TARGET_AVX512FP16")
+ (V16HF "TARGET_AVX512FP16 && TARGET_AVX512VL")
+ (V8HF "TARGET_AVX512FP16 && TARGET_AVX512VL")
+ V16SI (V8SI "TARGET_AVX512VL") (V4SI "TARGET_AVX512VL")
+ V8DI (V4DI "TARGET_AVX512VL") (V2DI "TARGET_AVX512VL")
+ (V64QI "TARGET_AVX512BW")
+ (V32QI "TARGET_AVX512VL && TARGET_AVX512BW")
+ (V16QI "TARGET_AVX512VL && TARGET_AVX512BW")
+ (V32HI "TARGET_AVX512BW")
+ (V16HI "TARGET_AVX512VL && TARGET_AVX512BW")
+ (V8HI "TARGET_AVX512VL && TARGET_AVX512BW")])
+
+;; Fold masked load into commutative op, no outer mask. Only ops
+;; where 0 op x == 0 (mult, and, umin) are safe here: the masked-off
+;; lanes of the load are zero, and forcing the result lanes to zero
+;; matches the op only for these. Other ops need the outer-mask form.
+(define_insn_and_split "*comm_maskload_fold_unmasked_op"
+ [(set (match_operand:V1248FH_AVX512VLBW 0 "register_operand")
+ (match_operator:V1248FH_AVX512VLBW 1 "zero_absorbing_operator"
+ [(vec_merge:V1248FH_AVX512VLBW
+ (unspec:V1248FH_AVX512VLBW
+ [(match_operand:V1248FH_AVX512VLBW 2 "memory_operand")]
+ UNSPEC_MASKLOAD)
+ (match_operand:V1248FH_AVX512VLBW 3 "const0_operand")
+ (match_operand:<avx512fmaskmode> 4 "register_operand"))
+ (match_operand:V1248FH_AVX512VLBW 5 "register_operand")]))]
+ "TARGET_AVX512F"
+ "#"
+ "&& 1"
+ [(const_int 0)]
+{
+ rtx op = gen_rtx_fmt_ee (GET_CODE (operands[1]), <MODE>mode,
+ operands[5], operands[2]);
+ rtx vm = gen_rtx_VEC_MERGE (<MODE>mode, op, operands[3], operands[4]);
+ emit_insn (gen_rtx_SET (operands[0], vm));
+ DONE;
+})
+
+;; Fold masked load into commutative op, outer mask.
+(define_insn_and_split "*comm_maskload_fold_masked_op"
+ [(set (match_operand:V1248FH_AVX512VLBW 0 "register_operand")
+ (vec_merge:V1248FH_AVX512VLBW
+ (match_operator:V1248FH_AVX512VLBW 1 "commutative_operator"
+ [(vec_merge:V1248FH_AVX512VLBW
+ (unspec:V1248FH_AVX512VLBW
+ [(match_operand:V1248FH_AVX512VLBW 2 "memory_operand")]
+ UNSPEC_MASKLOAD)
+ (match_operand:V1248FH_AVX512VLBW 3 "const0_operand")
+ (match_operand:<avx512fmaskmode> 4 "register_operand"))
+ (match_operand:V1248FH_AVX512VLBW 5 "register_operand")])
+ (match_operand:V1248FH_AVX512VLBW 6 "const0_operand")
+ (match_dup 4)))]
+ "TARGET_AVX512F"
+ "#"
+ "&& 1"
+ [(const_int 0)]
+{
+ rtx op = gen_rtx_fmt_ee (GET_CODE (operands[1]), <MODE>mode,
+ operands[5], operands[2]);
+ rtx vm = gen_rtx_VEC_MERGE (<MODE>mode, op, operands[6], operands[4]);
+ emit_insn (gen_rtx_SET (operands[0], vm));
+ DONE;
+})
+
+;; Fold masked load into non-commutative op, outer mask.
+(define_insn_and_split "*noncomm_maskload_fold_masked_op"
+ [(set (match_operand:V1248FH_AVX512VLBW 0 "register_operand")
+ (vec_merge:V1248FH_AVX512VLBW
+ (match_operator:V1248FH_AVX512VLBW 1 "sub_div_operator"
+ [(match_operand:V1248FH_AVX512VLBW 5 "register_operand")
+ (vec_merge:V1248FH_AVX512VLBW
+ (unspec:V1248FH_AVX512VLBW
+ [(match_operand:V1248FH_AVX512VLBW 2 "memory_operand")]
+ UNSPEC_MASKLOAD)
+ (match_operand:V1248FH_AVX512VLBW 3 "const0_operand")
+ (match_operand:<avx512fmaskmode> 4 "register_operand"))])
+ (match_operand:V1248FH_AVX512VLBW 6 "const0_operand")
+ (match_dup 4)))]
+ "TARGET_AVX512F"
+ "#"
+ "&& 1"
+ [(const_int 0)]
+{
+ rtx op = gen_rtx_fmt_ee (GET_CODE (operands[1]), <MODE>mode,
+ operands[5], operands[2]);
+ rtx vm = gen_rtx_VEC_MERGE (<MODE>mode, op, operands[6], operands[4]);
+ emit_insn (gen_rtx_SET (operands[0], vm));
+ DONE;
+})
+
+
;; Standard scalar operation patterns which preserve the rest of the
;; vector for combiner.
(define_insn "*<sse>_vm<multdiv_mnemonic><mode>3"
diff --git a/gcc/testsuite/gcc.target/i386/avx512-maskload-fold-1.c
b/gcc/testsuite/gcc.target/i386/avx512-maskload-fold-1.c
new file mode 100644
index 00000000000..9024eba79b5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/avx512-maskload-fold-1.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -march=x86-64-v4 --param vect-partial-vector-usage=1" } */
+
+/* { dg-final { scan-assembler-times "vaddpd\[
\\t\]+\[^\n\]*\\(\[^\n\]*%zmm\[0-9\]+\{%k\[1-7\]\}\{z\}" 1 } } */
+/* { dg-final { scan-assembler-times "vmulpd\[
\\t\]+\[^\n\]*\\(\[^\n\]*%zmm\[0-9\]+\{%k\[1-7\]\}\{z\}" 1 } } */
+/* { dg-final { scan-assembler-times "vpaddd\[
\\t\]+\[^\n\]*\\(\[^\n\]*%zmm\[0-9\]+\{%k\[1-7\]\}\{z\}" 1 } } */
+/* { dg-final { scan-assembler-times "vpmulld\[
\\t\]+\[^\n\]*\\(\[^\n\]*%zmm\[0-9\]+\{%k\[1-7\]\}\{z\}" 1 } } */
+/* { dg-final { scan-assembler-times "vsubps\[
\\t\]+\[^\n\]*\\(\[^\n\]*%zmm\[0-9\]+\{%k\[1-7\]\}\{z\}" 1 } } */
+/* { dg-final { scan-assembler-times "vpsubq\[
\\t\]+\[^\n\]*\\(\[^\n\]*%zmm\[0-9\]+\{%k\[1-7\]\}\{z\}" 1 } } */
+
+void vector_add_fp (double *__restrict dst, const double *__restrict a,
+ const double *__restrict b, int n)
+{ for (int i = 0; i < n; ++i) dst[i] = a[i] + b[i]; }
+
+void vector_mul_fp (double *__restrict dst, const double *__restrict a,
+ const double *__restrict b, int n)
+{ for (int i = 0; i < n; ++i) dst[i] = a[i] * b[i]; }
+
+void vector_add_int (int *__restrict dst, const int *__restrict a,
+ const int *__restrict b, int n)
+{ for (int i = 0; i < n; ++i) dst[i] = a[i] + b[i]; }
+
+void vector_mul_int (int *__restrict dst, const int *__restrict a,
+ const int *__restrict b, int n)
+{ for (int i = 0; i < n; ++i) dst[i] = a[i] * b[i]; }
+
+void vector_sub_fp (float *__restrict dst, const float *__restrict a,
+ const float *__restrict b, int n)
+{ for (int i = 0; i < n; ++i) dst[i] = a[i] - b[i]; }
+
+void vector_sub_int64 (long long *__restrict dst, const long long *__restrict
a,
+ const long long *__restrict b, int n)
+{ for (int i = 0; i < n; ++i) dst[i] = a[i] - b[i]; }
diff --git a/gcc/testsuite/gcc.target/i386/avx512-maskload-fold-2.c
b/gcc/testsuite/gcc.target/i386/avx512-maskload-fold-2.c
new file mode 100644
index 00000000000..6d69cc646d4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/avx512-maskload-fold-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -march=x86-64-v4 --param vect-partial-vector-usage=1" } */
+
+/* { dg-final { scan-assembler-not "vsubps\[
\\t\]+\[^\n\]*\\(\[^\n\]*%zmm\[0-9\]+\{%k\[1-7\]\}\{z\}" } } */
+
+void vector_sub_no_fold (float *__restrict dst, const float *__restrict a,
+ float b, int n)
+{ for (int i = 0; i < n; i++) dst[i] = a[i] - b; }
--
2.34.1
