On Wed, Oct 4, 2017 at 10:33 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> Most AVX* instructions have the same insn name between VEX and EVEX
> encoded insns and whether it is EVEX or VEX encoded is determined by
> the operands by the assembler (if there is masking/broadcast, or
> %[xy]mm16+ operands are present, or when using 512-bit vectors).
> This is not the case for the logical instruction, we have VEX
> encoded VP{AND,OR,XOR,ANDN} and EVEX encoded VP{AND,OR,XOR,ANDN}{D,Q}.
>
> Right now we use the d or q suffixes for -mavx512vl even when we could
> use VEX encoded insn, which results in larger opcode.
>
> The following patch fixes that, by emitting the suffix only when needed.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-10-04 Jakub Jelinek <ja...@redhat.com>
>
> PR target/82370
> * config/i386/sse.md (*andnot<mode>3,
> <mask_codefor><code><mode>3<mask_name>, *<code><mode>3): Split
> (=v,v,vm) alternative into (=x,x,xm) and (=v,v,vm), for 128-bit
> and 256-bit vectors, the (=x,x,xm) alternative and when mask is
> not applied use empty suffix even for TARGET_AVX512VL.
> * config/i386/subst.md (mask_prefix3, mask_prefix4): When mask
> is applied, supply evex,evex or evex,evex,evex instead of just
> evex.
LGTM, but Kirill should give the final approval for this patch.
Uros.
> --- gcc/config/i386/sse.md.jj 2017-09-22 20:51:51.000000000 +0200
> +++ gcc/config/i386/sse.md 2017-10-03 18:25:24.289527626 +0200
> @@ -11568,10 +11568,10 @@ (define_expand "<sse2_avx2>_andnot<mode>
> "TARGET_AVX512BW")
>
> (define_insn "*andnot<mode>3"
> - [(set (match_operand:VI 0 "register_operand" "=x,v")
> + [(set (match_operand:VI 0 "register_operand" "=x,x,v")
> (and:VI
> - (not:VI (match_operand:VI 1 "register_operand" "0,v"))
> - (match_operand:VI 2 "vector_operand" "xBm,vm")))]
> + (not:VI (match_operand:VI 1 "register_operand" "0,x,v"))
> + (match_operand:VI 2 "vector_operand" "xBm,xm,vm")))]
> "TARGET_SSE"
> {
> static char buf[64];
> @@ -11606,10 +11606,11 @@ (define_insn "*andnot<mode>3"
> case E_V4DImode:
> case E_V4SImode:
> case E_V2DImode:
> - ssesuffix = TARGET_AVX512VL ? "<ssemodesuffix>" : "";
> + ssesuffix = (TARGET_AVX512VL && which_alternative == 2
> + ? "<ssemodesuffix>" : "");
> break;
> default:
> - ssesuffix = TARGET_AVX512VL ? "q" : "";
> + ssesuffix = TARGET_AVX512VL && which_alternative == 2 ? "q" : "";
> }
> break;
>
> @@ -11635,6 +11636,7 @@ (define_insn "*andnot<mode>3"
> ops = "%s%s\t{%%2, %%0|%%0, %%2}";
> break;
> case 1:
> + case 2:
> ops = "v%s%s\t{%%2, %%1, %%0|%%0, %%1, %%2}";
> break;
> default:
> @@ -11644,7 +11646,7 @@ (define_insn "*andnot<mode>3"
> snprintf (buf, sizeof (buf), ops, tmp, ssesuffix);
> return buf;
> }
> - [(set_attr "isa" "noavx,avx")
> + [(set_attr "isa" "noavx,avx,avx")
> (set_attr "type" "sselog")
> (set (attr "prefix_data16")
> (if_then_else
> @@ -11652,7 +11654,7 @@ (define_insn "*andnot<mode>3"
> (eq_attr "mode" "TI"))
> (const_string "1")
> (const_string "*")))
> - (set_attr "prefix" "orig,vex")
> + (set_attr "prefix" "orig,vex,evex")
> (set (attr "mode")
> (cond [(and (match_test "<MODE_SIZE> == 16")
> (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
> @@ -11697,10 +11699,10 @@ (define_expand "<code><mode>3"
> })
>
> (define_insn "<mask_codefor><code><mode>3<mask_name>"
> - [(set (match_operand:VI48_AVX_AVX512F 0 "register_operand" "=x,v")
> + [(set (match_operand:VI48_AVX_AVX512F 0 "register_operand" "=x,x,v")
> (any_logic:VI48_AVX_AVX512F
> - (match_operand:VI48_AVX_AVX512F 1 "vector_operand" "%0,v")
> - (match_operand:VI48_AVX_AVX512F 2 "vector_operand" "xBm,vm")))]
> + (match_operand:VI48_AVX_AVX512F 1 "vector_operand" "%0,x,v")
> + (match_operand:VI48_AVX_AVX512F 2 "vector_operand" "xBm,xm,vm")))]
> "TARGET_SSE && <mask_mode512bit_condition>
> && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
> {
> @@ -11730,7 +11732,9 @@ (define_insn "<mask_codefor><code><mode>
> case E_V4DImode:
> case E_V4SImode:
> case E_V2DImode:
> - ssesuffix = TARGET_AVX512VL ? "<ssemodesuffix>" : "";
> + ssesuffix = (TARGET_AVX512VL
> + && (<mask_applied> || which_alternative == 2)
> + ? "<ssemodesuffix>" : "");
> break;
> default:
> gcc_unreachable ();
> @@ -11759,6 +11763,7 @@ (define_insn "<mask_codefor><code><mode>
> ops = "%s%s\t{%%2, %%0|%%0, %%2}";
> break;
> case 1:
> + case 2:
> ops = "v%s%s\t{%%2, %%1, %%0<mask_operand3_1>|%%0<mask_operand3_1>,
> %%1, %%2}";
> break;
> default:
> @@ -11768,7 +11773,7 @@ (define_insn "<mask_codefor><code><mode>
> snprintf (buf, sizeof (buf), ops, tmp, ssesuffix);
> return buf;
> }
> - [(set_attr "isa" "noavx,avx")
> + [(set_attr "isa" "noavx,avx,avx")
> (set_attr "type" "sselog")
> (set (attr "prefix_data16")
> (if_then_else
> @@ -11776,7 +11781,7 @@ (define_insn "<mask_codefor><code><mode>
> (eq_attr "mode" "TI"))
> (const_string "1")
> (const_string "*")))
> - (set_attr "prefix" "<mask_prefix3>")
> + (set_attr "prefix" "<mask_prefix3>,evex")
> (set (attr "mode")
> (cond [(and (match_test "<MODE_SIZE> == 16")
> (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
> @@ -11795,10 +11800,10 @@ (define_insn "<mask_codefor><code><mode>
> (const_string "<sseinsnmode>")))])
>
> (define_insn "*<code><mode>3"
> - [(set (match_operand:VI12_AVX_AVX512F 0 "register_operand" "=x,v")
> + [(set (match_operand:VI12_AVX_AVX512F 0 "register_operand" "=x,x,v")
> (any_logic: VI12_AVX_AVX512F
> - (match_operand:VI12_AVX_AVX512F 1 "vector_operand" "%0,v")
> - (match_operand:VI12_AVX_AVX512F 2 "vector_operand" "xBm,vm")))]
> + (match_operand:VI12_AVX_AVX512F 1 "vector_operand" "%0,x,v")
> + (match_operand:VI12_AVX_AVX512F 2 "vector_operand" "xBm,xm,vm")))]
> "TARGET_SSE && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
> {
> static char buf[64];
> @@ -11827,7 +11832,7 @@ (define_insn "*<code><mode>3"
> case E_V16HImode:
> case E_V16QImode:
> case E_V8HImode:
> - ssesuffix = TARGET_AVX512VL ? "q" : "";
> + ssesuffix = TARGET_AVX512VL && which_alternative == 2 ? "q" : "";
> break;
> default:
> gcc_unreachable ();
> @@ -11853,6 +11858,7 @@ (define_insn "*<code><mode>3"
> ops = "%s%s\t{%%2, %%0|%%0, %%2}";
> break;
> case 1:
> + case 2:
> ops = "v%s%s\t{%%2, %%1, %%0|%%0, %%1, %%2}";
> break;
> default:
> @@ -11862,7 +11868,7 @@ (define_insn "*<code><mode>3"
> snprintf (buf, sizeof (buf), ops, tmp, ssesuffix);
> return buf;
> }
> - [(set_attr "isa" "noavx,avx")
> + [(set_attr "isa" "noavx,avx,avx")
> (set_attr "type" "sselog")
> (set (attr "prefix_data16")
> (if_then_else
> @@ -11870,7 +11876,7 @@ (define_insn "*<code><mode>3"
> (eq_attr "mode" "TI"))
> (const_string "1")
> (const_string "*")))
> - (set_attr "prefix" "<mask_prefix3>")
> + (set_attr "prefix" "<mask_prefix3>,evex")
> (set (attr "mode")
> (cond [(and (match_test "<MODE_SIZE> == 16")
> (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
> --- gcc/config/i386/subst.md.jj 2017-07-06 20:31:45.000000000 +0200
> +++ gcc/config/i386/subst.md 2017-10-03 18:20:45.605867446 +0200
> @@ -62,8 +62,8 @@ (define_subst_attr "store_mask_constrain
> (define_subst_attr "store_mask_predicate" "mask" "nonimmediate_operand"
> "register_operand")
> (define_subst_attr "mask_prefix" "mask" "vex" "evex")
> (define_subst_attr "mask_prefix2" "mask" "maybe_vex" "evex")
> -(define_subst_attr "mask_prefix3" "mask" "orig,vex" "evex")
> -(define_subst_attr "mask_prefix4" "mask" "orig,orig,vex" "evex")
> +(define_subst_attr "mask_prefix3" "mask" "orig,vex" "evex,evex")
> +(define_subst_attr "mask_prefix4" "mask" "orig,orig,vex" "evex,evex,evex")
> (define_subst_attr "mask_expand_op3" "mask" "3" "5")
>
> (define_subst "mask"
>
> Jakub
