From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55798 invoked by alias); 14 Jul 2016 22:04:32 -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 55789 invoked by uid 89); 14 Jul 2016 22:04:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=grok, among X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 14 Jul 2016 22:04:21 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2230B13840; Thu, 14 Jul 2016 22:04:20 +0000 (UTC) Received: from localhost.localdomain (ovpn-116-70.phx2.redhat.com [10.3.116.70]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u6EM4JlN028826; Thu, 14 Jul 2016 18:04:19 -0400 Subject: Re: [PATCH, vec-tails 07/10] Support loop epilogue combining To: Ilya Enkovich References: <20160519194450.GH40563@msticlxl57.ims.intel.com> <18ccae1a-30c3-c23c-e28f-287f9d41eaa0@redhat.com> <20160628122439.GB4143@msticlxl57.ims.intel.com> Cc: gcc-patches@gcc.gnu.org From: Jeff Law Message-ID: Date: Thu, 14 Jul 2016 22:04:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160628122439.GB4143@msticlxl57.ims.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-07/txt/msg00880.txt.bz2 On 06/28/2016 06:24 AM, Ilya Enkovich wrote: > > Here is an updated patch version. > > Thanks, > Ilya > -- > gcc/ > > 2016-05-28 Ilya Enkovich > > * dbgcnt.def (vect_tail_combine): New. > * params.def (PARAM_VECT_COST_INCREASE_COMBINE_THRESHOLD): New. > * tree-vect-data-refs.c (vect_get_new_ssa_name): Support vect_mask_var. > * tree-vect-loop-manip.c (slpeel_tree_peel_loop_to_edge): Support > epilogue combined with loop body. > (vect_do_peeling_for_loop_bound): LIkewise. > (vect_do_peeling_for_alignment): ??? > * tree-vect-loop.c Include alias.h and dbgcnt.h. > (vect_estimate_min_profitable_iters): Add ret_min_profitable_combine_niters > arg, compute number of iterations for which loop epilogue combining is > profitable. > (vect_generate_tmps_on_preheader): Support combined apilogue. > (vect_gen_ivs_for_masking): New. > (vect_get_mask_index_for_elems): New. > (vect_get_mask_index_for_type): New. > (vect_gen_loop_masks): New. > (vect_mask_reduction_stmt): New. > (vect_mask_mask_load_store_stmt): New. > (vect_mask_load_store_stmt): New. > (vect_combine_loop_epilogue): New. > (vect_transform_loop): Support combined apilogue. > > > diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c > index 41b9380..08fad82 100644 > --- a/gcc/tree-vect-loop.c > +++ b/gcc/tree-vect-loop.c > @@ -6895,6 +7044,489 @@ vect_generate_tmps_on_preheader (loop_vec_info loop_vinfo, > return; > } > + > +/* Function vect_gen_loop_masks. > + > + Create masks to mask a loop described by LOOP_VINFO. Masks > + are created according to LOOP_VINFO_REQUIRED_MASKS and are stored > + into MASKS vector. > + > + Index of a mask in a vector is computed according to a number > + of masks's elements. Masks are sorted by number of its elements > + in descending order. Index 0 is used to access a mask with > + current_vector_size elements. Among masks with the same number > + of elements the one with lower index is used to mask iterations > + with smaller iteration counter. Note that you may get NULL elements > + for masks which are not required. Use vect_get_mask_index_for_elems > + or vect_get_mask_index_for_type to access resulting vector. */ > + > +static void > +vect_gen_loop_masks (loop_vec_info loop_vinfo, vec *masks) I find myself wondering if this ought to be broken down a bit (without changing the underlying semantics). > + > + /* Create narrowed masks. */ > + cur_mask_elems = iv_elems; > + nmasks = ivs.length (); > + while (cur_mask_elems < max_mask_elems) > + { > + prev_mask = vect_get_mask_index_for_elems (cur_mask_elems); > + > + cur_mask_elems <<= 1; > + nmasks >>= 1; > + > + cur_mask = vect_get_mask_index_for_elems (cur_mask_elems); > + > + mask_type = build_truth_vector_type (cur_mask_elems, vec_size); > + > + for (unsigned i = 0; i < nmasks; i++) > + { > + tree mask_low = (*masks)[prev_mask++]; > + tree mask_hi = (*masks)[prev_mask++]; > + mask = vect_get_new_ssa_name (mask_type, vect_mask_var); > + stmt = gimple_build_assign (mask, VEC_PACK_TRUNC_EXPR, > + mask_low, mask_hi); > + gsi_insert_before (&gsi, stmt, GSI_SAME_STMT); > + (*masks)[cur_mask++] = mask; > + } > + } For example, pull this into its own function as well as the code to create widened masks. In fact, didn't I see those functions in one of the other patches as their own separate subroutines? It's not a huge deal and I don't think it requires another round of review. I just found myself scrolling through multiple pages of this function and thought it'd be slightly easier to grok if were simply smaller. > + > +/* Function vect_mask_reduction_stmt. > + > + Mask given vectorized reduction statement STMT using > + MASK. In case scalar reduction statement is vectorized > + into several vector statements then PREV holds a > + preceding vetor statement copy for STMT. s/vetor/vector/ With the one function split up and the typo fix I think this is OK for the trunk when the set as a whole is ready. jeff