From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 78576 invoked by alias); 13 Nov 2019 14:13: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 78565 invoked by uid 89); 13 Nov 2019 14:13:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=odr X-HELO: mail-lj1-f196.google.com Received: from mail-lj1-f196.google.com (HELO mail-lj1-f196.google.com) (209.85.208.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Nov 2019 14:13:04 +0000 Received: by mail-lj1-f196.google.com with SMTP id r7so2768647ljg.2 for ; Wed, 13 Nov 2019 06:13:04 -0800 (PST) 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 :cc; bh=CyDKrfH8kZObjDRobOREqnlOHWFeAmngVp6L/WMkIeM=; b=h8oWSpybsZx4tkcz+dTsV+KIsZ09/4VTVZ8G2CTaIBslndy9MxukGDe1QAXHhZ8xDW BRPw1AShjZIAMOJoSB2fj9QXCwjmwoSdGgFRqZLIJs0gdjfHaV172tlXklybgCGmjUq1 IXNWjJwY1kUOHp793iIiimWzn6s4qiMkPqP8B1S1nkAuP/ia7FVXWmC/vVpzFp+htl1l QkTP3YfdgKDfJ206AzQnRfmHpREzI2fVImf9YxBzntCo3cWchqfgRBEmt6igXSvqx+zF PAzp9AqK4g/r9EqM0SPvCm+ovi+ueu1yKxbnQyizUYxVq8Nel/vWVHHk3k29MNa6vlKU +16A== MIME-Version: 1.0 References: <20191109142556.gtcby5qjth5vxuts@smtp.gmail.com> <20191112133102.rral4n7sg4jsfjl2@smtp.gmail.com> In-Reply-To: <20191112133102.rral4n7sg4jsfjl2@smtp.gmail.com> From: Richard Biener Date: Wed, 13 Nov 2019 14:16:00 -0000 Message-ID: Subject: Re: [PATCH] Refactor tree-loop-distribution for thread safety To: Giuliano Belinassi Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg01043.txt.bz2 On Tue, Nov 12, 2019 at 2:31 PM Giuliano Belinassi wrote: > > Hi, Richard. > > On 11/12, Richard Biener wrote: > > On Sat, Nov 9, 2019 at 3:26 PM Giuliano Belinassi > > wrote: > > > > > > Hi all, > > > > > > This patch refactors tree-loop-distribution.c for thread safety without > > > use of C11 __thread feature. All global variables were moved to a struct > > > which is initialized at ::execute time. > > > > Thanks for working on this. I've been thinking on how to make this > > nicer which naturally leads to the use of C++ classes and member > > functions which get 'this' for free. This means all functions that > > make use of 'priv' in your patch would need to become member > > functions of the class and pass_loop_distribution::execute would > > wrap it like > > Wouldn't it require that we have one instance of the > `pass_loop_distribution` class for each thread? I don't know how > the pass manager would handle this at the current state, but probably > I will need to patch it in my branch. I don't mean to re-use pass_loop_distribution itself, just move all functions as methods into a new separate class you instantiate at pass_loop_distribution::execute time, so much like your patch but use 'this' instead of the explicit passing of the structure pointer. > > > > unsigned int > > pass_loop_distribution::execute (function *fun) > > { > > return priv_pass_vars().execute (fun); > > } > > > > > > please find a better name for 'priv_pass_vars' since you can't > > reuse that name for other passes due to C++ ODR rules. > > I would suggest 'loop_distribution'. > > > > Can you try if going this route works well? > > Of course :) > > > > > Thanks, > > Richard. > > > > > I can install this patch myself in trunk if it's OK. > > > > > > gcc/ChangeLog > > > 2019-11-09 Giuliano Belinassi > > > > > > * cfgloop.c (get_loop_body_in_custom_order): New. > > > * cfgloop.h (get_loop_body_in_custom_order): New prototype. > > > * tree-loop-distribution.c (struct priv_pass_vars): New. > > > (bb_top_order_cmp_r): New. > > > (create_rdg_vertices): Update prototype. > > > (stmts_from_loop): Same as above. > > > (update_for_merge): Same as above. > > > (partition_merge_into): Same as above. > > > (get_data_dependence): Same as above. > > > (data_dep_in_cycle_p): Same as above. > > > (update_type_for_merge): Same as above. > > > (build_rdg_partition_for-vertex): Same as above. > > > (classify_builtin_ldst): Same as above. > > > (classify_partition): Same as above. > > > (share_memory_accesses): Same as above. > > > (rdg_build_partitions): Same as above. > > > (pg_add_dependence_edges): Same as above. > > > (build_partition_graph): Same as above. > > > (merge_dep_scc_partitions): Same as above. > > > (break_alias_scc_partitions): Same as above. > > > (finalize_partitions): Same as above. > > > (distribute_loop): Same as above. > > > (bb_top_order_init): New function. > > > (bb_top_order_destroy): New function. > > > (pass_loop_distribution::execute): Initialize struct priv. > > > > > > Thank you, > > > Giuliano. > > Thank you, > Giuliano.