On Tue, Jul 11, 2023 at 12:24 AM Alexander Monakov via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > On Mon, 10 Jul 2023, liuhongt via Gcc-patches wrote: > > > False dependency happens when destination is only updated by > > pternlog. There is no false dependency when destination is also used > > in source. So either a pxor should be inserted, or input operand > > should be set with constraint '0'. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}. > > Ready to push to trunk. > > Shouldn't this patch also remove uses of vpternlog in > standard_sse_constant_opcode? It's still needed when !optimize_function_for_speed_p (cfun). > > A couple more questions below: > > > --- a/gcc/config/i386/sse.md > > +++ b/gcc/config/i386/sse.md > > @@ -1382,6 +1382,29 @@ (define_insn "mov<mode>_internal" > > ] > > (symbol_ref "true")))]) > > > > +; False dependency happens on destination register which is not really > > +; used when moving all ones to vector register > > +(define_split > > + [(set (match_operand:VMOVE 0 "register_operand") > > + (match_operand:VMOVE 1 "int_float_vector_all_ones_operand"))] > > + "TARGET_AVX512F && reload_completed > > + && (<MODE_SIZE> == 64 || EXT_REX_SSE_REG_P (operands[0])) > > + && optimize_function_for_speed_p (cfun)" > > Yan's patch used optimize_insn_for_speed_p (), which looks more appropriate. > Doesn't it work here as well? I'm just aligned with lzcnt/popcnt case, the difference between option_insn_for_speed_p and optimized_function_for_speed_p is the former will consider !crtl->maybe_hot_insn_p but the latter just returns !optimize_function_for_size_p (cfun). It looks optimize_insn_for_speed_p() is more reasonable for single insn.
350optimize_insn_for_size_p (void) 351{ 352 enum optimize_size_level ret = optimize_function_for_size_p (cfun); 353 if (ret < OPTIMIZE_SIZE_BALANCED && !crtl->maybe_hot_insn_p) 354 ret = OPTIMIZE_SIZE_BALANCED; 355 return ret; > > > + [(set (match_dup 0) (match_dup 2)) > > + (parallel > > + [(set (match_dup 0) (match_dup 1)) > > + (unspec [(match_dup 0)] UNSPEC_INSN_FALSE_DEP)])] > > + "operands[2] = CONST0_RTX (<MODE>mode);") > > + > > +(define_insn "*vmov<mode>_constm1_pternlog_false_dep" > > + [(set (match_operand:VMOVE 0 "register_operand" "=v") > > + (match_operand:VMOVE 1 "int_float_vector_all_ones_operand" > > "<sseconstm1>")) > > + (unspec [(match_operand:VMOVE 2 "register_operand" "0")] > > UNSPEC_INSN_FALSE_DEP)] > > + "TARGET_AVX512VL || <MODE_SIZE> == 64" > > + "vpternlogd\t{$0xFF, %0, %0, %0|%0, %0, %0, 0xFF}" > > + [(set_attr "type" "sselog1") > > + (set_attr "prefix" "evex")]) > > + > > ;; If mem_addr points to a memory region with less than whole vector size > > bytes > > ;; of accessible memory and k is a mask that would prevent reading the > > inaccessible > > ;; bytes from mem_addr, add UNSPEC_MASKLOAD to prevent it to be > > transformed to vpblendd > > @@ -9336,7 +9359,7 @@ (define_expand > > "<avx512>_cvtmask2<ssemodesuffix><mode>" > > operands[3] = CONST0_RTX (<MODE>mode); > > }") > > > > -(define_insn "*<avx512>_cvtmask2<ssemodesuffix><mode>" > > +(define_insn_and_split "*<avx512>_cvtmask2<ssemodesuffix><mode>" > > [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v,v") > > (vec_merge:VI48_AVX512VL > > (match_operand:VI48_AVX512VL 2 "vector_all_ones_operand") > > @@ -9346,11 +9369,35 @@ (define_insn > > "*<avx512>_cvtmask2<ssemodesuffix><mode>" > > "@ > > vpmovm2<ssemodesuffix>\t{%1, %0|%0, %1} > > vpternlog<ssemodesuffix>\t{$0x81, %0, %0, %0%{%1%}%{z%}|%0%{%1%}%{z%}, > > %0, %0, 0x81}" > > + "&& !TARGET_AVX512DQ && reload_completed > > + && optimize_function_for_speed_p (cfun)" > > + [(set (match_dup 0) (match_dup 4)) > > + (parallel > > + [(set (match_dup 0) > > + (vec_merge:VI48_AVX512VL > > + (match_dup 2) > > + (match_dup 3) > > + (match_dup 1))) > > + (unspec [(match_dup 0)] UNSPEC_INSN_FALSE_DEP)])] > > + "operands[4] = CONST0_RTX (<MODE>mode);" > > [(set_attr "isa" "avx512dq,*") > > (set_attr "length_immediate" "0,1") > > (set_attr "prefix" "evex") > > (set_attr "mode" "<sseinsnmode>")]) > > > > +(define_insn "*<avx512>_cvtmask2<ssemodesuffix><mode>_pternlog_false_dep" > > + [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v") > > + (vec_merge:VI48_AVX512VL > > + (match_operand:VI48_AVX512VL 2 "vector_all_ones_operand") > > + (match_operand:VI48_AVX512VL 3 "const0_operand") > > + (match_operand:<avx512fmaskmode> 1 "register_operand" "Yk"))) > > + (unspec [(match_operand:VI48_AVX512VL 4 "register_operand" "0")] > > UNSPEC_INSN_FALSE_DEP)] > > + "TARGET_AVX512F && !TARGET_AVX512DQ" > > + "vpternlog<ssemodesuffix>\t{$0x81, %0, %0, %0%{%1%}%{z%}|%0%{%1%}%{z%}, > > %0, %0, 0x81}" > > + [(set_attr "length_immediate" "1") > > + (set_attr "prefix" "evex") > > + (set_attr "mode" "<sseinsnmode>")]) > > + > > (define_expand "extendv2sfv2df2" > > [(set (match_operand:V2DF 0 "register_operand") > > (float_extend:V2DF > > @@ -17166,20 +17213,32 @@ (define_expand "one_cmpl<mode>2" > > operands[2] = force_reg (<MODE>mode, operands[2]); > > }) > > > > -(define_insn "<mask_codefor>one_cmpl<mode>2<mask_name>" > > - [(set (match_operand:VI 0 "register_operand" "=v,v") > > - (xor:VI (match_operand:VI 1 "bcst_vector_operand" "vBr,m") > > - (match_operand:VI 2 "vector_all_ones_operand" "BC,BC")))] > > +(define_insn_and_split "<mask_codefor>one_cmpl<mode>2<mask_name>" > > + [(set (match_operand:VI 0 "register_operand" "=v,v,v") > > + (xor:VI (match_operand:VI 1 "bcst_vector_operand" " 0, m,Br") > > + (match_operand:VI 2 "vector_all_ones_operand" "BC,BC,BC")))] > > "TARGET_AVX512F > > && (!<mask_applied> > > || <ssescalarmode>mode == SImode > > || <ssescalarmode>mode == DImode)" > > { > > + if (!<mask_applied> && which_alternative > > + && optimize_function_for_speed_p (cfun)) > > + return "#"; > > + > > if (TARGET_AVX512VL) > > return "vpternlog<ternlogsuffix>\t{$0x55, %1, %0, > > %0<mask_operand3>|%0<mask_operand3>, %0, %1, 0x55}"; > > else > > return "vpternlog<ternlogsuffix>\t{$0x55, %g1, %g0, > > %g0<mask_operand3>|%g0<mask_operand3>, %g0, %g1, 0x55}"; > > } > > + "&& reload_completed && !REG_P (operands[1]) && !<mask_applied> > > + && optimize_function_for_speed_p (cfun)" > > + [(set (match_dup 0) (match_dup 3)) > > + (parallel > > + [(set (match_dup 0) > > + (xor:VI (match_dup 1) (match_dup 2))) > > + (unspec [(match_dup 0)] UNSPEC_INSN_FALSE_DEP)])] > > + "operands[3] = CONST0_RTX (<MODE>mode);" > > Perhaps I'm misreading this, but this seems to result in > > vpxor zmm0, zmm0 > vpternlog zmm0, zmm0, [mem], 0x55 > I thought the first alternative (v,0,BC) would handle that, looks not, i'll adjust the splitter to explicitly put operands[1] into operands[0] when it's memory. > while in the PR the agreement was to emit > > vmovdq? zmm0, [mem] > vpternlog zmm0, zmm0, zmm0, 0x55 > > when the source is in memory, because the former has three uops in fused > domain? > > > [(set_attr "type" "sselog") > > > (set_attr "prefix" "evex") > > (set (attr "mode") > > @@ -17191,6 +17250,30 @@ (define_insn > > "<mask_codefor>one_cmpl<mode>2<mask_name>" > > (symbol_ref "<MODE_SIZE> == 64 || TARGET_AVX512VL") > > (const_int 1)))]) > > > > +(define_insn "*one_cmpl<mode>2_pternlog_false_dep" > > + [(set (match_operand:VI 0 "register_operand" "=v,v") > > + (xor:VI (match_operand:VI 1 "bcst_vector_operand" "m, Br") > > + (match_operand:VI 2 "vector_all_ones_operand" "BC,BC"))) > > + (unspec [(match_operand:VI 3 "register_operand" "0,0")] > > + UNSPEC_INSN_FALSE_DEP)] > > + "TARGET_AVX512F" > > +{ > > + if (TARGET_AVX512VL) > > + return "vpternlog<ternlogsuffix>\t{$0x55, %1, %0, > > %0<mask_operand3>|%0<mask_operand3>, %0, %1, 0x55}"; > > + else > > + return "vpternlog<ternlogsuffix>\t{$0x55, %g1, %g0, > > %g0<mask_operand3>|%g0<mask_operand3>, %g0, %g1, 0x55}"; > > +} > > + [(set_attr "type" "sselog") > > + (set_attr "prefix" "evex") > > + (set (attr "mode") > > + (if_then_else (match_test "TARGET_AVX512VL") > > + (const_string "<sseinsnmode>") > > + (const_string "XI"))) > > + (set (attr "enabled") > > + (if_then_else (eq_attr "alternative" "0") > > + (symbol_ref "<MODE_SIZE> == 64 || TARGET_AVX512VL") > > + (const_int 1)))]) > > + > > (define_split > > [(set (match_operand:VI48_AVX512F 0 "register_operand") > > (vec_duplicate:VI48_AVX512F > > @@ -17226,7 +17309,7 @@ (define_insn "*andnot<mode>3" > > [(set (match_operand:VI 0 "register_operand" "=x,x,v,v,v") > > (and:VI > > (not:VI (match_operand:VI 1 "bcst_vector_operand" "0,x,v,m,Br")) > > - (match_operand:VI 2 "bcst_vector_operand" "xBm,xm,vmBr,v,v")))] > > + (match_operand:VI 2 "bcst_vector_operand" "xBm,xm,vmBr,0,0")))] > > "TARGET_SSE > > && (register_operand (operands[1], <MODE>mode) > > || register_operand (operands[2], <MODE>mode))" > > @@ -17685,8 +17768,8 @@ (define_insn "*iornot<mode>3" > > [(set (match_operand:VI 0 "register_operand" "=v,v,v,v") > > (ior:VI > > (not:VI > > - (match_operand:VI 1 "bcst_vector_operand" "v,Br,v,m")) > > - (match_operand:VI 2 "bcst_vector_operand" "vBr,v,m,v")))] > > + (match_operand:VI 1 "bcst_vector_operand" "0,m, 0,vBr")) > > + (match_operand:VI 2 "bcst_vector_operand" "m,0,vBr, 0")))] > > "(<MODE_SIZE> == 64 || TARGET_AVX512VL > > || (TARGET_AVX512F && !TARGET_PREFER_AVX256)) > > && (register_operand (operands[1], <MODE>mode) > > @@ -17710,7 +17793,7 @@ (define_insn "*iornot<mode>3" > > (const_string "<sseinsnmode>") > > (const_string "XI"))) > > (set (attr "enabled") > > - (if_then_else (eq_attr "alternative" "2,3") > > + (if_then_else (eq_attr "alternative" "0,1") > > (symbol_ref "<MODE_SIZE> == 64 || TARGET_AVX512VL") > > (const_string "*")))]) > > > > @@ -17718,8 +17801,8 @@ (define_insn "*xnor<mode>3" > > [(set (match_operand:VI 0 "register_operand" "=v,v") > > (not:VI > > (xor:VI > > - (match_operand:VI 1 "bcst_vector_operand" "%v,v") > > - (match_operand:VI 2 "bcst_vector_operand" "vBr,m"))))] > > + (match_operand:VI 1 "bcst_vector_operand" "%0, 0") > > + (match_operand:VI 2 "bcst_vector_operand" " m,vBr"))))] > > "(<MODE_SIZE> == 64 || TARGET_AVX512VL > > || (TARGET_AVX512F && !TARGET_PREFER_AVX256)) > > && (register_operand (operands[1], <MODE>mode) > > @@ -17738,7 +17821,7 @@ (define_insn "*xnor<mode>3" > > (const_string "<sseinsnmode>") > > (const_string "XI"))) > > (set (attr "enabled") > > - (if_then_else (eq_attr "alternative" "1") > > + (if_then_else (eq_attr "alternative" "0") > > (symbol_ref "<MODE_SIZE> == 64 || TARGET_AVX512VL") > > (const_string "*")))]) > > > > @@ -17749,8 +17832,8 @@ (define_code_attr ternlog_nlogic [(and "0x11") (ior > > "0x77")]) > > (define_insn "*<nlogic><mode>3" > > [(set (match_operand:VI 0 "register_operand" "=v,v") > > (andor:VI > > - (not:VI (match_operand:VI 1 "bcst_vector_operand" "%v,v")) > > - (not:VI (match_operand:VI 2 "bcst_vector_operand" "vBr,m"))))] > > + (not:VI (match_operand:VI 1 "bcst_vector_operand" "%0, 0")) > > + (not:VI (match_operand:VI 2 "bcst_vector_operand" "m,vBr"))))] > > "(<MODE_SIZE> == 64 || TARGET_AVX512VL > > || (TARGET_AVX512F && !TARGET_PREFER_AVX256)) > > && (register_operand (operands[1], <MODE>mode) > > @@ -17769,7 +17852,7 @@ (define_insn "*<nlogic><mode>3" > > (const_string "<sseinsnmode>") > > (const_string "XI"))) > > (set (attr "enabled") > > - (if_then_else (eq_attr "alternative" "1") > > + (if_then_else (eq_attr "alternative" "0") > > (symbol_ref "<MODE_SIZE> == 64 || TARGET_AVX512VL") > > (const_string "*")))]) > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr110438.c > > b/gcc/testsuite/gcc.target/i386/pr110438.c > > new file mode 100644 > > index 00000000000..11b8cc59fd2 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/pr110438.c > > @@ -0,0 +1,30 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-mavx512f -O2 -ftree-vectorize -mno-avx512dq -dp > > -mprefer-vector-width=512" } */ > > +/* { dg-final { scan-assembler-times {cvtmask2.*_pternlog} "1" } } */ > > +/* { dg-final { scan-assembler-times {constm1_pternlog} "1" } } */ > > +/* { dg-final { scan-assembler-not {(?n)vpternlogd.*\(} } } */ > > + > > + > > +#include <immintrin.h> > > + > > +__m512i g(void) > > +{ > > + return (__m512i){ 0 } - 1; > > +} > > + > > +__m512i g1(__m512i* a) > > +{ > > + return ~(*a); > > +} > > + > > +void > > +foo (int* a, int* __restrict b) > > +{ > > + for (int i = 0; i != 16; i++) > > + { > > + if (b[i]) > > + a[i] = -1; > > + else > > + a[i] = 0; > > + } > > +} > > -- BR, Hongtao