| Message ID | ZVZaRu6BvaxQVqJ4@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Rework ldp/stp patterns, add new ldp/stp pass | expand |
Alex Coplan <alex.coplan@arm.com> writes: > This adds some helpers to access-utils.h for removing accesses from an > access_array. This is needed by the upcoming aarch64 load/store pair > fusion pass. > > Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk? > > gcc/ChangeLog: > > * rtl-ssa/access-utils.h (filter_accesses): New. > (remove_regno_access): New. > (check_remove_regno_access): New. > --- > gcc/rtl-ssa/access-utils.h | 42 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/gcc/rtl-ssa/access-utils.h b/gcc/rtl-ssa/access-utils.h > index f078625babf..31259d742d9 100644 > --- a/gcc/rtl-ssa/access-utils.h > +++ b/gcc/rtl-ssa/access-utils.h > @@ -78,6 +78,48 @@ drop_memory_access (T accesses) > return T (arr.begin (), accesses.size () - 1); > } > > +// Filter ACCESSES to return an access_array of only those accesses that > +// satisfy PREDICATE. Alocate the new array above WATERMARK. > +template<typename T, typename FilterPredicate> > +inline T > +filter_accesses (obstack_watermark &watermark, > + T accesses, > + FilterPredicate predicate) > +{ > + access_array_builder builder (watermark); > + builder.reserve (accesses.size ()); > + auto it = accesses.begin (); > + auto end = accesses.end (); > + for (; it != end; it++) > + if (predicate (*it)) > + builder.quick_push (*it); It looks like the last five lines could be simplified to: for (access_info *access : accesses) if (!predicate (access)) builder.quick_push (access); > + return T (builder.finish ()); > +} > + > +// Given an array of ACCESSES, remove any access with regno REGNO. > +// Allocate the new access array above WM. > +template<typename T> > +inline T > +remove_regno_access (obstack_watermark &watermark, > + T accesses, unsigned int regno) > +{ > + using Access = decltype (accesses[0]); > + auto pred = [regno](Access a) { return a->regno () != regno; }; > + return filter_accesses (watermark, accesses, pred); > +} > + > +// As above, but additionally check that we actually did remove an access. > +template<typename T> > +inline T > +check_remove_regno_access (obstack_watermark &watermark, > + T accesses, unsigned regno) > +{ > + auto orig_size = accesses.size (); > + auto result = remove_regno_access (watermark, accesses, regno); > + gcc_assert (result.size () < orig_size); > + return result; > +} > + Could you also use the helper to replace: access_array_builder builder (watermark); builder.reserve (accesses.size ()); for (access_info *access2 : accesses) if (!access2->only_occurs_in_notes ()) builder.quick_push (access2); return builder.finish (); in remove_note_accesses_base? OK with those changes, thanks. Richard > // If sorted array ACCESSES includes a reference to REGNO, return the > // access, otherwise return null. > template<typename T>
Richard Sandiford <richard.sandiford@arm.com> writes: > Alex Coplan <alex.coplan@arm.com> writes: >> This adds some helpers to access-utils.h for removing accesses from an >> access_array. This is needed by the upcoming aarch64 load/store pair >> fusion pass. >> >> Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk? >> >> gcc/ChangeLog: >> >> * rtl-ssa/access-utils.h (filter_accesses): New. >> (remove_regno_access): New. >> (check_remove_regno_access): New. >> --- >> gcc/rtl-ssa/access-utils.h | 42 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 42 insertions(+) >> >> diff --git a/gcc/rtl-ssa/access-utils.h b/gcc/rtl-ssa/access-utils.h >> index f078625babf..31259d742d9 100644 >> --- a/gcc/rtl-ssa/access-utils.h >> +++ b/gcc/rtl-ssa/access-utils.h >> @@ -78,6 +78,48 @@ drop_memory_access (T accesses) >> return T (arr.begin (), accesses.size () - 1); >> } >> >> +// Filter ACCESSES to return an access_array of only those accesses that >> +// satisfy PREDICATE. Alocate the new array above WATERMARK. >> +template<typename T, typename FilterPredicate> >> +inline T >> +filter_accesses (obstack_watermark &watermark, >> + T accesses, >> + FilterPredicate predicate) >> +{ >> + access_array_builder builder (watermark); >> + builder.reserve (accesses.size ()); >> + auto it = accesses.begin (); >> + auto end = accesses.end (); >> + for (; it != end; it++) >> + if (predicate (*it)) >> + builder.quick_push (*it); > > It looks like the last five lines could be simplified to: > > for (access_info *access : accesses) > if (!predicate (access)) > builder.quick_push (access); Oops, I meant: if (predicate (access)) of course :)
On 21/11/2023 16:49, Richard Sandiford wrote: > Richard Sandiford <richard.sandiford@arm.com> writes: > > Alex Coplan <alex.coplan@arm.com> writes: > >> This adds some helpers to access-utils.h for removing accesses from an > >> access_array. This is needed by the upcoming aarch64 load/store pair > >> fusion pass. > >> > >> Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk? > >> > >> gcc/ChangeLog: > >> > >> * rtl-ssa/access-utils.h (filter_accesses): New. > >> (remove_regno_access): New. > >> (check_remove_regno_access): New. > >> --- > >> gcc/rtl-ssa/access-utils.h | 42 ++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 42 insertions(+) > >> > >> diff --git a/gcc/rtl-ssa/access-utils.h b/gcc/rtl-ssa/access-utils.h > >> index f078625babf..31259d742d9 100644 > >> --- a/gcc/rtl-ssa/access-utils.h > >> +++ b/gcc/rtl-ssa/access-utils.h > >> @@ -78,6 +78,48 @@ drop_memory_access (T accesses) > >> return T (arr.begin (), accesses.size () - 1); > >> } > >> > >> +// Filter ACCESSES to return an access_array of only those accesses that > >> +// satisfy PREDICATE. Alocate the new array above WATERMARK. > >> +template<typename T, typename FilterPredicate> > >> +inline T > >> +filter_accesses (obstack_watermark &watermark, > >> + T accesses, > >> + FilterPredicate predicate) > >> +{ > >> + access_array_builder builder (watermark); > >> + builder.reserve (accesses.size ()); > >> + auto it = accesses.begin (); > >> + auto end = accesses.end (); > >> + for (; it != end; it++) > >> + if (predicate (*it)) > >> + builder.quick_push (*it); > > > > It looks like the last five lines could be simplified to: > > > > for (access_info *access : accesses) > > if (!predicate (access)) > > builder.quick_push (access); So I implemented these changes, but I found that I had to use auto instead of access_info * for the type of access in the for loop. That allows callers to use the most specific/derived type for the parameter in the predicate (e.g. use `use_info *` for an array of uses). Is it OK with that change? I've attached a revised patch. Bootstrapped/regtested on aarch64-linux-gnu. Thanks, Alex > > Oops, I meant: > > if (predicate (access)) > > of course :) diff --git a/gcc/rtl-ssa/access-utils.h b/gcc/rtl-ssa/access-utils.h index f078625babf..9a62addfd2a 100644 --- a/gcc/rtl-ssa/access-utils.h +++ b/gcc/rtl-ssa/access-utils.h @@ -78,6 +78,46 @@ drop_memory_access (T accesses) return T (arr.begin (), accesses.size () - 1); } +// Filter ACCESSES to return an access_array of only those accesses that +// satisfy PREDICATE. Alocate the new array above WATERMARK. +template<typename T, typename FilterPredicate> +inline T +filter_accesses (obstack_watermark &watermark, + T accesses, + FilterPredicate predicate) +{ + access_array_builder builder (watermark); + builder.reserve (accesses.size ()); + for (auto access : accesses) + if (predicate (access)) + builder.quick_push (access); + return T (builder.finish ()); +} + +// Given an array of ACCESSES, remove any access with regno REGNO. +// Allocate the new access array above WM. +template<typename T> +inline T +remove_regno_access (obstack_watermark &watermark, + T accesses, unsigned int regno) +{ + using Access = decltype (accesses[0]); + auto pred = [regno](Access a) { return a->regno () != regno; }; + return filter_accesses (watermark, accesses, pred); +} + +// As above, but additionally check that we actually did remove an access. +template<typename T> +inline T +check_remove_regno_access (obstack_watermark &watermark, + T accesses, unsigned regno) +{ + auto orig_size = accesses.size (); + auto result = remove_regno_access (watermark, accesses, regno); + gcc_assert (result.size () < orig_size); + return result; +} + // If sorted array ACCESSES includes a reference to REGNO, return the // access, otherwise return null. template<typename T> diff --git a/gcc/rtl-ssa/accesses.cc b/gcc/rtl-ssa/accesses.cc index 76d70fd8bd3..9ec0e6be071 100644 --- a/gcc/rtl-ssa/accesses.cc +++ b/gcc/rtl-ssa/accesses.cc @@ -1597,16 +1597,14 @@ access_array rtl_ssa::remove_note_accesses_base (obstack_watermark &watermark, access_array accesses) { + auto predicate = [](access_info *a) { + return !a->only_occurs_in_notes (); + }; + for (access_info *access : accesses) if (access->only_occurs_in_notes ()) - { - access_array_builder builder (watermark); - builder.reserve (accesses.size ()); - for (access_info *access2 : accesses) - if (!access2->only_occurs_in_notes ()) - builder.quick_push (access2); - return builder.finish (); - } + return filter_accesses (watermark, accesses, predicate); + return accesses; }
Alex Coplan <alex.coplan@arm.com> writes: > On 21/11/2023 16:49, Richard Sandiford wrote: >> Richard Sandiford <richard.sandiford@arm.com> writes: >> > Alex Coplan <alex.coplan@arm.com> writes: >> >> This adds some helpers to access-utils.h for removing accesses from an >> >> access_array. This is needed by the upcoming aarch64 load/store pair >> >> fusion pass. >> >> >> >> Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk? >> >> >> >> gcc/ChangeLog: >> >> >> >> * rtl-ssa/access-utils.h (filter_accesses): New. >> >> (remove_regno_access): New. >> >> (check_remove_regno_access): New. >> >> --- >> >> gcc/rtl-ssa/access-utils.h | 42 ++++++++++++++++++++++++++++++++++++++ >> >> 1 file changed, 42 insertions(+) >> >> >> >> diff --git a/gcc/rtl-ssa/access-utils.h b/gcc/rtl-ssa/access-utils.h >> >> index f078625babf..31259d742d9 100644 >> >> --- a/gcc/rtl-ssa/access-utils.h >> >> +++ b/gcc/rtl-ssa/access-utils.h >> >> @@ -78,6 +78,48 @@ drop_memory_access (T accesses) >> >> return T (arr.begin (), accesses.size () - 1); >> >> } >> >> >> >> +// Filter ACCESSES to return an access_array of only those accesses that >> >> +// satisfy PREDICATE. Alocate the new array above WATERMARK. >> >> +template<typename T, typename FilterPredicate> >> >> +inline T >> >> +filter_accesses (obstack_watermark &watermark, >> >> + T accesses, >> >> + FilterPredicate predicate) >> >> +{ >> >> + access_array_builder builder (watermark); >> >> + builder.reserve (accesses.size ()); >> >> + auto it = accesses.begin (); >> >> + auto end = accesses.end (); >> >> + for (; it != end; it++) >> >> + if (predicate (*it)) >> >> + builder.quick_push (*it); >> > >> > It looks like the last five lines could be simplified to: >> > >> > for (access_info *access : accesses) >> > if (!predicate (access)) >> > builder.quick_push (access); > > So I implemented these changes, but I found that I had to use auto > instead of access_info * for the type of access in the for loop. > > That allows callers to use the most specific/derived type for the > parameter in the predicate (e.g. use `use_info *` for an array of uses). > > Is it OK with that change? I've attached a revised patch. > Bootstrapped/regtested on aarch64-linux-gnu. OK, thanks. Richard > Thanks, > Alex > >> >> Oops, I meant: >> >> if (predicate (access)) >> >> of course :) > > diff --git a/gcc/rtl-ssa/access-utils.h b/gcc/rtl-ssa/access-utils.h > index f078625babf..9a62addfd2a 100644 > --- a/gcc/rtl-ssa/access-utils.h > +++ b/gcc/rtl-ssa/access-utils.h > @@ -78,6 +78,46 @@ drop_memory_access (T accesses) > return T (arr.begin (), accesses.size () - 1); > } > > +// Filter ACCESSES to return an access_array of only those accesses that > +// satisfy PREDICATE. Alocate the new array above WATERMARK. > +template<typename T, typename FilterPredicate> > +inline T > +filter_accesses (obstack_watermark &watermark, > + T accesses, > + FilterPredicate predicate) > +{ > + access_array_builder builder (watermark); > + builder.reserve (accesses.size ()); > + for (auto access : accesses) > + if (predicate (access)) > + builder.quick_push (access); > + return T (builder.finish ()); > +} > + > +// Given an array of ACCESSES, remove any access with regno REGNO. > +// Allocate the new access array above WM. > +template<typename T> > +inline T > +remove_regno_access (obstack_watermark &watermark, > + T accesses, unsigned int regno) > +{ > + using Access = decltype (accesses[0]); > + auto pred = [regno](Access a) { return a->regno () != regno; }; > + return filter_accesses (watermark, accesses, pred); > +} > + > +// As above, but additionally check that we actually did remove an access. > +template<typename T> > +inline T > +check_remove_regno_access (obstack_watermark &watermark, > + T accesses, unsigned regno) > +{ > + auto orig_size = accesses.size (); > + auto result = remove_regno_access (watermark, accesses, regno); > + gcc_assert (result.size () < orig_size); > + return result; > +} > + > // If sorted array ACCESSES includes a reference to REGNO, return the > // access, otherwise return null. > template<typename T> > diff --git a/gcc/rtl-ssa/accesses.cc b/gcc/rtl-ssa/accesses.cc > index 76d70fd8bd3..9ec0e6be071 100644 > --- a/gcc/rtl-ssa/accesses.cc > +++ b/gcc/rtl-ssa/accesses.cc > @@ -1597,16 +1597,14 @@ access_array > rtl_ssa::remove_note_accesses_base (obstack_watermark &watermark, > access_array accesses) > { > + auto predicate = [](access_info *a) { > + return !a->only_occurs_in_notes (); > + }; > + > for (access_info *access : accesses) > if (access->only_occurs_in_notes ()) > - { > - access_array_builder builder (watermark); > - builder.reserve (accesses.size ()); > - for (access_info *access2 : accesses) > - if (!access2->only_occurs_in_notes ()) > - builder.quick_push (access2); > - return builder.finish (); > - } > + return filter_accesses (watermark, accesses, predicate); > + > return accesses; > } >
diff --git a/gcc/rtl-ssa/access-utils.h b/gcc/rtl-ssa/access-utils.h index f078625babf..31259d742d9 100644 --- a/gcc/rtl-ssa/access-utils.h +++ b/gcc/rtl-ssa/access-utils.h @@ -78,6 +78,48 @@ drop_memory_access (T accesses) return T (arr.begin (), accesses.size () - 1); } +// Filter ACCESSES to return an access_array of only those accesses that +// satisfy PREDICATE. Alocate the new array above WATERMARK. +template<typename T, typename FilterPredicate> +inline T +filter_accesses (obstack_watermark &watermark, + T accesses, + FilterPredicate predicate) +{ + access_array_builder builder (watermark); + builder.reserve (accesses.size ()); + auto it = accesses.begin (); + auto end = accesses.end (); + for (; it != end; it++) + if (predicate (*it)) + builder.quick_push (*it); + return T (builder.finish ()); +} + +// Given an array of ACCESSES, remove any access with regno REGNO. +// Allocate the new access array above WM. +template<typename T> +inline T +remove_regno_access (obstack_watermark &watermark, + T accesses, unsigned int regno) +{ + using Access = decltype (accesses[0]); + auto pred = [regno](Access a) { return a->regno () != regno; }; + return filter_accesses (watermark, accesses, pred); +} + +// As above, but additionally check that we actually did remove an access. +template<typename T> +inline T +check_remove_regno_access (obstack_watermark &watermark, + T accesses, unsigned regno) +{ + auto orig_size = accesses.size (); + auto result = remove_regno_access (watermark, accesses, regno); + gcc_assert (result.size () < orig_size); + return result; +} + // If sorted array ACCESSES includes a reference to REGNO, return the // access, otherwise return null. template<typename T>