From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128453 invoked by alias); 18 Sep 2015 13:21:13 -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 128441 invoked by uid 89); 18 Sep 2015 13:21:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-io0-f177.google.com Received: from mail-io0-f177.google.com (HELO mail-io0-f177.google.com) (209.85.223.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 18 Sep 2015 13:21:09 +0000 Received: by iofb144 with SMTP id b144so56133344iof.1 for ; Fri, 18 Sep 2015 06:21:07 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.107.160.196 with SMTP id j187mr14170736ioe.91.1442582467277; Fri, 18 Sep 2015 06:21:07 -0700 (PDT) Received: by 10.36.73.14 with HTTP; Fri, 18 Sep 2015 06:21:07 -0700 (PDT) In-Reply-To: <55FAF9DD.6080505@redhat.com> References: <20150901130800.GA55610@msticlxl57.ims.intel.com> <55EA0230.3000102@redhat.com> <20150915135233.GA26618@msticlxl57.ims.intel.com> <55FAF9DD.6080505@redhat.com> Date: Fri, 18 Sep 2015 13:26:00 -0000 Message-ID: Subject: Re: [RFC] Try vector as a new representation for vector masks From: Ilya Enkovich To: Richard Henderson Cc: Jeff Law , Richard Biener , GCC Patches Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg01406.txt.bz2 2015-09-17 20:35 GMT+03:00 Richard Henderson : > On 09/15/2015 06:52 AM, Ilya Enkovich wrote: >> I made a step forward forcing vector comparisons have a mask (vec)= result and disabling bool patterns in case vector comparison is supported = by target. Several issues were met. >> >> - c/c++ front-ends generate vector comparison with integer vector resul= t. I had to make some modifications to use vec_cond instead. Don't know i= f there are other front-ends producing vector comparisons. >> - vector lowering fails to expand vector masks due to mismatch of type = and mode sizes. I fixed vector type size computation to match mode size an= d added a special handling of mask expand. >> - I disabled canonical type creation for vector mask because we can't l= ayout it with VOID mode. I don't know why we may need a canonical type here= . But get_mask_mode call may be moved into type layout to get it. >> - Expand of vec constants/contstructors requires special handling= . Common case should require target hooks/optabs to expand vector into req= uired mode. But I suppose we want to have a generic code to handle vector = of int mode case to avoid modification of multiple targets which use defaul= t vec modes. >> >> Currently 'make check' shows two types of regression. >> - missed vector expression pattern recongnition (MIN, MAX, ABX, VEC_CO= ND). This must be due to my front-end changes. Hope it will be easy to fi= x. >> - missed vectorization. All of them appear due to bool patterns disabl= ing. I didn't look into all of them but it seems the main problem is in mi= xed type sizes. With bool patterns and integer vector masks we just put in= t->(other sized int) conversion for masks and it gives us required mask tra= nsformation. With boolean mask we don't have a proper scalar statements to= do that. I think mask widening/narrowing may be directly supported in mas= ked statements vectorization. Going to look into it. >> >> I attach what I currently have for a prototype. It grows bigger so I sp= lit into several parts. > > The general approach looks good. > Great! > >> +/* By defaults a vector of integers is used as a mask. */ >> + >> +machine_mode >> +default_get_mask_mode (unsigned nunits, unsigned vector_size) >> +{ >> + unsigned elem_size =3D vector_size / nunits; >> + machine_mode elem_mode >> + =3D smallest_mode_for_size (elem_size * BITS_PER_UNIT, MODE_INT); > > Why these arguments as opposed to passing elem_size? It seems that every= hook > is going to have to do this division... Every target would have nunits =3D vector_size / elem_size because nunits is used to create a vector mode. Thus no difference. > >> +#define VECTOR_MASK_TYPE_P(TYPE) \ >> + (TREE_CODE (TYPE) =3D=3D VECTOR_TYPE \ >> + && TREE_CODE (TREE_TYPE (TYPE)) =3D=3D BOOLEAN_TYPE) > > Perhaps better as VECTOR_BOOLEAN_TYPE_P, since that's exactly what's bein= g tested? OK > >> @@ -3464,10 +3464,10 @@ verify_gimple_comparison (tree type, tree op0, t= ree op1) >> return true; >> } >> } >> - /* Or an integer vector type with the same size and element count >> + /* Or a boolean vector type with the same element count >> as the comparison operand types. */ >> else if (TREE_CODE (type) =3D=3D VECTOR_TYPE >> - && TREE_CODE (TREE_TYPE (type)) =3D=3D INTEGER_TYPE) >> + && TREE_CODE (TREE_TYPE (type)) =3D=3D BOOLEAN_TYPE) > > VECTOR_BOOLEAN_TYPE_P. > >> @@ -122,7 +122,19 @@ tree_vec_extract (gimple_stmt_iterator *gsi, tree t= ype, >> tree t, tree bitsize, tree bitpos) >> { >> if (bitpos) >> - return gimplify_build3 (gsi, BIT_FIELD_REF, type, t, bitsize, bitpo= s); >> + { >> + if (TREE_CODE (type) =3D=3D BOOLEAN_TYPE) >> + { >> + tree itype >> + =3D build_nonstandard_integer_type (tree_to_uhwi (bitsize), 0); >> + tree field =3D gimplify_build3 (gsi, BIT_FIELD_REF, itype, t, >> + bitsize, bitpos); >> + return gimplify_build2 (gsi, NE_EXPR, type, field, >> + build_zero_cst (itype)); >> + } >> + else >> + return gimplify_build3 (gsi, BIT_FIELD_REF, type, t, bitsize, bitp= os); >> + } >> else >> return gimplify_build1 (gsi, VIEW_CONVERT_EXPR, type, t); >> } > > So... this is us lowering vector operations on a target that doesn't supp= ort > them. Which means that we get to decide what value is produced for a > comparison? Which means that we can settle on the "normal" -1, correct? > > Which means that we ought not need to extract the entire element and then > compare for non-zero, but rather simply extract a single bit from the ele= ment, > and directly call that a boolean result, correct? Didn't think about that. I'll give it a try. > > I assume you tested all this code with -mno-sse or equivalent arch defaul= t? I didn't make some special runs for that. Just used regression testing which seems to have such tests. > >> @@ -1885,7 +1885,9 @@ expand_MASK_LOAD (gcall *stmt) >> create_output_operand (&ops[0], target, TYPE_MODE (type)); >> create_fixed_operand (&ops[1], mem); >> create_input_operand (&ops[2], mask, TYPE_MODE (TREE_TYPE (maskt))); >> - expand_insn (optab_handler (maskload_optab, TYPE_MODE (type)), 3, ops= ); >> + expand_insn (convert_optab_handler (maskload_optab, TYPE_MODE (type), >> + TYPE_MODE (TREE_TYPE (maskt))), >> + 3, ops); > > Why do we now need a conversion here? Mask mode was implicit for masked loads and stores. Now it becomes explicit because we may load the same value using different masks. E.g. for i386 we may load 256bit vector using both vector and scalar masks. > >> + if (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE ((op0)))) !=3D MODE_VECT= OR_INT) >> + { >> + /* This is a vcond with mask. To be supported soon... */ >> + gcc_unreachable (); >> + } > > Leave this out til we need it? I can't see that you replace this later i= n the > patch series... Currently we just shouldn't generate such vec_cond. But later I want to enable such statements generation and this will need to handle non vector masks. I don't have it in my patches set yet. But it is not finished and have lots of unfinished work. I don't expect detailed review for these patches. Once we make a decision that this representation works fine and we want to have, I'll address opened issues and send it a separate series. Thanks, Ilya > > > r~