Alex Coplan <alex.cop...@arm.com> writes: > Hi Richard, > > I had a quick look through the patch and noticed a couple of minor typos. > Otherwise looks like a nice cleanup!
Thanks for the review! I've fixed the typos in my local copy. Richard > 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>
