From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5280 invoked by alias); 9 Sep 2019 10:09:53 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 5272 invoked by uid 89); 9 Sep 2019 10:09:53 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=uros X-HELO: mail-io1-f66.google.com Received: from mail-io1-f66.google.com (HELO mail-io1-f66.google.com) (209.85.166.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Sep 2019 10:09:51 +0000 Received: by mail-io1-f66.google.com with SMTP id r26so27228558ioh.8 for ; Mon, 09 Sep 2019 03:09:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fKGRmxcm4vfqHdY2li90CTyLlUQApvL8kA5OTsisysE=; b=Cvm4EyDDQcBq2nioKndvt2Vv0A30TtDGZgD0WwOrUYPCE1L3qgEADgHhNLMWLmWxzd KAjEB6hbL5A7GXeYUREwsqPjHJnwJ/jyWbGCZx9+aoHi67oi++laB0XySJL3uBgrxPj7 1QeNTdQcVC0HcOnaJEcbSixjNTUR/Ng3MJ7mmrnUz6OFAUecRlk2waieBZAUbUlqGKDs 3+9IqwHS+6npymZ/TrVHyvG1X2PCY0iW77tr18suUTfR+Cs4A4SyLZqQB9OSMqt1ceSl 7jtHjPFQT3lty2JZvnN6D+6Yum9JeZllqTYi7d0ZCOONf7Hn6Uhx54yZNwXKUFqrslsV QR1g== MIME-Version: 1.0 References: <20190909091811.GO2120@tucnak> In-Reply-To: <20190909091811.GO2120@tucnak> From: Uros Bizjak Date: Mon, 09 Sep 2019 10:09:00 -0000 Message-ID: Subject: Re: [PATCH] Fix up -funsigned-char behavior of _mm256_cmpgt_epi8 (PR target/91704) To: Jakub Jelinek Cc: "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2019-09/txt/msg00483.txt.bz2 On Mon, Sep 9, 2019 at 11:18 AM Jakub Jelinek wrote: > > Hi! > > This PR is a repetition of PR87853, just for avx2 instead of sse2. > See https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00195.html > for the previous patch. > > This time there is just one intrinsic with the problem (note, the previous > patch didn't have to change _mm_cmpeq_epi8, as __v16qi vs. __v16qs doesn't > make any difference for equality comparisons). Can you please change the above intrinsic back to __v16qi? IMO _mm and __mm256 intrinsics should be consistent between each other as much as possible. > I've grepped for similar issues in other headers, including 512-byte > vectors, but couldn't find any, in those cases we use a builtin with a mask > argument. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and > release branches? I've also noticed PR87853 didn't come up with a testcase, > so that is also attached, ok to commit too? Sure! > As for tests, I chose not to do a dg-do run test with -funsigned-char, > because that option is an ABI change and when including some headers that > also include system headers one is never sure what will become of that. > > 2019-09-09 Jakub Jelinek > > PR target/91704 > * config/i386/avxintrin.h (__v32qs): New typedef. > * config/i386/avx2intrin.h (_mm256_cmpgt_epi8): Use casts to __v32qs > instead of __v32qi. > > * gcc.target/i386/pr91704.c: New test. OK. Thanks, Uros. > --- gcc/config/i386/avxintrin.h.jj 2019-08-12 17:55:19.039139772 +0200 > +++ gcc/config/i386/avxintrin.h 2019-09-08 23:22:11.829573162 +0200 > @@ -47,6 +47,7 @@ typedef unsigned int __v8su __attribute_ > typedef short __v16hi __attribute__ ((__vector_size__ (32))); > typedef unsigned short __v16hu __attribute__ ((__vector_size__ (32))); > typedef char __v32qi __attribute__ ((__vector_size__ (32))); > +typedef signed char __v32qs __attribute__ ((__vector_size__ (32))); > typedef unsigned char __v32qu __attribute__ ((__vector_size__ (32))); > > /* The Intel API is flexible enough that we must allow aliasing with other > --- gcc/config/i386/avx2intrin.h.jj 2019-01-01 12:37:32.000731417 +0100 > +++ gcc/config/i386/avx2intrin.h 2019-09-08 23:24:23.391560853 +0200 > @@ -258,7 +258,7 @@ extern __inline __m256i > __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) > _mm256_cmpgt_epi8 (__m256i __A, __m256i __B) > { > - return (__m256i) ((__v32qi)__A > (__v32qi)__B); > + return (__m256i) ((__v32qs)__A > (__v32qs)__B); > } > > extern __inline __m256i > --- gcc/testsuite/gcc.target/i386/pr91704.c.jj 2019-09-09 11:01:14.588282654 +0200 > +++ gcc/testsuite/gcc.target/i386/pr91704.c 2019-09-09 11:09:55.659355290 +0200 > @@ -0,0 +1,14 @@ > +/* PR target/91704 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -funsigned-char -mavx2 -mavx512f -masm=att" } */ > +/* { dg-final { scan-assembler-times "\tvpcmpgtb\t%ymm" 1 } } */ > +/* { dg-final { scan-assembler-not "\tvpsubusb\t" } } */ > +/* { dg-final { scan-assembler-not "\tvpcmpeqb\t" } } */ > + > +#include > + > +__m256i > +foo (__m256i x, __m256i y) > +{ > + return _mm256_cmpgt_epi8 (x, y); > +} > > Jakub