From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb29.google.com (mail-yb1-xb29.google.com [IPv6:2607:f8b0:4864:20::b29]) by sourceware.org (Postfix) with ESMTPS id 056BB3856973 for ; Mon, 31 Oct 2022 09:22:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 056BB3856973 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-yb1-xb29.google.com with SMTP id m125so12981270ybb.6 for ; Mon, 31 Oct 2022 02:22:17 -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=z1UOixpThWc2ZvD+cLXMdWRR7WzqRha95GAyK/4ThNY=; b=Wd+42xVULQXm6Y6zf90+FyV3F1+5xd42C9N6JF3W4aHTcG20e628LDH+CgAvuMQYgc TLYlBNofAFFd+Reo4Ku01hoaX8WDzbo0MneQ5ZMyya+w5AVvl72zHtkgKMI70S9PBNiU WoedTiVE/tbCIdolqt+H9jYPaA1gqjq4f0jJgrN4Bm1eOX6ko9rDZILIBdUnWZwdFVsl f6bGT9pNmws2tTNtA0yds2L4TXSpoakYHBo2F9ajVRZ80eMVWQvqqrNnqi8JCxTuXnqz VexlzikWKWjTYKGxcwYGHGDtbG5mMxtiyrEmFtnfsi1n7LqZUAA0jusMtPer5tGM0/ns fdYA== 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=z1UOixpThWc2ZvD+cLXMdWRR7WzqRha95GAyK/4ThNY=; b=QWpU+b5LsqSfBu1zfnAA2/i7cNCEwvTpnb/++JwH9a6mld8653Oq9VKp8PS7XLsro3 G4RHT4bneekolTW1zOb3zibu4kOIskishsaJ/NZzDGRvDKLPm2m17ifPHXmNoPYFhtO0 hJItot7mKiwPwRxXwck5LbFIjAYzzwfZvOWPUf+2xh3tKMFIzUAGyzRJ/qGhWfpOGWpJ NvCVrae1+4X+zOPfpbzreFL9A2aDH/nAgnHBtQmctqLjHsFFL2dFRwpLdhm5won7W/VM jDIdvQxZdRARM8dlhnvo5YNZuF0nDOUD+/rqT9voPuBILxoroZFXBsbCHPQR/uwtbYq8 aALw== X-Gm-Message-State: ACrzQf10O+/+rePJSZg8BZ0wuj3mKL4q3OjI2wXRi8RLBYKAV+inN6Kq Ezf5C4KnzdoDzcntL0zcdHWlDmPaJcjzMVsZAs0= X-Google-Smtp-Source: AMsMyM77ycb7K6nxRJ9oHBSJNehr04RDdvV7Tll2bdsbJAFam0WLlrJWlpKs3vnMcnDUorXeLu9xFAaW1lDd0skWbZU= X-Received: by 2002:a05:6902:309:b0:6b4:61c3:d991 with SMTP id b9-20020a056902030900b006b461c3d991mr11968025ybs.394.1667208136153; Mon, 31 Oct 2022 02:22:16 -0700 (PDT) MIME-Version: 1.0 References: <20221031011036.1158443-1-hongtao.liu@intel.com> In-Reply-To: <20221031011036.1158443-1-hongtao.liu@intel.com> From: Uros Bizjak Date: Mon, 31 Oct 2022 10:22:05 +0100 Message-ID: Subject: Re: [PATCH V2] [x86] Fix incorrect digit constraint To: liuhongt Cc: gcc-patches@gcc.gnu.org, crazylht@gmail.com, hjl.tools@gmail.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.7 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 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. 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 >