From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x112c.google.com (mail-yw1-x112c.google.com [IPv6:2607:f8b0:4864:20::112c]) by sourceware.org (Postfix) with ESMTPS id 84B933858D28 for ; Tue, 1 Nov 2022 08:39:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 84B933858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-yw1-x112c.google.com with SMTP id 00721157ae682-3321c2a8d4cso130005617b3.5 for ; Tue, 01 Nov 2022 01:39:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=MzlNYBF0HVJQwgK0FFVgkrrflEDlxDu1pk56C7E+I4M=; b=JgXePi58CDvtDGouo5oU9TnXZnT/E1XhyJDYcgOyDZZaE5cqFkDNKNdtu+tW+63VAA /3keyACEygMq98upyrjBWC8rwpePhqXroTshzTXnaDnKKPoMO/uM+D6ryS7QBP7wVpqa YqSboGO6o/alO8hTwdf6nDlAHX5a7keU370rdYQGtgM1187x0ttmfobqZ+4jaDGnFf5U MWLvjFG5+NH5155wy6e9U89EntfsVDXny6XAK1Wiajl3XAtZISIGfgDs2tXEfNCptPmR UQicGeF5B2mND5Gn1NEv9xNVSWz8adq0wDE28G8RvHWO52OA37kLP0aghf0RnWtynJrG cLlg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=MzlNYBF0HVJQwgK0FFVgkrrflEDlxDu1pk56C7E+I4M=; b=JrNsZ7kGwxXfrnONDyQIj991YzMoVgzscs+RCtEBvW/GG+/5NXRbOP8H34l1OdN6hg B7s/OiJNDz049GcR+pKsBZvV2YG1jbLXok5kTyObc0jJoQL0Y+DidXU2FVX1jphwYF1J z1AW6rxi9MF4Im4FDsW9iN6uyXFGFNJlc8dbZ2/NgbblLKqiDFev+t6l92jMBfwunAmZ OCW+Vxy0jYUVZl4GGbxVGV3pCcIKEwIOzPkWHlXZADDb0+9j+VzPn1CS3Fkh6ChhiULb CyxuM+RV99NC2Wo7Rl6UvrLdXaQgj9u7GoYo8xqS708hJshn0qsoiYYnUkShitgHFEp5 0Exw== X-Gm-Message-State: ACrzQf35le2fTkB+Bsu5BF77iPubHnEWch4HfNEBZ1EiNUpjjuD3XqRM Cv2ticwteV3jjCSArx+RCHsffVTNcdc2zz3/6D4= X-Google-Smtp-Source: AMsMyM6HfN2uvqL0EoKoMkZpuZjsfJUVgjGXYhhYAcRbU/HXq3WLkYHjGAN7tZ9f3bo2kReinC8xo1HRIymWi4ubdz8= X-Received: by 2002:a0d:f7c5:0:b0:367:b4b3:3952 with SMTP id h188-20020a0df7c5000000b00367b4b33952mr80880ywf.508.1667291971768; Tue, 01 Nov 2022 01:39:31 -0700 (PDT) MIME-Version: 1.0 References: <20221031011036.1158443-1-hongtao.liu@intel.com> In-Reply-To: From: Hongtao Liu Date: Tue, 1 Nov 2022 16:39:20 +0800 Message-ID: Subject: Re: [PATCH V2] [x86] Fix incorrect digit constraint To: Uros Bizjak Cc: liuhongt , gcc-patches@gcc.gnu.org, hjl.tools@gmail.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Oct 31, 2022 at 5:22 PM Uros Bizjak wrote: > > On Mon, Oct 31, 2022 at 2:10 AM liuhongt wrote: > > > > >You have a couple of other patterns where operand 1 is matched to > > >produce vmovddup insn. These are *avx512f_unpcklpd512 and > > >avx_unpcklpd256. You can also remove expander in both > > >cases. > > > > Yes, changed in V2 patch. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}. > > Ok for trunk? > > > > > > Matching constraints are used in these circumstances. More precisely, > > the two operands that match must include one input-only operand and > > one output-only operand. Moreover, the digit must be a smaller number > > than the number of the operand that uses it in the constraint. > > > > In pr107057, the 2 operands in the pattern are both input operands. > > > > gcc/ChangeLog: > > > > PR target/107057 > > * config/i386/sse.md (*vec_interleave_highv2df): Remove > > constraint 1. > > (*vec_interleave_lowv2df): Ditto. > > (vec_concatv2df): Ditto. > > (*avx512f_unpcklpd512): Ditto and renamed to .. > > (avx512f_unpcklpd512): .. this. > > (avx512f_movddup512): Change to define_insn. > > (avx_movddup256): Ditto. > > (*avx_unpcklpd256): Remove constraint 1 and renamed > > to .. > > (avx_unpcklpd256): .. this. > > * config/i386/i386.cc (ix86_vec_interleave_v2df_operator_ok): > > Disallow MEM_P (op1) && MEM_P (op2). > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/i386/pr107057.c: New test. > OK with two small adjustments. Changed and committed. > > Thanks, > Uros. > > > --- > > gcc/config/i386/i386.cc | 2 +- > > gcc/config/i386/sse.md | 140 +++++++++-------------- > > gcc/testsuite/gcc.target/i386/pr107057.c | 19 +++ > > 3 files changed, 77 insertions(+), 84 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr107057.c > > > > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc > > index aeea26ef4be..e3b7bea0d68 100644 > > --- a/gcc/config/i386/i386.cc > > +++ b/gcc/config/i386/i386.cc > > @@ -15652,7 +15652,7 @@ ix86_vec_interleave_v2df_operator_ok (rtx operands[3], bool high) > > if (MEM_P (operands[0])) > > return rtx_equal_p (operands[0], operands[1 + high]); > > if (MEM_P (operands[1]) && MEM_P (operands[2])) > > - return TARGET_SSE3 && rtx_equal_p (operands[1], operands[2]); > > + return false; > > return true; > > } > > > > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md > > index f4b5506703f..b7922521734 100644 > > --- a/gcc/config/i386/sse.md > > +++ b/gcc/config/i386/sse.md > > @@ -12170,107 +12170,88 @@ (define_expand "vec_interleave_highv2df" > > }) > > > > (define_insn "*vec_interleave_highv2df" > > - [(set (match_operand:V2DF 0 "nonimmediate_operand" "=x,v,v,x,v,m") > > + [(set (match_operand:V2DF 0 "nonimmediate_operand" "=x,v,x,v,m") > > (vec_select:V2DF > > (vec_concat:V4DF > > - (match_operand:V2DF 1 "nonimmediate_operand" " 0,v,o,o,o,v") > > - (match_operand:V2DF 2 "nonimmediate_operand" " x,v,1,0,v,0")) > > + (match_operand:V2DF 1 "nonimmediate_operand" " 0,v,o,o,v") > > + (match_operand:V2DF 2 "nonimmediate_operand" " x,v,0,v,0")) > > (parallel [(const_int 1) > > (const_int 3)])))] > > "TARGET_SSE2 && ix86_vec_interleave_v2df_operator_ok (operands, 1)" > > "@ > > unpckhpd\t{%2, %0|%0, %2} > > vunpckhpd\t{%2, %1, %0|%0, %1, %2} > > - %vmovddup\t{%H1, %0|%0, %H1} > > movlpd\t{%H1, %0|%0, %H1} > > vmovlpd\t{%H1, %2, %0|%0, %2, %H1} > > %vmovhpd\t{%1, %0|%q0, %1}" > > - [(set_attr "isa" "noavx,avx,sse3,noavx,avx,*") > > - (set_attr "type" "sselog,sselog,sselog,ssemov,ssemov,ssemov") > > + [(set_attr "isa" "noavx,avx,noavx,avx,*") > > + (set_attr "type" "sselog,sselog,ssemov,ssemov,ssemov") > > (set (attr "prefix_data16") > > - (if_then_else (eq_attr "alternative" "3,5") > > + (if_then_else (eq_attr "alternative" "2,4") > > (const_string "1") > > (const_string "*"))) > > - (set_attr "prefix" "orig,maybe_evex,maybe_vex,orig,maybe_evex,maybe_vex") > > - (set_attr "mode" "V2DF,V2DF,DF,V1DF,V1DF,V1DF")]) > > + (set_attr "prefix" "orig,maybe_evex,orig,maybe_evex,maybe_vex") > > + (set_attr "mode" "V2DF,V2DF,V1DF,V1DF,V1DF")]) > > > > -(define_expand "avx512f_movddup512" > > - [(set (match_operand:V8DF 0 "register_operand") > > +(define_insn "avx512f_movddup512" > > + [(set (match_operand:V8DF 0 "register_operand" "=v") > > (vec_select:V8DF > > (vec_concat:V16DF > > - (match_operand:V8DF 1 "nonimmediate_operand") > > + (match_operand:V8DF 1 "memory_operand" "m") > > I think you should leave nonimmediate_operand here with "m" predicate. > Reload is able to move the register to the memory, and it is > beneficial to allow registers for possible combine opportunities. > > > (match_dup 1)) > > (parallel [(const_int 0) (const_int 8) > > (const_int 2) (const_int 10) > > (const_int 4) (const_int 12) > > (const_int 6) (const_int 14)])))] > > - "TARGET_AVX512F") > > - > > -(define_expand "avx512f_unpcklpd512" > > - [(set (match_operand:V8DF 0 "register_operand") > > - (vec_select:V8DF > > - (vec_concat:V16DF > > - (match_operand:V8DF 1 "register_operand") > > - (match_operand:V8DF 2 "nonimmediate_operand")) > > - (parallel [(const_int 0) (const_int 8) > > - (const_int 2) (const_int 10) > > - (const_int 4) (const_int 12) > > - (const_int 6) (const_int 14)])))] > > - "TARGET_AVX512F") > > + "TARGET_AVX512F" > > + "vmovddup\t{%1, %0|%0, %1}" > > + [(set_attr "type" "sselog") > > + (set_attr "prefix" "evex") > > + (set_attr "mode" "V8DF")]) > > > > -(define_insn "*avx512f_unpcklpd512" > > - [(set (match_operand:V8DF 0 "register_operand" "=v,v") > > +(define_insn "avx512f_unpcklpd512" > > + [(set (match_operand:V8DF 0 "register_operand" "=v") > > (vec_select:V8DF > > (vec_concat:V16DF > > - (match_operand:V8DF 1 "nonimmediate_operand" "vm, v") > > - (match_operand:V8DF 2 "nonimmediate_operand" "1 ,vm")) > > + (match_operand:V8DF 1 "register_operand" "v") > > + (match_operand:V8DF 2 "nonimmediate_operand" "vm")) > > (parallel [(const_int 0) (const_int 8) > > (const_int 2) (const_int 10) > > (const_int 4) (const_int 12) > > (const_int 6) (const_int 14)])))] > > "TARGET_AVX512F" > > - "@ > > - vmovddup\t{%1, %0|%0, %1} > > - vunpcklpd\t{%2, %1, %0|%0, %1, %2}" > > + "vunpcklpd\t{%2, %1, %0|%0, %1, %2}" > > [(set_attr "type" "sselog") > > (set_attr "prefix" "evex") > > (set_attr "mode" "V8DF")]) > > > > ;; Recall that the 256-bit unpck insns only shuffle within their lanes. > > -(define_expand "avx_movddup256" > > - [(set (match_operand:V4DF 0 "register_operand") > > +(define_insn "avx_movddup256" > > + [(set (match_operand:V4DF 0 "register_operand" "=v") > > (vec_select:V4DF > > (vec_concat:V8DF > > - (match_operand:V4DF 1 "nonimmediate_operand") > > + (match_operand:V4DF 1 "memory_operand" "m") > > Also here, leave nonimmediate_operand. > > > (match_dup 1)) > > (parallel [(const_int 0) (const_int 4) > > (const_int 2) (const_int 6)])))] > > - "TARGET_AVX && ") > > - > > -(define_expand "avx_unpcklpd256" > > - [(set (match_operand:V4DF 0 "register_operand") > > - (vec_select:V4DF > > - (vec_concat:V8DF > > - (match_operand:V4DF 1 "register_operand") > > - (match_operand:V4DF 2 "nonimmediate_operand")) > > - (parallel [(const_int 0) (const_int 4) > > - (const_int 2) (const_int 6)])))] > > - "TARGET_AVX && ") > > + "TARGET_AVX && " > > + "vmovddup\t{%1, %0|%0, %1}" > > + [(set_attr "type" "sselog") > > + (set_attr "prefix" "") > > + (set_attr "mode" "V4DF")]) > > > > -(define_insn "*avx_unpcklpd256" > > - [(set (match_operand:V4DF 0 "register_operand" "=v,v") > > +(define_insn "avx_unpcklpd256" > > + [(set (match_operand:V4DF 0 "register_operand" "=v") > > (vec_select:V4DF > > (vec_concat:V8DF > > - (match_operand:V4DF 1 "nonimmediate_operand" " v,m") > > - (match_operand:V4DF 2 "nonimmediate_operand" "vm,1")) > > + (match_operand:V4DF 1 "register_operand" " v") > > + (match_operand:V4DF 2 "nonimmediate_operand" "vm")) > > (parallel [(const_int 0) (const_int 4) > > (const_int 2) (const_int 6)])))] > > "TARGET_AVX && " > > - "@ > > - vunpcklpd\t{%2, %1, %0|%0, %1, %2} > > - vmovddup\t{%1, %0|%0, %1}" > > + "vunpcklpd\t{%2, %1, %0|%0, %1, %2}" > > [(set_attr "type" "sselog") > > - (set_attr "prefix" "vex") > > + (set_attr "prefix" "") > > (set_attr "mode" "V4DF")]) > > > > (define_expand "vec_interleave_lowv4df" > > @@ -12332,29 +12313,28 @@ (define_expand "vec_interleave_lowv2df" > > }) > > > > (define_insn "*vec_interleave_lowv2df" > > - [(set (match_operand:V2DF 0 "nonimmediate_operand" "=x,v,v,x,v,o") > > + [(set (match_operand:V2DF 0 "nonimmediate_operand" "=x,v,x,v,o") > > (vec_select:V2DF > > (vec_concat:V4DF > > - (match_operand:V2DF 1 "nonimmediate_operand" " 0,v,m,0,v,0") > > - (match_operand:V2DF 2 "nonimmediate_operand" " x,v,1,m,m,v")) > > + (match_operand:V2DF 1 "nonimmediate_operand" " 0,v,0,v,0") > > + (match_operand:V2DF 2 "nonimmediate_operand" " x,v,m,m,v")) > > (parallel [(const_int 0) > > (const_int 2)])))] > > "TARGET_SSE2 && ix86_vec_interleave_v2df_operator_ok (operands, 0)" > > "@ > > unpcklpd\t{%2, %0|%0, %2} > > vunpcklpd\t{%2, %1, %0|%0, %1, %2} > > - %vmovddup\t{%1, %0|%0, %q1} > > movhpd\t{%2, %0|%0, %q2} > > vmovhpd\t{%2, %1, %0|%0, %1, %q2} > > %vmovlpd\t{%2, %H0|%H0, %2}" > > - [(set_attr "isa" "noavx,avx,sse3,noavx,avx,*") > > - (set_attr "type" "sselog,sselog,sselog,ssemov,ssemov,ssemov") > > + [(set_attr "isa" "noavx,avx,noavx,avx,*") > > + (set_attr "type" "sselog,sselog,ssemov,ssemov,ssemov") > > (set (attr "prefix_data16") > > - (if_then_else (eq_attr "alternative" "3,5") > > + (if_then_else (eq_attr "alternative" "2,4") > > (const_string "1") > > (const_string "*"))) > > - (set_attr "prefix" "orig,maybe_evex,maybe_vex,orig,maybe_evex,maybe_vex") > > - (set_attr "mode" "V2DF,V2DF,DF,V1DF,V1DF,V1DF")]) > > + (set_attr "prefix" "orig,maybe_evex,orig,maybe_evex,maybe_vex") > > + (set_attr "mode" "V2DF,V2DF,V1DF,V1DF,V1DF")]) > > > > (define_split > > [(set (match_operand:V2DF 0 "memory_operand") > > @@ -13560,56 +13540,50 @@ (define_insn "vec_dupv2df" > > (set_attr "mode" "V2DF,DF,DF")]) > > > > (define_insn "vec_concatv2df" > > - [(set (match_operand:V2DF 0 "register_operand" "=x,x,v,x,v,x,x, v,x,x") > > + [(set (match_operand:V2DF 0 "register_operand" "=x,x,v,x,x, v,x,x") > > (vec_concat:V2DF > > - (match_operand:DF 1 "nonimmediate_operand" " 0,x,v,m,m,0,x,vm,0,0") > > - (match_operand:DF 2 "nonimm_or_0_operand" " x,x,v,1,1,m,m, C,x,m")))] > > - "TARGET_SSE > > - && (!(MEM_P (operands[1]) && MEM_P (operands[2])) > > - || (TARGET_SSE3 && rtx_equal_p (operands[1], operands[2])))" > > + (match_operand:DF 1 "nonimmediate_operand" " 0,x,v,0,x,vm,0,0") > > + (match_operand:DF 2 "nonimm_or_0_operand" " x,x,v,m,m, C,x,m")))] > > + "TARGET_SSE && !(MEM_P (operands[1]) && MEM_P (operands[2]))" > > "@ > > unpcklpd\t{%2, %0|%0, %2} > > vunpcklpd\t{%2, %1, %0|%0, %1, %2} > > vunpcklpd\t{%2, %1, %0|%0, %1, %2} > > - %vmovddup\t{%1, %0|%0, %1} > > - vmovddup\t{%1, %0|%0, %1} > > movhpd\t{%2, %0|%0, %2} > > vmovhpd\t{%2, %1, %0|%0, %1, %2} > > %vmovq\t{%1, %0|%0, %1} > > movlhps\t{%2, %0|%0, %2} > > movhps\t{%2, %0|%0, %2}" > > [(set (attr "isa") > > - (cond [(eq_attr "alternative" "0,5") > > + (cond [(eq_attr "alternative" "0,3") > > (const_string "sse2_noavx") > > - (eq_attr "alternative" "1,6") > > + (eq_attr "alternative" "1,4") > > (const_string "avx") > > - (eq_attr "alternative" "2,4") > > + (eq_attr "alternative" "2") > > (const_string "avx512vl") > > - (eq_attr "alternative" "3") > > - (const_string "sse3") > > - (eq_attr "alternative" "7") > > + (eq_attr "alternative" "5") > > (const_string "sse2") > > ] > > (const_string "noavx"))) > > (set (attr "type") > > (if_then_else > > - (eq_attr "alternative" "0,1,2,3,4") > > + (eq_attr "alternative" "0,1,2") > > (const_string "sselog") > > (const_string "ssemov"))) > > (set (attr "prefix_data16") > > - (if_then_else (eq_attr "alternative" "5") > > + (if_then_else (eq_attr "alternative" "3") > > (const_string "1") > > (const_string "*"))) > > (set (attr "prefix") > > - (cond [(eq_attr "alternative" "1,6") > > + (cond [(eq_attr "alternative" "1,4") > > (const_string "vex") > > - (eq_attr "alternative" "2,4") > > + (eq_attr "alternative" "2") > > (const_string "evex") > > - (eq_attr "alternative" "3,7") > > + (eq_attr "alternative" "5") > > (const_string "maybe_vex") > > ] > > (const_string "orig"))) > > - (set_attr "mode" "V2DF,V2DF,V2DF, DF, DF, V1DF,V1DF,DF,V4SF,V2SF")]) > > + (set_attr "mode" "V2DF,V2DF,V2DF,V1DF,V1DF,DF,V4SF,V2SF")]) > > > > ;; vmovq clears also the higher bits. > > (define_insn "vec_set_0" > > diff --git a/gcc/testsuite/gcc.target/i386/pr107057.c b/gcc/testsuite/gcc.target/i386/pr107057.c > > new file mode 100644 > > index 00000000000..40b49ac21ec > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/pr107057.c > > @@ -0,0 +1,19 @@ > > +/* { dg-do compile { target { ! ia32 } } } */ > > +/* { dg-options "-mavx -mcmodel=large -O3" } */ > > + > > +typedef double v2df __attribute__ ((vector_size (16))); > > +v2df f (double a, double b) > > +{ > > + v2df v; > > + double *c = (double *)&v; > > + *c = a; > > + *(c+1) = b; > > + return v; > > +} > > +void g () > > +{ > > + v2df x = f (1.0, 1.0); > > + v2df y = f (2.0, 2.0); > > + for (;*(double *)&x<=8; x+=y) > > + g (); > > +} > > -- > > 2.27.0 > > -- BR, Hongtao