* [PATCH 02/11] rtl-ssa: Add some helpers for removing accesses
@ 2023-11-16 18:07 Alex Coplan
2023-11-21 11:58 ` Richard Sandiford
0 siblings, 1 reply; 5+ messages in thread
From: Alex Coplan @ 2023-11-16 18:07 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford, Kyrylo Tkachov
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
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(+)
[-- Attachment #2: 0002-rtl-ssa-Add-some-helpers-for-removing-accesses.patch --]
[-- Type: text/x-patch, Size: 1755 bytes --]
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 02/11] rtl-ssa: Add some helpers for removing accesses
2023-11-16 18:07 [PATCH 02/11] rtl-ssa: Add some helpers for removing accesses Alex Coplan
@ 2023-11-21 11:58 ` Richard Sandiford
2023-11-21 16:49 ` Richard Sandiford
0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2023-11-21 11:58 UTC (permalink / raw)
To: Alex Coplan; +Cc: gcc-patches, Kyrylo Tkachov
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 02/11] rtl-ssa: Add some helpers for removing accesses
2023-11-21 11:58 ` Richard Sandiford
@ 2023-11-21 16:49 ` Richard Sandiford
2023-11-23 17:06 ` Alex Coplan
0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2023-11-21 16:49 UTC (permalink / raw)
To: Alex Coplan; +Cc: gcc-patches, Kyrylo Tkachov
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 :)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 02/11] rtl-ssa: Add some helpers for removing accesses
2023-11-21 16:49 ` Richard Sandiford
@ 2023-11-23 17:06 ` Alex Coplan
2023-11-23 17:09 ` Richard Sandiford
0 siblings, 1 reply; 5+ messages in thread
From: Alex Coplan @ 2023-11-23 17:06 UTC (permalink / raw)
To: gcc-patches, Kyrylo Tkachov, richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]
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 :)
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2513 bytes --]
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;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 02/11] rtl-ssa: Add some helpers for removing accesses
2023-11-23 17:06 ` Alex Coplan
@ 2023-11-23 17:09 ` Richard Sandiford
0 siblings, 0 replies; 5+ messages in thread
From: Richard Sandiford @ 2023-11-23 17:09 UTC (permalink / raw)
To: Alex Coplan; +Cc: gcc-patches, Kyrylo Tkachov
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;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-23 17:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-16 18:07 [PATCH 02/11] rtl-ssa: Add some helpers for removing accesses Alex Coplan
2023-11-21 11:58 ` Richard Sandiford
2023-11-21 16:49 ` Richard Sandiford
2023-11-23 17:06 ` Alex Coplan
2023-11-23 17:09 ` Richard Sandiford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).