From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id CD4D63858400 for ; Mon, 11 Jul 2022 07:20:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CD4D63858400 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id A1705226F6; Mon, 11 Jul 2022 07:20:26 +0000 (UTC) Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 9B3FA2C141; Mon, 11 Jul 2022 07:20:26 +0000 (UTC) Date: Mon, 11 Jul 2022 07:20:26 +0000 (UTC) From: Richard Biener To: Roger Sayle cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Move reload_completed and other rtl.h globals to crtl structure. In-Reply-To: <00e601d89489$a5133740$ef39a5c0$@nextmovesoftware.com> Message-ID: References: <00e601d89489$a5133740$ef39a5c0$@nextmovesoftware.com> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Jul 2022 07:20:29 -0000 On Sun, 10 Jul 2022, Roger Sayle wrote: > > This patch builds upon Richard Biener's suggestion of avoiding global > variables to track state/identify which passes have already been run. > In the early middle-end, the tree-ssa passes use the curr_properties > field in cfun to track this. This patch uses a new rtl_pass_progress > int field in crtl to do something similar. Why not simply add PROP_rtl_... and use the existing curr_properties for this? RTL passes are also passes and this has the advantage you can add things like reload_completed to the passes properties_set field hand have the flag setting handled by the pass manager as it was intended? Otherwise I of course welcome the change - since I suggested it I'd like somebody else to ack it as well of course. Thanks, Richard. > This patch allows the global variables lra_in_progress, reload_in_progress, > reload_completed, epilogue_completed and regstack_completed to be removed > from rtl.h and implemented as bits within the new crtl->rtl_pass_progress. > I've also taken the liberty of adding a new combine_completed bit at the > same time [to respond the Segher's comment it's easy to change this to > combine1_completed and combine2_completed if we ever perform multiple > combine passes (or multiple reload/regstack passes)]. At the same time, > I've also refactored bb_reorder_complete into the same new field; > interestingly bb_reorder_complete was already a bool in crtl. > > One very minor advantage of this implementation/refactoring is that the > predicate "can_create_pseudo_p ()" which is semantically defined to be > !reload_in_progress && !reload_completed, can now be performed very > efficiently as effectively the test (progress & 12) == 0, i.e. a single > test instruction on x86. > > For consistency, I've also moved cse_not_expected (the last remaining > global variable in rtl.h) into crtl, as its own bool field. > > The vast majority of this patch is then churn to handle these changes. > Thanks to macros, most code is unaffected, assuming it treats those > global variables as r-values, though some source files required/may > require tweaks as these "variables" are now defined in emit-rtl.h > instead of rtl.h. > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > and make -k check, both with and without --target_board=unix{-m32}, > with no new failures. Might this clean-up be acceptable in stage 1, > given the possible temporary disruption transitioning some backends? > I'll start checking various backends myself with cross-compilers, but if > Jeff Law could spin this patch on his build farm, that would help > identify targets that need attention. > > 2022-07-10 Roger Sayle > > gcc/ChangeLog > * bb-reorder.cc (reorder_basic_blocks): bb_reorder_complete is > now a bit in crtl->rtl_pass_progress. > * cfgrtl.cc (rtl_split_edge): Likewise. > (fixup_partitions): Likewise. > (verify_hot_cold_block_grouping): Likewise. > (cfg_layout_initialize): Likewise. > * combine.cc (rest_of_handle_combine): Set combine_completed > bit in crtl->rtl_pass_progress. > * cse.cc (rest_of_handle_cse): cse_not_expected is now a field > in crtl. > (rest_of_handle_cse2): Likewise. > (rest_of_handle_cse_after_global_opts): Likewise. > * df-problems.cc: Include emit-rtl.h to access RTL pass progress > variables. > > * emit-rtl.h (PROGRESS_reload_completed): New bit masks. > (rtl_data::rtl_pass_progress): New integer field to track progress. > (rtl_data::bb_reorder_complete): Delete, no part of > rtl_pass_progress. > (rtl_data::cse_not_expected): New bool field, previously a global > variable. > (crtl_pass_progress): New convience macro. > (combine_completed): New macro. > (lra_in_progress): New macro replacing global variable. > (reload_in_progress): Likewise. > (reload_completed): Likewise. > (bb_reorder_complete): New macro replacing bool field in crtl. > (epilogue_completed): New macro replacing global variable. > (regstack_completed): Likewise. > (can_create_pseudo_p): Move from rtl.h and update definition. > > * explow.cc (memory_address_addr_space): cse_not_expected is now > a field in crtl. > (use_anchored_address): Likewise. > * final.c (rest_of_clean_state): Reset crtl->rtl_pass_progress > to zero. > * function.cc (prepare_function_start): cse_not_expected is now > a field in crtl. > (thread_prologue_and_epilogue_insns): epilogue_completed is now > a bit in crtl->rtl_pass_progress. > * ifcvt.cc (noce_try_cmove_arith): cse_not_expected is now a > field in crtl. > * lra-eliminations.cc (init_elim_table): lra_in_progress is now > a bit in crtl->rtl_pass_progress. > * lra.cc (lra_in_progress): Delete global variable. > (lra): lra_in_progress and reload_completed are now bits in > crtl->rtl_pass_progress. > * modulo-sched.cc (sms_schedule): reload_completed is now a bit > in crtl->rtl_pass_progress. > * passes.cc (skip_pass): reload_completed and epilogue_completed > are now bits in crtl->rtl_pass_progress. > * recog.cc (reload_completed): Delete global variable. > (epilogue_completed): Likewise. > * reg-stack.cc (regstack_completed): Likewise. > (rest_of_handle_stack_regs): regstack_completed is now a bit in > crtl->rtl_pass_progress. > * regstat.cc: Include memmodel.h and emit-rtl.h to access RTL > pass progress variables. > * reload1.cc (reload_in_progress): Delete global variable. > (reload): reload_in_progress and reload_completed are now bits > in crtl->rtl_pass_progress. > * rtl.h (reload_completed): Delete global variable prototype. > (epilogue_completed): Likewise. > (reload_in_progress): Likewise. > (lra_in_progress): Likewise. > (can_create_pseudo_p): Delete, moved to emit-rtl.h. > (regstack_completed): Delete global variable prototype. > (cse_not_expected): Likewise. > * sched-ebb.cc: Include memmodel.h and emit-rtl.h to access RTL > pass progress variables. > > * config/i386/x86-tune-sched-atom.cc: Include memmodel.h and > emit-rtl.h to access RTL pass progress variables. > * config/i386/x86-tune-sched.cc: Likewise. > > > Thanks in advance, > Roger > -- > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstra