* [RFA] gcc: fix PR rtl-optimization/107482
@ 2022-11-07 9:46 Max Filippov
2022-11-07 20:52 ` Vladimir Makarov
0 siblings, 1 reply; 3+ messages in thread
From: Max Filippov @ 2022-11-07 9:46 UTC (permalink / raw)
To: gcc-patches; +Cc: Vladimir Makarov, Max Filippov
gcc/
* ira-color.cc (update_costs_from_allocno): Check that allocno
is in the consideration_allocno_bitmap before dereferencing
ALLOCNO_COLOR_DATA (allocno).
---
This fixes the invalid memory access, but I'm not sure if that's
sufficient and there's no remaining higher level logical issue.
Regtested for target=xtensa-linux-uclibc, no new regressions.
gcc/ira-color.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gcc/ira-color.cc b/gcc/ira-color.cc
index 4a1a325e8e31..4527eab39bb7 100644
--- a/gcc/ira-color.cc
+++ b/gcc/ira-color.cc
@@ -1434,6 +1434,8 @@ update_costs_from_allocno (ira_allocno_t allocno, int hard_regno,
if (another_allocno == from
|| (ALLOCNO_COLOR_DATA (another_allocno) != NULL
+ && bitmap_bit_p (consideration_allocno_bitmap,
+ ALLOCNO_NUM (allocno))
&& (ALLOCNO_COLOR_DATA (allocno)->first_thread_allocno
!= ALLOCNO_COLOR_DATA (another_allocno)->first_thread_allocno)))
continue;
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFA] gcc: fix PR rtl-optimization/107482
2022-11-07 9:46 [RFA] gcc: fix PR rtl-optimization/107482 Max Filippov
@ 2022-11-07 20:52 ` Vladimir Makarov
2022-11-08 10:05 ` Max Filippov
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Makarov @ 2022-11-07 20:52 UTC (permalink / raw)
To: Max Filippov, gcc-patches
On 2022-11-07 04:46, Max Filippov wrote:
> gcc/
> * ira-color.cc (update_costs_from_allocno): Check that allocno
> is in the consideration_allocno_bitmap before dereferencing
> ALLOCNO_COLOR_DATA (allocno).
> ---
> This fixes the invalid memory access, but I'm not sure if that's
> sufficient and there's no remaining higher level logical issue.
Thank you for reporting and working on this issue.
I believe your approach is sufficient. Although the patch could be
improved by three ways:
The simplest one is to move consideration allocno check out of loop by
using the following patch
diff --git a/gcc/ira-color.cc b/gcc/ira-color.cc
index 4a1a325e8e3..a8e52b6b265 100644
--- a/gcc/ira-color.cc
+++ b/gcc/ira-color.cc
@@ -1413,7 +1413,9 @@ update_costs_from_allocno (ira_allocno_t allocno,
int hard_regno,
ira_copy_t cp, next_cp;
rclass = REGNO_REG_CLASS (hard_regno);
- do
+ if (!bitmap_bit_p (consideration_allocno_bitmap, ALLOCNO_NUM (allocno)))
+ return;
+ do
{
mode = ALLOCNO_MODE (allocno);
ira_init_register_move_cost_if_necessary (mode);
or by even better patch:
diff --git a/gcc/ira-color.cc b/gcc/ira-color.cc
index 4a1a325e8e3..ffe73b61c45 100644
--- a/gcc/ira-color.cc
+++ b/gcc/ira-color.cc
@@ -2209,8 +2209,8 @@ assign_hard_reg (ira_allocno_t a, bool retry_p)
restore_costs_from_copies (a);
ALLOCNO_HARD_REGNO (a) = best_hard_regno;
ALLOCNO_ASSIGNED_P (a) = true;
- if (best_hard_regno >= 0)
- update_costs_from_copies (a, true, ! retry_p);
+ if (best_hard_regno >= 0 && !retry_p)
+ update_costs_from_copies (a, true, true);
ira_assert (ALLOCNO_CLASS (a) == aclass);
/* We don't need updated costs anymore. */
ira_free_allocno_updated_costs (a);
Probably the best way would be to allocate and set up data for new
allocnos of pseudos created on the borders of the allocation regions.
But it is too complicated and I am not sure it will give some visible
performance improvement.
So I'd prefer the second patch with change in assign_hard_reg.
Please, check that my proposed patch works and commit it in the case of
success.
Thank you.
> Regtested for target=xtensa-linux-uclibc, no new regressions.
>
> gcc/ira-color.cc | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/gcc/ira-color.cc b/gcc/ira-color.cc
> index 4a1a325e8e31..4527eab39bb7 100644
> --- a/gcc/ira-color.cc
> +++ b/gcc/ira-color.cc
> @@ -1434,6 +1434,8 @@ update_costs_from_allocno (ira_allocno_t allocno, int hard_regno,
>
> if (another_allocno == from
> || (ALLOCNO_COLOR_DATA (another_allocno) != NULL
> + && bitmap_bit_p (consideration_allocno_bitmap,
> + ALLOCNO_NUM (allocno))
> && (ALLOCNO_COLOR_DATA (allocno)->first_thread_allocno
> != ALLOCNO_COLOR_DATA (another_allocno)->first_thread_allocno)))
> continue;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFA] gcc: fix PR rtl-optimization/107482
2022-11-07 20:52 ` Vladimir Makarov
@ 2022-11-08 10:05 ` Max Filippov
0 siblings, 0 replies; 3+ messages in thread
From: Max Filippov @ 2022-11-08 10:05 UTC (permalink / raw)
To: Vladimir Makarov; +Cc: gcc-patches
On Mon, Nov 7, 2022 at 12:52 PM Vladimir Makarov <vmakarov@redhat.com> wrote:
> even better patch:
>
> diff --git a/gcc/ira-color.cc b/gcc/ira-color.cc
> index 4a1a325e8e3..ffe73b61c45 100644
> --- a/gcc/ira-color.cc
> +++ b/gcc/ira-color.cc
> @@ -2209,8 +2209,8 @@ assign_hard_reg (ira_allocno_t a, bool retry_p)
> restore_costs_from_copies (a);
> ALLOCNO_HARD_REGNO (a) = best_hard_regno;
> ALLOCNO_ASSIGNED_P (a) = true;
> - if (best_hard_regno >= 0)
> - update_costs_from_copies (a, true, ! retry_p);
> + if (best_hard_regno >= 0 && !retry_p)
> + update_costs_from_copies (a, true, true);
> ira_assert (ALLOCNO_CLASS (a) == aclass);
> /* We don't need updated costs anymore. */
> ira_free_allocno_updated_costs (a);
>
...
> Please, check that my proposed patch works and commit it in the case of
> success.
Thank you for taking a look and suggesting a better fix.
I've tested your version for target=xtensa-linux-uclibc, it fixes
the issue without new regressions. I've committed the fix to the
master branch and will backport it to gcc-10, -11 and -12 in a few
days.
--
Thanks.
-- Max
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-08 10:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-07 9:46 [RFA] gcc: fix PR rtl-optimization/107482 Max Filippov
2022-11-07 20:52 ` Vladimir Makarov
2022-11-08 10:05 ` Max Filippov
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).