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 <ro...@nextmovesoftware.com>
>
> 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 <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstra
