From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69828 invoked by alias); 16 Jun 2016 06:17:06 -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 69809 invoked by uid 89); 16 Jun 2016 06:17:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=him 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, 16 Jun 2016 06:16:54 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (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 6318E804E7; Thu, 16 Jun 2016 06:16:53 +0000 (UTC) Received: from localhost.localdomain (ovpn-116-111.phx2.redhat.com [10.3.116.111]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u5G6GpCj023799; Thu, 16 Jun 2016 02:16:51 -0400 Subject: Re: [PATCH, vec-tails 04/10] Add masking cost To: Ilya Enkovich , gcc-patches@gcc.gnu.org References: <20160519194036.GE40563@msticlxl57.ims.intel.com> From: Jeff Law Message-ID: <9259cabd-b505-679a-8345-1496c7e11b27@redhat.com> Date: Thu, 16 Jun 2016 06:17:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <20160519194036.GE40563@msticlxl57.ims.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-06/txt/msg01201.txt.bz2 On 05/19/2016 01:40 PM, Ilya Enkovich wrote: > Hi, > > This patch extends vectorizer cost model to include masking cost by > adding new cost model locations and new target hook to compute > masking cost. > > Thanks, > Ilya > -- > gcc/ > > 2016-05-19 Ilya Enkovich > > * config/i386/i386.c (ix86_init_cost): Extend costs array. > (ix86_add_stmt_masking_cost): New. > (ix86_finish_cost): Add masking_prologue_cost and masking_body_cost > args. > (TARGET_VECTORIZE_ADD_STMT_MASKING_COST): New. > * config/i386/i386.h (TARGET_INCREASE_MASK_STORE_COST): New. > * config/i386/x86-tune.def (X86_TUNE_INCREASE_MASK_STORE_COST): New. > * config/rs6000/rs6000.c (_rs6000_cost_data): Extend cost array. > (rs6000_init_cost): Initialize new cost elements. > (rs6000_finish_cost): Add masking_prologue_cost and masking_body_cost. > * config/spu/spu.c (spu_init_cost): Extend costs array. > (spu_finish_cost): Add masking_prologue_cost and masking_body_cost args. > * doc/tm.texi.in (TARGET_VECTORIZE_ADD_STMT_MASKING_COST): New. > * doc/tm.texi: Regenerated. > * target.def (add_stmt_masking_cost): New. > (finish_cost): Add masking_prologue_cost and masking_body_cost args. > * target.h (enum vect_cost_for_stmt): Add vector_mask_load and > vector_mask_store. > (enum vect_cost_model_location): Add vect_masking_prologue > and vect_masking_body. > * targhooks.c (default_builtin_vectorization_cost): Support > vector_mask_load and vector_mask_store. > (default_init_cost): Extend costs array. > (default_add_stmt_masking_cost): New. > (default_finish_cost): Add masking_prologue_cost and masking_body_cost > args. > * targhooks.h (default_add_stmt_masking_cost): New. > * tree-vect-loop.c (vect_estimate_min_profitable_iters): Adjust > finish_cost call. > * tree-vect-slp.c (vect_bb_vectorization_profitable_p): Likewise. > * tree-vectorizer.h (add_stmt_masking_cost): New. > (finish_cost): Add masking_prologue_cost and masking_body_cost args. > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 9f62089..6c2c364 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -53932,8 +53932,12 @@ ix86_spill_class (reg_class_t rclass, machine_mode mode) > static void * > ix86_init_cost (struct loop *) > { > - unsigned *cost = XNEWVEC (unsigned, 3); > - cost[vect_prologue] = cost[vect_body] = cost[vect_epilogue] = 0; > + unsigned *cost = XNEWVEC (unsigned, 5); > + cost[vect_prologue] = 0; > + cost[vect_body] = 0; > + cost[vect_epilogue] = 0; > + cost[vect_masking_prologue] = 0; > + cost[vect_masking_body] = 0; > return cost; Trivial nit -- no need or desire to use whitespace to line up the initializers. It looks like others may have done this in the duplicated instances of finish_cost. But we shouldn't propagate that mistake into the init_cost hooks ;-) @@ -1115,8 +1117,12 @@ default_get_mask_mode (unsigned nunits, unsigned vector_size) > void * > default_init_cost (struct loop *loop_info ATTRIBUTE_UNUSED) > { > - unsigned *cost = XNEWVEC (unsigned, 3); > - cost[vect_prologue] = cost[vect_body] = cost[vect_epilogue] = 0; > + unsigned *cost = XNEWVEC (unsigned, 5); > + cost[vect_prologue] = 0; > + cost[vect_body] = 0; > + cost[vect_epilogue] = 0; > + cost[vect_masking_prologue] = 0; > + cost[vect_masking_body] = 0; > return cost; Here too. There's others. I won't point them all out. Please double check for this nit in any added code. You don't have to go back and fix existing problems of this nature. I don't see anything here I really object to -- Richi and I may disagree on the compute-costs once in a single scan vs restarting the scan. If Richi feels strongly about restarting for some reason, I'll defer to him -- he's done more work in the vectorizer than myself. I'd suggest taking another stab at the docs for the hooks based on Richi's question about whether or not the hook returns the cost of hte masked statement or the cost of masking the statement. jeff