From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70069 invoked by alias); 25 Jul 2018 08:46:09 -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 68955 invoked by uid 89); 25 Jul 2018 08:45:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=grandchild, Built, sk:vect_bu, group_size X-HELO: mail-lj1-f180.google.com Received: from mail-lj1-f180.google.com (HELO mail-lj1-f180.google.com) (209.85.208.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 25 Jul 2018 08:45:29 +0000 Received: by mail-lj1-f180.google.com with SMTP id q5-v6so5953979ljh.12 for ; Wed, 25 Jul 2018 01:45:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=Wi1fGp1oFfKutkd813gtEv/pLh5zMEOy09PtzxBpnYU=; b=UB7xZgmEP0P7raL2nUWy2rpQXDTADvIzZbi28aY/4pjJEx2mgquJ1eGlCY+Ufco2+4 1WXOR067kDrtvdi1JBmvJbeXU5EGGHS8oujeF3HUNEc7RU29HUrEkFJNikRzjhHGSbmX x8D38AkKOB3pDojczipbbSEQyJ3puYhwLHvBhm8A9d+ju9d3beaxg9DnUaRyaMFSUu/s rCVDzKK8rV3/n3UC7/sX0vdIGJwFX2qpq8iOzfmvmG9k9iP6uhCbWrcyWp+AVdsCEt2m TKStHAT89FxGwrDO6Yg6hD32YKH6S3Q20VrmbGpg3tt10fD79F/APCPhxdSrLK6hChuj mrbQ== MIME-Version: 1.0 References: <87wotlrmen.fsf@arm.com> <87k1plrmc1.fsf@arm.com> In-Reply-To: <87k1plrmc1.fsf@arm.com> From: Richard Biener Date: Wed, 25 Jul 2018 08:46:00 -0000 Message-ID: Subject: Re: [03/46] Remove unnecessary update of NUM_SLP_USES To: GCC Patches , richard.sandiford@arm.com Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-07/txt/msg01464.txt.bz2 On Tue, Jul 24, 2018 at 11:53 AM Richard Sandiford wrote: > > vect_free_slp_tree had: > > gimple *stmt; > FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt) > /* After transform some stmts are removed and thus their vinfo is gone. */ > if (vinfo_for_stmt (stmt)) > { > gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0); > STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--; > } > > But after transform this update is redundant even for statements that do > exist, so it seems better to skip this loop for the final teardown. OK > > 2018-07-24 Richard Sandiford > > gcc/ > * tree-vectorizer.h (vect_free_slp_instance): Add a final_p parameter. > * tree-vect-slp.c (vect_free_slp_tree): Likewise. Don't update > STMT_VINFO_NUM_SLP_USES when it's true. > (vect_free_slp_instance): Add a final_p parameter and pass it to > vect_free_slp_tree. > (vect_build_slp_tree_2): Update call to vect_free_slp_instance. > (vect_analyze_slp_instance): Likewise. > (vect_slp_analyze_operations): Likewise. > (vect_slp_analyze_bb_1): Likewise. > * tree-vectorizer.c (vec_info): Likewise. > * tree-vect-loop.c (vect_transform_loop): Likewise. > > Index: gcc/tree-vectorizer.h > =================================================================== > --- gcc/tree-vectorizer.h 2018-07-03 10:59:30.480481417 +0100 > +++ gcc/tree-vectorizer.h 2018-07-24 10:22:09.237496975 +0100 > @@ -1634,7 +1634,7 @@ extern int vect_get_known_peeling_cost ( > extern tree cse_and_gimplify_to_preheader (loop_vec_info, tree); > > /* In tree-vect-slp.c. */ > -extern void vect_free_slp_instance (slp_instance); > +extern void vect_free_slp_instance (slp_instance, bool); > extern bool vect_transform_slp_perm_load (slp_tree, vec , > gimple_stmt_iterator *, poly_uint64, > slp_instance, bool, unsigned *); > Index: gcc/tree-vect-slp.c > =================================================================== > --- gcc/tree-vect-slp.c 2018-07-23 16:58:06.000000000 +0100 > +++ gcc/tree-vect-slp.c 2018-07-24 10:22:09.237496975 +0100 > @@ -47,25 +47,32 @@ Software Foundation; either version 3, o > #include "internal-fn.h" > > > -/* Recursively free the memory allocated for the SLP tree rooted at NODE. */ > +/* Recursively free the memory allocated for the SLP tree rooted at NODE. > + FINAL_P is true if we have vectorized the instance or if we have > + made a final decision not to vectorize the statements in any way. */ > > static void > -vect_free_slp_tree (slp_tree node) > +vect_free_slp_tree (slp_tree node, bool final_p) > { > int i; > slp_tree child; > > FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (node), i, child) > - vect_free_slp_tree (child); > + vect_free_slp_tree (child, final_p); > > - gimple *stmt; > - FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt) > - /* After transform some stmts are removed and thus their vinfo is gone. */ > - if (vinfo_for_stmt (stmt)) > - { > - gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0); > - STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--; > - } > + /* Don't update STMT_VINFO_NUM_SLP_USES if it isn't relevant. > + Some statements might no longer exist, after having been > + removed by vect_transform_stmt. Updating the remaining > + statements would be redundant. */ > + if (!final_p) > + { > + gimple *stmt; > + FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt) > + { > + gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0); > + STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--; > + } > + } > > SLP_TREE_CHILDREN (node).release (); > SLP_TREE_SCALAR_STMTS (node).release (); > @@ -76,12 +83,14 @@ vect_free_slp_tree (slp_tree node) > } > > > -/* Free the memory allocated for the SLP instance. */ > +/* Free the memory allocated for the SLP instance. FINAL_P is true if we > + have vectorized the instance or if we have made a final decision not > + to vectorize the statements in any way. */ > > void > -vect_free_slp_instance (slp_instance instance) > +vect_free_slp_instance (slp_instance instance, bool final_p) > { > - vect_free_slp_tree (SLP_INSTANCE_TREE (instance)); > + vect_free_slp_tree (SLP_INSTANCE_TREE (instance), final_p); > SLP_INSTANCE_LOADS (instance).release (); > free (instance); > } > @@ -1284,7 +1293,7 @@ vect_build_slp_tree_2 (vec_info *vinfo, > if (++this_tree_size > max_tree_size) > { > FOR_EACH_VEC_ELT (children, j, child) > - vect_free_slp_tree (child); > + vect_free_slp_tree (child, false); > vect_free_oprnd_info (oprnds_info); > return NULL; > } > @@ -1315,7 +1324,7 @@ vect_build_slp_tree_2 (vec_info *vinfo, > this_loads.truncate (old_nloads); > this_tree_size = old_tree_size; > FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (child), j, grandchild) > - vect_free_slp_tree (grandchild); > + vect_free_slp_tree (grandchild, false); > SLP_TREE_CHILDREN (child).truncate (0); > > dump_printf_loc (MSG_NOTE, vect_location, > @@ -1495,7 +1504,7 @@ vect_build_slp_tree_2 (vec_info *vinfo, > this_loads.truncate (old_nloads); > this_tree_size = old_tree_size; > FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (child), j, grandchild) > - vect_free_slp_tree (grandchild); > + vect_free_slp_tree (grandchild, false); > SLP_TREE_CHILDREN (child).truncate (0); > > dump_printf_loc (MSG_NOTE, vect_location, > @@ -1519,7 +1528,7 @@ vect_build_slp_tree_2 (vec_info *vinfo, > fail: > gcc_assert (child == NULL); > FOR_EACH_VEC_ELT (children, j, child) > - vect_free_slp_tree (child); > + vect_free_slp_tree (child, false); > vect_free_oprnd_info (oprnds_info); > return NULL; > } > @@ -2036,13 +2045,13 @@ vect_analyze_slp_instance (vec_info *vin > "Build SLP failed: store group " > "size not a multiple of the vector size " > "in basic block SLP\n"); > - vect_free_slp_tree (node); > + vect_free_slp_tree (node, false); > loads.release (); > return false; > } > /* Fatal mismatch. */ > matches[group_size / const_max_nunits * const_max_nunits] = false; > - vect_free_slp_tree (node); > + vect_free_slp_tree (node, false); > loads.release (); > } > else > @@ -2102,7 +2111,7 @@ vect_analyze_slp_instance (vec_info *vin > dump_gimple_stmt (MSG_MISSED_OPTIMIZATION, > TDF_SLIM, stmt, 0); > } > - vect_free_slp_instance (new_instance); > + vect_free_slp_instance (new_instance, false); > return false; > } > } > @@ -2133,7 +2142,7 @@ vect_analyze_slp_instance (vec_info *vin > dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, > "Built SLP cancelled: can use " > "load/store-lanes\n"); > - vect_free_slp_instance (new_instance); > + vect_free_slp_instance (new_instance, false); > return false; > } > } > @@ -2668,7 +2677,7 @@ vect_slp_analyze_operations (vec_info *v > dump_gimple_stmt (MSG_NOTE, TDF_SLIM, > SLP_TREE_SCALAR_STMTS > (SLP_INSTANCE_TREE (instance))[0], 0); > - vect_free_slp_instance (instance); > + vect_free_slp_instance (instance, false); > vinfo->slp_instances.ordered_remove (i); > cost_vec.release (); > } > @@ -2947,7 +2956,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera > dump_gimple_stmt (MSG_NOTE, TDF_SLIM, > SLP_TREE_SCALAR_STMTS > (SLP_INSTANCE_TREE (instance))[0], 0); > - vect_free_slp_instance (instance); > + vect_free_slp_instance (instance, false); > BB_VINFO_SLP_INSTANCES (bb_vinfo).ordered_remove (i); > continue; > } > Index: gcc/tree-vectorizer.c > =================================================================== > --- gcc/tree-vectorizer.c 2018-06-27 10:27:09.894649672 +0100 > +++ gcc/tree-vectorizer.c 2018-07-24 10:22:09.237496975 +0100 > @@ -466,7 +466,7 @@ vec_info::~vec_info () > unsigned int i; > > FOR_EACH_VEC_ELT (slp_instances, i, instance) > - vect_free_slp_instance (instance); > + vect_free_slp_instance (instance, true); > > destroy_cost_data (target_cost_data); > free_stmt_vec_infos (&stmt_vec_infos); > Index: gcc/tree-vect-loop.c > =================================================================== > --- gcc/tree-vect-loop.c 2018-07-24 10:22:06.269523330 +0100 > +++ gcc/tree-vect-loop.c 2018-07-24 10:22:09.237496975 +0100 > @@ -2229,7 +2229,7 @@ vect_analyze_loop_2 (loop_vec_info loop_ > LOOP_VINFO_VECT_FACTOR (loop_vinfo) = saved_vectorization_factor; > /* Free the SLP instances. */ > FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), j, instance) > - vect_free_slp_instance (instance); > + vect_free_slp_instance (instance, false); > LOOP_VINFO_SLP_INSTANCES (loop_vinfo).release (); > /* Reset SLP type to loop_vect on all stmts. */ > for (i = 0; i < LOOP_VINFO_LOOP (loop_vinfo)->num_nodes; ++i) > @@ -8683,7 +8683,7 @@ vect_transform_loop (loop_vec_info loop_ > won't work. */ > slp_instance instance; > FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), i, instance) > - vect_free_slp_instance (instance); > + vect_free_slp_instance (instance, true); > LOOP_VINFO_SLP_INSTANCES (loop_vinfo).release (); > /* Clear-up safelen field since its value is invalid after vectorization > since vectorized loop can have loop-carried dependencies. */