From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57061 invoked by alias); 26 Oct 2015 15:21:27 -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 57050 invoked by uid 89); 26 Oct 2015 15:21:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yk0-f180.google.com Received: from mail-yk0-f180.google.com (HELO mail-yk0-f180.google.com) (209.85.160.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 26 Oct 2015 15:21:25 +0000 Received: by ykdr3 with SMTP id r3so187013584ykd.1 for ; Mon, 26 Oct 2015 08:21:23 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.13.202.141 with SMTP id m135mr11894712ywd.305.1445872883444; Mon, 26 Oct 2015 08:21:23 -0700 (PDT) Received: by 10.37.117.136 with HTTP; Mon, 26 Oct 2015 08:21:23 -0700 (PDT) In-Reply-To: <20151014161352.GO63757@msticlxl57.ims.intel.com> References: <20151008151140.GE63757@msticlxl57.ims.intel.com> <20151014161352.GO63757@msticlxl57.ims.intel.com> Date: Mon, 26 Oct 2015 15:25:00 -0000 Message-ID: Subject: Re: [vec-cmp, patch 4/6] Support vector mask invariants From: Richard Biener To: Ilya Enkovich Cc: GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg02764.txt.bz2 On Wed, Oct 14, 2015 at 6:13 PM, Ilya Enkovich wrote: > On 14 Oct 13:50, Ilya Enkovich wrote: >> 2015-10-14 11:49 GMT+03:00 Richard Biener : >> > On Tue, Oct 13, 2015 at 4:52 PM, Ilya Enkovich wrote: >> >> I don't understand what you mean. vect_get_vec_def_for_operand has two >> >> changes made. >> >> 1. For boolean invariants use build_same_sized_truth_vector_type >> >> instead of get_vectype_for_scalar_type in case statement produces a >> >> boolean vector. This covers cases when we use invariants in >> >> comparison, AND, IOR, XOR. >> > >> > Yes, I understand we need this special-casing to differentiate between >> > the vector type >> > used for boolean-typed loads/stores and the type for boolean typed constants. >> > What happens if we mix them btw, like with >> > >> > _Bool b = bools[i]; >> > _Bool c = b || d; >> > ... >> > >> > ? >> >> Here both statements should get vector of char as a vectype and we >> never go VECTOR_BOOLEAN_TYPE_P way for them >> >> > >> >> 2. COND_EXPR is an exception because it has built-in boolean vector >> >> result not reflected in its vecinfo. Thus I added additional operand >> >> for vect_get_vec_def_for_operand to directly specify vectype for >> >> vector definition in case it is a loop invariant. >> >> So what do you propose to do with these changes? >> > >> > This is the change I don't like and don't see why we need it. It works today >> > and the comparison operands should be of appropriate type already? >> >> Today it works because we always create vector of integer constant. >> With boolean vectors it may be either integer vector or boolean vector >> depending on context. Consider: >> >> _Bool _1; >> int _2; >> >> _2 = _1 != 0 ? 0 : 1 >> >> We have two zero constants here requiring different vectypes. >> >> Ilya >> >> > >> > Richard. >> > >> >> Thanks, >> >> Ilya > > Here is an updated patch version. > > Thanks, > Ilya > -- > gcc/ > > 2015-10-14 Ilya Enkovich > > * expr.c (const_vector_mask_from_tree): New. > (const_vector_from_tree): Use const_vector_mask_from_tree > for boolean vectors. > * tree-vect-stmts.c (vect_init_vector): Support boolean vector > invariants. > (vect_get_vec_def_for_operand): Add VECTYPE arg. > (vectorizable_condition): Directly provide vectype for invariants > used in comparison. > * tree-vectorizer.h (vect_get_vec_def_for_operand): Add VECTYPE > arg. > > > diff --git a/gcc/expr.c b/gcc/expr.c > index b5ff598..ab25d1a 100644 > --- a/gcc/expr.c > +++ b/gcc/expr.c > @@ -11344,6 +11344,40 @@ try_tablejump (tree index_type, tree index_expr, tree minval, tree range, > return 1; > } > > +/* Return a CONST_VECTOR rtx representing vector mask for > + a VECTOR_CST of booleans. */ > +static rtx > +const_vector_mask_from_tree (tree exp) > +{ > + rtvec v; > + unsigned i; > + int units; > + tree elt; > + machine_mode inner, mode; > + > + mode = TYPE_MODE (TREE_TYPE (exp)); > + units = GET_MODE_NUNITS (mode); > + inner = GET_MODE_INNER (mode); > + > + v = rtvec_alloc (units); > + > + for (i = 0; i < VECTOR_CST_NELTS (exp); ++i) > + { > + elt = VECTOR_CST_ELT (exp, i); > + > + gcc_assert (TREE_CODE (elt) == INTEGER_CST); > + if (integer_zerop (elt)) > + RTVEC_ELT (v, i) = CONST0_RTX (inner); > + else if (integer_onep (elt) > + || integer_minus_onep (elt)) > + RTVEC_ELT (v, i) = CONSTM1_RTX (inner); > + else > + gcc_unreachable (); > + } > + > + return gen_rtx_CONST_VECTOR (mode, v); > +} > + > /* Return a CONST_VECTOR rtx for a VECTOR_CST tree. */ > static rtx > const_vector_from_tree (tree exp) > @@ -11359,6 +11393,9 @@ const_vector_from_tree (tree exp) > if (initializer_zerop (exp)) > return CONST0_RTX (mode); > > + if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp))) > + return const_vector_mask_from_tree (exp); > + > units = GET_MODE_NUNITS (mode); > inner = GET_MODE_INNER (mode); > > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c > index 6a52895..01168ae 100644 > --- a/gcc/tree-vect-stmts.c > +++ b/gcc/tree-vect-stmts.c > @@ -1308,7 +1308,22 @@ vect_init_vector (gimple *stmt, tree val, tree type, gimple_stmt_iterator *gsi) > if (!types_compatible_p (TREE_TYPE (type), TREE_TYPE (val))) > { > if (CONSTANT_CLASS_P (val)) > - val = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (type), val); > + { > + /* Can't use VIEW_CONVERT_EXPR for booleans because > + of possibly different sizes of scalar value and > + vector element. */ > + if (VECTOR_BOOLEAN_TYPE_P (type)) > + { > + if (integer_zerop (val)) > + val = build_int_cst (TREE_TYPE (type), 0); > + else if (integer_onep (val)) > + val = build_int_cst (TREE_TYPE (type), 1); > + else > + gcc_unreachable (); > + } > + else > + val = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (type), val); I think the existing code is fine with using fold_convert () here which should also work for the boolean types. So does just val = fold_convert (TREE_TYPE (type), val); work? > + } > else > { > new_temp = make_ssa_name (TREE_TYPE (type)); > @@ -1339,16 +1354,19 @@ vect_init_vector (gimple *stmt, tree val, tree type, gimple_stmt_iterator *gsi) > STMT_VINFO_VEC_STMT of the defining stmt holds the relevant def. > > In case OP is an invariant or constant, a new stmt that creates a vector def > - needs to be introduced. */ > + needs to be introduced. VECTYPE may be used to specify a required type for > + vector invariant. */ > > tree > -vect_get_vec_def_for_operand (tree op, gimple *stmt, tree *scalar_def) > +vect_get_vec_def_for_operand (tree op, gimple *stmt, tree *scalar_def, > + tree vectype) > { > tree vec_oprnd; > gimple *vec_stmt; > gimple *def_stmt; > stmt_vec_info def_stmt_info = NULL; > stmt_vec_info stmt_vinfo = vinfo_for_stmt (stmt); > + tree stmt_vectype = STMT_VINFO_VECTYPE (stmt_vinfo); > unsigned int nunits; > loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo); > tree def; > @@ -1392,7 +1410,14 @@ vect_get_vec_def_for_operand (tree op, gimple *stmt, tree *scalar_def) > /* Case 1: operand is a constant. */ > case vect_constant_def: > { > - vector_type = get_vectype_for_scalar_type (TREE_TYPE (op)); > + if (vectype) > + vector_type = vectype; > + else if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE > + && VECTOR_BOOLEAN_TYPE_P (stmt_vectype)) > + vector_type = build_same_sized_truth_vector_type (stmt_vectype); > + else > + vector_type = get_vectype_for_scalar_type (TREE_TYPE (op)); > + > gcc_assert (vector_type); > nunits = TYPE_VECTOR_SUBPARTS (vector_type); > > @@ -1410,7 +1435,13 @@ vect_get_vec_def_for_operand (tree op, gimple *stmt, tree *scalar_def) > /* Case 2: operand is defined outside the loop - loop invariant. */ > case vect_external_def: > { > - vector_type = get_vectype_for_scalar_type (TREE_TYPE (def)); > + if (vectype) > + vector_type = vectype; > + else if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE > + && VECTOR_BOOLEAN_TYPE_P (stmt_vectype)) > + vector_type = build_same_sized_truth_vector_type (stmt_vectype); > + else > + vector_type = get_vectype_for_scalar_type (TREE_TYPE (def)); > gcc_assert (vector_type); > > if (scalar_def) > @@ -7428,13 +7459,13 @@ vectorizable_condition (gimple *stmt, gimple_stmt_iterator *gsi, > gimple *gtemp; > vec_cond_lhs = > vect_get_vec_def_for_operand (TREE_OPERAND (cond_expr, 0), > - stmt, NULL); > + stmt, NULL, comp_vectype); > vect_is_simple_use (TREE_OPERAND (cond_expr, 0), stmt, > loop_vinfo, >emp, &def, &dts[0]); > > vec_cond_rhs = > vect_get_vec_def_for_operand (TREE_OPERAND (cond_expr, 1), > - stmt, NULL); > + stmt, NULL, comp_vectype); > vect_is_simple_use (TREE_OPERAND (cond_expr, 1), stmt, > loop_vinfo, >emp, &def, &dts[1]); I still don't like this very much but I guess without some major refactoring of all the functions there isn't a better way to do it for now. Thus, ok with trying the change suggested above. Thanks, Richard. > if (reduc_index == 1) > diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h > index f8d1e97..8eca7be 100644 > --- a/gcc/tree-vectorizer.h > +++ b/gcc/tree-vectorizer.h > @@ -994,7 +994,7 @@ extern unsigned record_stmt_cost (stmt_vector_for_cost *, int, > extern void vect_finish_stmt_generation (gimple *, gimple *, > gimple_stmt_iterator *); > extern bool vect_mark_stmts_to_be_vectorized (loop_vec_info); > -extern tree vect_get_vec_def_for_operand (tree, gimple *, tree *); > +extern tree vect_get_vec_def_for_operand (tree, gimple *, tree *, tree = NULL); > extern tree vect_init_vector (gimple *, tree, tree, > gimple_stmt_iterator *); > extern tree vect_get_vec_def_for_stmt_copy (enum vect_def_type, tree);