Hi Richard, I had a quick look through the patch and noticed a couple of minor typos. Otherwise looks like a nice cleanup!
On 20/06/2024 14:34, Richard Sandiford wrote: > rtl-ssa has routines for scanning forwards or backwards for something > under the control of an exclusion set. These searches are currently > used for two main things: > > - to work out where an instruction can be moved within its EBB > - to work out whether recog can add a new hard register clobber > > The exclusion set was originally a callback function that returned > true for insns that should be ignored. However, for the late-combine > work, I'd also like to be able to skip an entire definition, along > with all its uses. > > This patch prepares for that by turning the exclusion set into an > object that provides predicate member functions. Currently the > only two member functions are: > > - should_ignore_insn: what the old callback did > - should_ignore_def: the new functionality > > but more could be added later. > > Doing this also makes it easy to remove some assymmetry that I think s/assymmetry/asymmetry/ > in hindsight was a mistake: in forward scans, ignoring an insn meant > ignoring all definitions in that insn (ok) and all uses of those > definitions (non-obvious). The new interface makes it possible > to select the required behaviour, with that behaviour being applied > consistently in both directions. > > Now that the exclusion set is a dedicated object, rather than > just a "random" function, I think it makes sense to remove the > _ignoring suffix from the function names. The suffix was originally > there to describe the callback, and in particular to emphasise that > a true return meant "ignore" rather than "heed". > > gcc/ > * rtl-ssa.h: Include predicates.h. > * rtl-ssa/predicates.h: New file. > * rtl-ssa/access-utils.h (prev_call_clobbers_ignoring): Rename to... > (prev_call_clobbers): ...this and treat the ignore parameter as an > object with the same interface as ignore_nothing. > (next_call_clobbers_ignoring): Rename to... > (next_call_clobbers): ...this and treat the ignore parameter as an > object with the same interface as ignore_nothing. > (first_nondebug_insn_use_ignoring): Rename to... > (first_nondebug_insn_use): ...this and treat the ignore parameter as > an object with the same interface as ignore_nothing. > (last_nondebug_insn_use_ignoring): Rename to... > (last_nondebug_insn_use): ...this and treat the ignore parameter as > an object with the same interface as ignore_nothing. > (last_access_ignoring): Rename to... > (last_access): ...this and treat the ignore parameter as an object > with the same interface as ignore_nothing. Conditionally skip > definitions. > (prev_access_ignoring): Rename to... > (prev_access): ...this and treat the ignore parameter as an object > with the same interface as ignore_nothing. > (first_def_ignoring): Replace with... > (first_access): ...this new function. > (next_access_ignoring): Rename to... > (next_access): ...this and treat the ignore parameter as an object > with the same interface as ignore_nothing. Conditionally skip > definitions. > * rtl-ssa/change-utils.h (insn_is_changing): Delete. > (restrict_movement_ignoring): Rename to... > (restrict_movement): ...this and treat the ignore parameter as an > object with the same interface as ignore_nothing. > (recog_ignoring): Rename to... > (recog): ...this and treat the ignore parameter as an object with > the same interface as ignore_nothing. > * rtl-ssa/changes.h (insn_is_changing_closure): Delete. > * rtl-ssa/functions.h (function_info::add_regno_clobber): Treat > the ignore parameter as an object with the same interface as > ignore_nothing. > * rtl-ssa/insn-utils.h (insn_is): Delete. > * rtl-ssa/insns.h (insn_is_closure): Delete. > * rtl-ssa/member-fns.inl > (insn_is_changing_closure::insn_is_changing_closure): Delete. > (insn_is_changing_closure::operator()): Likewise. > (function_info::add_regno_clobber): Treat the ignore parameter > as an object with the same interface as ignore_nothing. > (ignore_changing_insns::ignore_changing_insns): New function. > (ignore_changing_insns::should_ignore_insn): Likewise. > * rtl-ssa/movement.h (restrict_movement_for_dead_range): Treat > the ignore parameter as an object with the same interface as > ignore_nothing. > (restrict_movement_for_defs_ignoring): Rename to... > (restrict_movement_for_defs): ...this and treat the ignore parameter > as an object with the same interface as ignore_nothing. > (restrict_movement_for_uses_ignoring): Rename to... > (restrict_movement_for_uses): ...this and treat the ignore parameter > as an object with the same interface as ignore_nothing. Conditionally > skip definitions. > * doc/rtl.texi: Update for above name changes. Use > ignore_changing_insns instead of insn_is_changing. > * config/aarch64/aarch64-cc-fusion.cc (cc_fusion::parallelize_insns): > Likewise. > * pair-fusion.cc (no_ignore): Delete. > (latest_hazard_before, first_hazard_after): Update for above name > changes. Use ignore_nothing instead of no_ignore. > (pair_fusion_bb_info::fuse_pair): Update for above name changes. > Use ignore_changing_insns instead of insn_is_changing. > (pair_fusion::try_promote_writeback): Likewise. > --- > gcc/config/aarch64/aarch64-cc-fusion.cc | 4 +- > gcc/doc/rtl.texi | 14 +-- > gcc/pair-fusion.cc | 34 +++--- > gcc/rtl-ssa.h | 1 + > gcc/rtl-ssa/access-utils.h | 145 +++++++++++++----------- > gcc/rtl-ssa/change-utils.h | 67 +++++------ > gcc/rtl-ssa/changes.h | 13 --- > gcc/rtl-ssa/functions.h | 16 ++- > gcc/rtl-ssa/insn-utils.h | 8 -- > gcc/rtl-ssa/insns.h | 12 -- > gcc/rtl-ssa/member-fns.inl | 35 +++--- > gcc/rtl-ssa/movement.h | 118 +++++++++---------- > gcc/rtl-ssa/predicates.h | 58 ++++++++++ > 13 files changed, 275 insertions(+), 250 deletions(-) > create mode 100644 gcc/rtl-ssa/predicates.h > <snip> > diff --git a/gcc/rtl-ssa/functions.h b/gcc/rtl-ssa/functions.h > index f5aca643beb..479c6992e97 100644 > --- a/gcc/rtl-ssa/functions.h > +++ b/gcc/rtl-ssa/functions.h > @@ -165,16 +165,22 @@ public: > > // If CHANGE doesn't already clobber REGNO, try to add such a clobber, > // limiting the movement range in order to make the clobber valid. > - // When determining whether REGNO is live, ignore accesses made by an > - // instruction I if IGNORE (I) is true. The caller then assumes the > - // responsibility of ensuring that CHANGE and I are placed in a valid > order. > + // Use IGNORE to guide this process, where IGNORE is an object that > + // provides the same interface as ignore_nothing. > + // > + // That is, when determining whether REGNO is live, ignore accesses made > + // by an instruction I if IGNORE says that I should be ignored. The caller > + // then assumes the responsibility of ensuring that CHANGE and I are placed > + // in a valid order. Similarly, ignore live ranges associated/ with a Stray '/' after associated? Thanks, Alex > + // definition of REGNO if IGNORE says that that definition should be > + // ignored. > // > // Return true on success. Leave CHANGE unmodified when returning false. > // > // WATERMARK is a watermark returned by new_change_attempt (). > - template<typename IgnorePredicate> > + template<typename IgnorePredicates> > bool add_regno_clobber (obstack_watermark &watermark, insn_change &change, > - unsigned int regno, IgnorePredicate ignore); > + unsigned int regno, IgnorePredicates ignore); > > // Return true if change_insns will be able to perform the changes > // described by CHANGES. <snip>
