From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 101485 invoked by alias); 25 Jul 2017 21:12:18 -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 100732 invoked by uid 89); 25 Jul 2017 21:12:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=H*Ad:D*de.ibm.com X-HELO: gate.crashing.org Received: from gate.crashing.org (HELO gate.crashing.org) (63.228.1.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 25 Jul 2017 21:12:15 +0000 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.13.8) with ESMTP id v6PKsQAu023450; Tue, 25 Jul 2017 15:54:32 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id v6PKr6qc023072; Tue, 25 Jul 2017 15:53:06 -0500 Date: Tue, 25 Jul 2017 21:12:00 -0000 From: Segher Boessenkool To: Jakub Jelinek Cc: Richard Biener , Uros Bizjak , David Edelsohn , Marcus Shawcroft , Richard Earnshaw , Andreas Krebbel , Matthew Fortune , Eric Botcazou , Andrew Jenner , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846) Message-ID: <20170725205256.GI13471@gate.crashing.org> References: <20170725091432.GQ2123@tucnak> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170725091432.GQ2123@tucnak> User-Agent: Mutt/1.4.2.3i X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg01610.txt.bz2 Hi Jakub, On Tue, Jul 25, 2017 at 11:14:32AM +0200, Jakub Jelinek wrote: > The following patch adjusts the vec_init and vec_extract optabs, so that > they don't have in the expander names just the vector mode, but also another > mode, for vec_extract the mode of the result and for vec_init the mode of > the elts of the vector passed as second operand. > > Without this patch, the second mode has been implicit, GET_MODE_INNER of > the vector mode, so one could just extract a single element from a vector > or construct vector from elements. While that is most common, we allow > in GIMPLE e.g. construction of V8DImode from 4 V2DImode elements etc. > and the vectorizer uses them. By having the second mode in the name > it allows the generic code (vectorizer, expansion) to query whether the > backend supports such vector from vector expansions or inits from vector > elts and use them if available. > > For vec_extract, if we say want to extract high V2SImode from V4SImode > the fallback is try to expand it as DImode extraction from V2DImode. > This works well in many cases, but doesn't really work for very large > vectors, say if we want to extract high V8SImode from V16SImode on x86, > we'd need OImode extraction from V2OImode, which is something the backend > doesn't have any support for. > For vec_init, the fallback is usually to go through memory, which is slow in > many cases. > > This patch only adds new vector from vector extract and init patterns to > the i386 backend, but I had to change many other targets too, because > it needs to have the element mode in the vec_extract/vec_init expander > names. Seems most of the backends didn't really have a mode attribute > usable for this or had it only in uppercase, while for the names we need > lowercase. Some backends had a convention on how to name lower case > vs. upper case modes, others didn't have any. So I'm CCing maintainers > of affected backends to seek advice on what mode attributes they want to > use. Would it be possible (and useful) to _also_ keep the old names? Or do you expect all targets will want to support all combinations? > --- gcc/config/rs6000/vector.md.jj 2017-06-08 20:50:49.000000000 +0200 > +++ gcc/config/rs6000/vector.md 2017-07-24 17:44:44.699580927 +0200 > @@ -74,6 +74,16 @@ (define_mode_attr VEC_base [(V16QI "QI") > (V1TI "TI") > (TI "TI")]) > > +;; As above, but in lower case > +(define_mode_attr VEC_base_l [(V16QI "qi") > + (V8HI "hi") > + (V4SI "si") > + (V2DI "di") > + (V4SF "sf") > + (V2DF "df") > + (V1TI "ti") > + (TI "ti")]) > + > ;; Same size integer type for floating point data > (define_mode_attr VEC_int [(V4SF "v4si") > (V2DF "v2di")]) > @@ -520,6 +520,17 @@ (define_mode_attr VEL [(V8QI "QI") (V16Q > (SI "SI") (HI "HI") > (QI "QI")]) > > +;; Define element mode for each vector mode (lower case). > +(define_mode_attr Vel [(V8QI "qi") (V16QI "qi") > + (V4HI "hi") (V8HI "hi") > + (V2SI "si") (V4SI "si") > + (DI "di") (V2DI "di") > + (V4HF "hf") (V8HF "hf") > + (V2SF "sf") (V4SF "sf") > + (V2DF "df") (DF "df") > + (SI "si") (HI "hi") > + (QI "qi")]) (Inconsistent spacing, please fix). ("vel" instead of "Vel" for this name?) How is this different from VEC_base_l? They can just be merged it seems. (And for that matter, VEC_base and VEL as well). We'll handle that I suppose, don't let it hold up this patch :-) Segher