From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x141.google.com (mail-il1-x141.google.com [IPv6:2607:f8b0:4864:20::141]) by sourceware.org (Postfix) with ESMTPS id 69998385B80B for ; Fri, 3 Apr 2020 17:19:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 69998385B80B Received: by mail-il1-x141.google.com with SMTP id r5so8023308ilq.6 for ; Fri, 03 Apr 2020 10:19:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=dX5DBEku8RKZf0va8zJoxBtKM/rSR0jjAASzoHauX7c=; b=nR1RDnzqzWMgLA8n95ic5aXVtCrESfcZTQVHpxpi4TU0xS+lWs+u+Q9fok8hQ7jBST lvXoq2SPUKQrYYM+Etx1UYD3Lt0SC4zqrctdU8Tz03p4V/pKmFpMpDkAn97C3nSRBZe5 hxZXRzsQHtO7wEAOVJLqTAYyEsd/39cpsDA1SiD8jRjNqv5ZP1tLUI9f3LL6TEcYmwf1 ieFD7tlQJX1AFQp+dpJsxOLoNskTvKBOLxIWJa57t4/NeoV8KL9tM8WUnUkPnMfiNIpp GWDaw8PkH00nbXkLjFx1YBtrW4KXH5HF+AQdWrW4fOws16x7ssI3M/sWIFN66Cs6O7a+ OQDA== X-Gm-Message-State: AGi0Publv+VRaqvcRZ6IQ1u+FfNb3liAnmlIld+e5fl4yKcLlkGNEzHf N5uj6oRYWHvFlkF+pIbW4emwvVZypifoxLKLxQE= X-Google-Smtp-Source: APiQypJnYqESRfWywXchXDoP3L1crkG0XKoxRMDO2hyHgcYKyrFFTnSGL0clglz5Y4REBvsMPLvxIe+H62vbBPKFufg= X-Received: by 2002:a92:c910:: with SMTP id t16mr9825323ilp.254.1585934340882; Fri, 03 Apr 2020 10:19:00 -0700 (PDT) MIME-Version: 1.0 References: <20200403170612.GC2212@tucnak> In-Reply-To: <20200403170612.GC2212@tucnak> From: Uros Bizjak Date: Fri, 3 Apr 2020 19:18:49 +0200 Message-ID: Subject: Re: [PATCH] i386: Fix vph{add, subs?}[wd] 256-bit AVX2 RTL patterns [PR94460] To: Jakub Jelinek Cc: Jeff Law , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SCC_5_SHORT_WORD_LINES, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Apr 2020 17:19:03 -0000 On Fri, Apr 3, 2020 at 7:06 PM Jakub Jelinek wrote: > > Hi! > > The following testcase is miscompiled, because the AVX2 patterns don't > describe correctly what the insn does. E.g. vphaddd with %ymm* operands > (the second pattern) instruction as per: > https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm256_hadd_epi32&expand=2941 > does { a0+a1, a2+a3, b0+b1, b2+b3, a4+a5, a6+a7, b4+b5, b6+b7 } > but our RTL pattern did > { a0+a1, a2+a3, a4+a5, a6+a7, b0+b1, b2+b3, b4+b5, b6+b7 } > where the first and last 64 bits are the same and two middle 64 bits > swapped. > https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm256_hadd_epi16&expand=2939 > similarly, insn does: > { a0+a1, a2+a3, a4+a5, a6+a7, b0+b1, b2+b3, b4+b5, b6+b7, > a8+a9, a10+a11, a12+a13, a14+a15, b8+b9, b10+b11, b12+b13, b14+b15 } > but RTL pattern did > { a0+a1, a2+a3, a4+a5, a6+a7, a8+a9, a10+a11, a12+a13, a14+a15, > b0+b1, b2+b3, b4+b5, b6+b7, b8+b9, b10+b11, b12+b13, b14+b15 } > again, first and last 64 bits are the same and the two middle 64 bits > swapped. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? > > As a follow-up for GCC11, I have simplification for the patterns. > > 2020-04-03 Jakub Jelinek > > PR target/94460 > * config/i386/sse.md (avx2_phwv16hi3, > avx2_phdv8si3): Fix up RTL pattern to do > second half of first lane from first lane of second operand and > first half of second lane from second lane of first operand. > > * gcc.target/i386/avx2-pr94460.c: New test. OK for trunk and backports. Thanks, Uros. > --- gcc/config/i386/sse.md.jj 2020-03-30 18:04:31.942435289 +0200 > +++ gcc/config/i386/sse.md 2020-04-03 10:21:51.110564277 +0200 > @@ -16060,22 +16060,6 @@ (define_insn "avx2_ph (vec_concat:V4HI > (vec_concat:V2HI > (ssse3_plusminus:HI > - (vec_select:HI (match_dup 1) (parallel [(const_int 8)])) > - (vec_select:HI (match_dup 1) (parallel [(const_int 9)]))) > - (ssse3_plusminus:HI > - (vec_select:HI (match_dup 1) (parallel [(const_int 10)])) > - (vec_select:HI (match_dup 1) (parallel [(const_int 11)])))) > - (vec_concat:V2HI > - (ssse3_plusminus:HI > - (vec_select:HI (match_dup 1) (parallel [(const_int 12)])) > - (vec_select:HI (match_dup 1) (parallel [(const_int 13)]))) > - (ssse3_plusminus:HI > - (vec_select:HI (match_dup 1) (parallel [(const_int 14)])) > - (vec_select:HI (match_dup 1) (parallel [(const_int 15)])))))) > - (vec_concat:V8HI > - (vec_concat:V4HI > - (vec_concat:V2HI > - (ssse3_plusminus:HI > (vec_select:HI > (match_operand:V16HI 2 "nonimmediate_operand" "xm") > (parallel [(const_int 0)])) > @@ -16089,7 +16073,23 @@ (define_insn "avx2_ph (vec_select:HI (match_dup 2) (parallel [(const_int 5)]))) > (ssse3_plusminus:HI > (vec_select:HI (match_dup 2) (parallel [(const_int 6)])) > - (vec_select:HI (match_dup 2) (parallel [(const_int 7)]))))) > + (vec_select:HI (match_dup 2) (parallel [(const_int 7)])))))) > + (vec_concat:V8HI > + (vec_concat:V4HI > + (vec_concat:V2HI > + (ssse3_plusminus:HI > + (vec_select:HI (match_dup 1) (parallel [(const_int 8)])) > + (vec_select:HI (match_dup 1) (parallel [(const_int 9)]))) > + (ssse3_plusminus:HI > + (vec_select:HI (match_dup 1) (parallel [(const_int 10)])) > + (vec_select:HI (match_dup 1) (parallel [(const_int 11)])))) > + (vec_concat:V2HI > + (ssse3_plusminus:HI > + (vec_select:HI (match_dup 1) (parallel [(const_int 12)])) > + (vec_select:HI (match_dup 1) (parallel [(const_int 13)]))) > + (ssse3_plusminus:HI > + (vec_select:HI (match_dup 1) (parallel [(const_int 14)])) > + (vec_select:HI (match_dup 1) (parallel [(const_int 15)]))))) > (vec_concat:V4HI > (vec_concat:V2HI > (ssse3_plusminus:HI > @@ -16224,21 +16224,21 @@ (define_insn "avx2_ph (vec_select:SI (match_dup 1) (parallel [(const_int 3)])))) > (vec_concat:V2SI > (plusminus:SI > - (vec_select:SI (match_dup 1) (parallel [(const_int 4)])) > - (vec_select:SI (match_dup 1) (parallel [(const_int 5)]))) > - (plusminus:SI > - (vec_select:SI (match_dup 1) (parallel [(const_int 6)])) > - (vec_select:SI (match_dup 1) (parallel [(const_int 7)]))))) > - (vec_concat:V4SI > - (vec_concat:V2SI > - (plusminus:SI > (vec_select:SI > (match_operand:V8SI 2 "nonimmediate_operand" "xm") > (parallel [(const_int 0)])) > (vec_select:SI (match_dup 2) (parallel [(const_int 1)]))) > (plusminus:SI > (vec_select:SI (match_dup 2) (parallel [(const_int 2)])) > - (vec_select:SI (match_dup 2) (parallel [(const_int 3)])))) > + (vec_select:SI (match_dup 2) (parallel [(const_int 3)]))))) > + (vec_concat:V4SI > + (vec_concat:V2SI > + (plusminus:SI > + (vec_select:SI (match_dup 1) (parallel [(const_int 4)])) > + (vec_select:SI (match_dup 1) (parallel [(const_int 5)]))) > + (plusminus:SI > + (vec_select:SI (match_dup 1) (parallel [(const_int 6)])) > + (vec_select:SI (match_dup 1) (parallel [(const_int 7)])))) > (vec_concat:V2SI > (plusminus:SI > (vec_select:SI (match_dup 2) (parallel [(const_int 4)])) > --- gcc/testsuite/gcc.target/i386/avx2-pr94460.c.jj 2020-04-03 11:21:20.932237701 +0200 > +++ gcc/testsuite/gcc.target/i386/avx2-pr94460.c 2020-04-03 11:20:55.566617063 +0200 > @@ -0,0 +1,31 @@ > +/* PR target/94460 */ > +/* { dg-do run { target { avx2 && int128 } } } */ > +/* { dg-options "-O2 -mavx2" } */ > + > +#include > +#include "avx2-check.h" > + > +typedef __int128 v2ti __attribute__ ((__vector_size__ (32))); > + > +static inline v2ti > +foo (__v16hi b) > +{ > + return (v2ti) _mm256_hsub_epi16 ((__m256i) b, (__m256i) b); > +} > + > +static inline v2ti > +bar (__v8si b) > +{ > + return (v2ti) _mm256_hsub_epi32 ((__m256i) b, (__m256i) b); > +} > + > +static void > +avx2_test (void) > +{ > + v2ti x = foo ((__v16hi) { 1 }); > + if (x[0] != ((__int128)1 << 64 | 1) || x[1] != 0) > + abort (); > + x = bar ((__v8si) { 1 }); > + if (x[0] != ((__int128)1 << 64 | 1) || x[1] != 0) > + abort (); > +} > > Jakub >