From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 785 invoked by alias); 9 Sep 2007 08:16:35 -0000 Received: (qmail 772 invoked by uid 22791); 9 Sep 2007 08:16:34 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate3.de.ibm.com (HELO mtagate3.de.ibm.com) (195.212.29.152) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sun, 09 Sep 2007 08:16:28 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate3.de.ibm.com (8.13.8/8.13.8) with ESMTP id l898GPYk163812 for ; Sun, 9 Sep 2007 08:16:25 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.5) with ESMTP id l898GPeF2199612 for ; Sun, 9 Sep 2007 10:16:25 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id l898GPEZ026407 for ; Sun, 9 Sep 2007 10:16:25 +0200 Received: from d12mc102.megacenter.de.ibm.com (d12mc102.megacenter.de.ibm.com [9.149.167.114]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id l898GNpK026376 for ; Sun, 9 Sep 2007 10:16:25 +0200 In-Reply-To: Subject: Re: [patch] Loop-aware SLP 4/5 To: Dorit Nuzman Cc: gcc-patches@gcc.gnu.org X-Mailer: Lotus Notes Release 7.0 HF277 June 21, 2006 Message-ID: From: Ira Rosen Date: Sun, 09 Sep 2007 09:05:00 -0000 MIME-Version: 1.0 Content-type: text/plain; charset=US-ASCII X-IsSubscribed: yes 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 X-SW-Source: 2007-09/txt/msg00750.txt.bz2 Dorit Nuzman/Haifa/IBM wrote on 09/09/2007 11:00:13: > Ira Rosen/Haifa/IBM wrote on 09/09/2007 10:12:40: > > > Dorit Nuzman/Haifa/IBM wrote on 18/08/2007 21:36:27: > > > > > Ira Rosen/Haifa/IBM wrote on 14/08/2007 16:15:19: > > > > > > > This part adds an adjustment of vectorization cost model to SLP. > > > > > > > > SLP costs are calculated for each SLP computation tree recursively > > > > during its analysis, and added to the overall costs if we decide to SLP. > > > > > > > > Thanks, > > > > Ira > > > > > > > > > > +/* SLP costs are calculated according to SLP instance unrolling > > factor (i.e., > > > + the number of created vector stmts depends on the unrolling > > > factor). However, > > > + the actual number of vector stmts for every SLP node depends on > > > VF which is > > > + set later in vect_analyze_operations(). Hence, SLP costs should > > > be updated. > > > + In this function we assume that the inside costs calculated in > > > + vect_model_xxx_cost are linear in ncopies. */ > > > > > > can you explain why this assumption holds (or when it may be broken, > > > and assert against that)? > > > > > > + /* We assume that costs are linear in ncopies. */ > > > + if (SLP_INSTANCE_UNROLLING_FACTOR (instance) != vf) > > > + SLP_INSTANCE_INSIDE_OF_LOOP_COST (instance) *= vf > > > + / SLP_INSTANCE_UNROLLING_FACTOR (instance); > > > > > > to me it's less confusing if you remove the 'if' (when unrolling==vf > > > you'll just multiply by 1, so no need for the special check). > > > > > > > Done. > > > > > and lastly, about the following repeating change: > > > > > > + int *inside_cost_field, *outside_cost_field; > > > + > > > + /* Take addresses of relevant fields to update in the function. */ > > > + if (slp_node) > > > + { > > > + inside_cost_field = &(SLP_TREE_INSIDE_OF_LOOP_COST (slp_node)); > > > + outside_cost_field = &(SLP_TREE_OUTSIDE_OF_LOOP_COST (slp_node)); > > > + } > > > + else > > > + { > > > + inside_cost_field = &(STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info)); > > > + outside_cost_field = &(STMT_VINFO_OUTSIDE_OF_LOOP_COST > (stmt_info)); > > > + } > > > ... > > > - STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info) = outer_cost; > > > + *outside_cost_field = outer_cost; > > > > > > inner_cost += ncopies * (TARG_VEC_LOAD_COST + > TARG_VEC_STMT_COST); > > > > > > @@ -607,12 +654,12 @@ vect_model_load_cost (stmt_vec_info stmt > > > gcc_unreachable (); > > > } > > > > > > - STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) = inner_cost; > > > + *inside_cost_field = inner_cost; > > > > > > Again, in an attempt to reduce "if (slp_node)" switches in the code, > > > I'd replace > > > STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) = cost; > > > STMT_VINFO_OUTSIDE_OF_LOOP_COST (stmt_info) = cost; > > > with > > > stmt_vinfo_inside_of_loop_cost (stmt_info, slp_node, cost); > > > stmt_vinfo_outside_of_loop_cost (stmt_info, slp_node, cost); > > > > > > ...and hide the "if (slp_node)" switch there: > > > stmt_vinfo_inside_of_loop_cost (stmt_info, slp_node, cost) > > > { > > > if (slp_node) > > > SLP_TREE_INSIDE_OF_LOOP_COST (slp_node) = cost; > > > else > > > STMT_VINFO_INSIDE_OF_LOOP_COST (stmt_info) = cost; > > > } > > > > > > (for your consideration...) > > > > I moved the choice of the relevant field to a different (inlined) function. > > > > I prefer the alternative I suggested above - it's a bit less code > (no need for the call to the selection function) and more > importantly the whole field selection machinery is entirely hidden. > Please consider changing (or explain why you prefer the other solution). > > In the meantime, ok to commit this patch and all the other parts as > well (the above change can be applied separately later). O.K. I will reconsider it. Committed as is meantime. Thanks, Ira > > thanks for addressing the comments, > > dorit > > > Thanks, > > Ira > > > > > > > > ok otherwise, > > > > > > thanks, > > > dorit > > > > > > > ChangeLog: > > > > > > > > * tree-vectorizer.h (_slp_tree): Add new fields for costs and their > > > > access functions. > > > > (_slp_instance): Likewise. > > > > (vect_model_simple_cost, vect_model_store_cost, vect_model_load_cost): > > > > Declare (make extern). > > > > * tree-vect-analyze.c (vect_update_slp_costs_according_to_vf): New. > > > > (vect_analyze_operations): Call vect_update_slp_costs_according_to_vf. > > > > (vect_get_and_check_slp_defs): Calculate costs. Add arguments. > > > > (vect_build_slp_tree): Likewise. > > > > (vect_analyze_slp_instance): Initialize cost fields. Update > > > > arguments of vect_build_slp_tree. > > > > * tree-vect-transform.c (vect_estimate_min_profitable_iters): Take > > > > SLP costs into account. > > > > (vect_model_simple_cost): Make extern, add SLP parameter and handle > > > > SLP. > > > > (vect_model_store_cost, vect_model_load_cost): Likewise. > > > > (vectorizable_call): Add argument to vect_model_simple_cost. > > > > (vectorizable_assignment): Call vect_model_simple_cost > only for not > > > > pure SLP stmts. > > > > (vectorizable_operation): Likewise. > > > > (vectorizable_type_demotion): Add argument to > > > > vect_model_simple_cost. > > > > (vectorizable_type_promotion): Likewise. > > > > (vectorizable_store): Call vect_model_simple_cost only > for not pure > > > > SLP stmts. > > > > (vectorizable_load): Likewise. > > > > > > > > [attachment "slp-part4.txt" deleted by Dorit Nuzman/Haifa/IBM]