Re: [PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
Robin Dapp writes: >> Could you try the attached? > > The series with the patch is OK from a testsuite point of view. The > other problem appears later. OK, thanks for checking. I've pushed the patch as obvious. Richard
Re: [PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
> Could you try the attached? The series with the patch is OK from a testsuite point of view. The other problem appears later. Regards Robin
Re: [PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
On 1/11/22 09:52, Richard Sandiford via Gcc-patches wrote: Robin Dapp writes: Hi Richard, this causes a bootstrap error on s390 (where IRA_HARD_REGNO_ADD_COST_MULTIPLIER is defined). rclass is used in the #define-guarded area. Gah, sorry about that. I guess you also wanted to move this to the new function ira_caller_save_cost? No, the IRA_HARD_REGNO_ADD_COST_MULTIPLIER heuristic is a separate thing. It's just that I had to remove the rclass variable to allow bootstrap on other targets. Could you try the attached? Hello. I noticed the same failure. Please push the patch. Thanks, Martin Thanks, Richard
Re: [PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
Hi Richard, > Could you try the attached? build and bootstrap look OK with it. Testsuite shows lots of fallout but the proper bisect isn't finished yet. The commit before your series is still fine - the problem could also be after it, though. Will report back later. Thanks Robin
Re: [PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
Robin Dapp writes:
> Hi Richard,
>
> this causes a bootstrap error on s390 (where
> IRA_HARD_REGNO_ADD_COST_MULTIPLIER is defined). rclass is used in the
> #define-guarded area.
Gah, sorry about that.
> I guess you also wanted to move this to the new function
> ira_caller_save_cost?
No, the IRA_HARD_REGNO_ADD_COST_MULTIPLIER heuristic is a separate thing.
It's just that I had to remove the rclass variable to allow bootstrap on
other targets.
Could you try the attached?
Thanks,
Richard
>From 74cca9d27da840dbb79aa9ed9edc6f529945d477 Mon Sep 17 00:00:00 2001
From: Richard Sandiford
Date: Tue, 11 Jan 2022 08:44:56 +
Subject: [PATCH] ira: Fix s390 build
My g:01f3e6a40e7202310abbeb41c345d325bd69554f broke the s390
build because the rclass variable was still needed by the
IRA_HARD_REGNO_ADD_COST_MULTIPLIER code.
gcc/
* ira-costs.c (ira_tune_allocno_costs): Fix missing rclass
definition in IRA_HARD_REGNO_ADD_COST_MULTIPLIER code.
---
gcc/ira-costs.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/gcc/ira-costs.c b/gcc/ira-costs.c
index cbb58d32be8..1e4cf5a35e4 100644
--- a/gcc/ira-costs.c
+++ b/gcc/ira-costs.c
@@ -2351,10 +2351,13 @@ ira_tune_allocno_costs (void)
if (ira_need_caller_save_p (a, regno))
cost += ira_caller_save_cost (a);
#ifdef IRA_HARD_REGNO_ADD_COST_MULTIPLIER
- cost += ((ira_memory_move_cost[mode][rclass][0]
- + ira_memory_move_cost[mode][rclass][1])
- * ALLOCNO_FREQ (a)
- * IRA_HARD_REGNO_ADD_COST_MULTIPLIER (regno) / 2);
+ {
+ auto rclass = REGNO_REG_CLASS (regno);
+ cost += ((ira_memory_move_cost[mode][rclass][0]
+ + ira_memory_move_cost[mode][rclass][1])
+ * ALLOCNO_FREQ (a)
+ * IRA_HARD_REGNO_ADD_COST_MULTIPLIER (regno) / 2);
+ }
#endif
if (INT_MAX - cost < reg_costs[j])
reg_costs[j] = INT_MAX;
--
2.25.1
Re: [PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
Hi Richard, this causes a bootstrap error on s390 (where IRA_HARD_REGNO_ADD_COST_MULTIPLIER is defined). rclass is used in the #define-guarded area. I guess you also wanted to move this to the new function ira_caller_save_cost? Regards Robin -- ../../gcc/ira-costs.c: In function ‘void ira_tune_allocno_costs()’: ../../gcc/ira-costs.c:2354:45: error: ‘rclass’ was not declared in this scope; did you mean ‘aclass’? 2354 |cost += ((ira_memory_move_cost[mode][rclass][0] | ^~ | aclass
Re: [PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
> From: Richard Sandiford via Gcc-patches > Date: Thu, 6 Jan 2022 15:48:01 +0100 > If an allocno A in an inner loop L spans a call, a parent allocno AP > can choose to handle a call-clobbered/caller-saved hard register R > in one of two ways: > > (1) save R before each call in L and restore R after each call > (2) spill R to memory throughout L > > (2) can be cheaper than (1) in some cases, particularly if L does > not reference A. > > Before the patch we always did (1). The patch adds support for > picking (2) instead, when it seems cheaper. It builds on the > earlier support for not propagating conflicts to parent allocnos. > > gcc/ > PR rtl-optimization/98782 > * ira-int.h (ira_caller_save_cost): New function. > (ira_caller_save_loop_spill_p): Likewise. > * ira-build.c (ira_propagate_hard_reg_costs): Test whether it is > cheaper to spill a call-clobbered register throughout a loop rather > than spill it around each individual call. If so, treat all > call-clobbered registers as conflicts and... > (propagate_allocno_info): ...do not propagate call information > from the child to the parent. > * ira-color.c (move_spill_restore): Update accordingly. > * ira-costs.c (ira_tune_allocno_costs): Use ira_caller_save_cost. I bisected a broken build for cris-elf to this patch. Details in https://gcc.gnu.org/PR103974 supposedly sufficient to find a quick resolution. (JFTR, as you're already CC:ed by your @gcc.gnu.org account.) Perhaps some of these patches are better postponed for stage 1? brgds, H-P
Re: [PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
On 2022-01-06 09:48, Richard Sandiford wrote: If an allocno A in an inner loop L spans a call, a parent allocno AP can choose to handle a call-clobbered/caller-saved hard register R in one of two ways: (1) save R before each call in L and restore R after each call (2) spill R to memory throughout L (2) can be cheaper than (1) in some cases, particularly if L does not reference A. Before the patch we always did (1). The patch adds support for picking (2) instead, when it seems cheaper. It builds on the earlier support for not propagating conflicts to parent allocnos. Another cost calculation improvement for calls would be taking into account that allocno can be saved and restored once for several subsequent calls (e.g. in one BB). gcc/ PR rtl-optimization/98782 * ira-int.h (ira_caller_save_cost): New function. (ira_caller_save_loop_spill_p): Likewise. * ira-build.c (ira_propagate_hard_reg_costs): Test whether it is cheaper to spill a call-clobbered register throughout a loop rather than spill it around each individual call. If so, treat all call-clobbered registers as conflicts and... (propagate_allocno_info): ...do not propagate call information from the child to the parent. * ira-color.c (move_spill_restore): Update accordingly. * ira-costs.c (ira_tune_allocno_costs): Use ira_caller_save_cost. gcc/testsuite/ * gcc.target/aarch64/reg-alloc-3.c: New test. OK for me. Thank you for the patch.
[PATCH 5/6] ira: Consider modelling caller-save allocations as loop spills
If an allocno A in an inner loop L spans a call, a parent allocno AP
can choose to handle a call-clobbered/caller-saved hard register R
in one of two ways:
(1) save R before each call in L and restore R after each call
(2) spill R to memory throughout L
(2) can be cheaper than (1) in some cases, particularly if L does
not reference A.
Before the patch we always did (1). The patch adds support for
picking (2) instead, when it seems cheaper. It builds on the
earlier support for not propagating conflicts to parent allocnos.
gcc/
PR rtl-optimization/98782
* ira-int.h (ira_caller_save_cost): New function.
(ira_caller_save_loop_spill_p): Likewise.
* ira-build.c (ira_propagate_hard_reg_costs): Test whether it is
cheaper to spill a call-clobbered register throughout a loop rather
than spill it around each individual call. If so, treat all
call-clobbered registers as conflicts and...
(propagate_allocno_info): ...do not propagate call information
from the child to the parent.
* ira-color.c (move_spill_restore): Update accordingly.
* ira-costs.c (ira_tune_allocno_costs): Use ira_caller_save_cost.
gcc/testsuite/
* gcc.target/aarch64/reg-alloc-3.c: New test.
---
gcc/ira-build.c | 23 ---
gcc/ira-color.c | 13 ++--
gcc/ira-costs.c | 7 +-
gcc/ira-int.h | 39 +++
.../gcc.target/aarch64/reg-alloc-3.c | 65 +++
5 files changed, 129 insertions(+), 18 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/reg-alloc-3.c
diff --git a/gcc/ira-build.c b/gcc/ira-build.c
index 875b4d8ed7c..ab3e87164e1 100644
--- a/gcc/ira-build.c
+++ b/gcc/ira-build.c
@@ -2000,6 +2000,8 @@ ira_propagate_hard_reg_costs (ira_allocno_t parent_a,
ira_allocno_t a,
int spill_cost)
{
HARD_REG_SET conflicts = ira_total_conflict_hard_regs (a);
+ if (ira_caller_save_loop_spill_p (parent_a, a, spill_cost))
+conflicts |= ira_need_caller_save_regs (a);
conflicts &= ~ira_total_conflict_hard_regs (parent_a);
auto costs = ALLOCNO_HARD_REG_COSTS (a);
@@ -2069,15 +2071,18 @@ propagate_allocno_info (void)
if (!ira_subloop_allocnos_can_differ_p (parent_a))
merge_hard_reg_conflicts (a, parent_a, true);
- ALLOCNO_CALL_FREQ (parent_a) += ALLOCNO_CALL_FREQ (a);
- ALLOCNO_CALLS_CROSSED_NUM (parent_a)
- += ALLOCNO_CALLS_CROSSED_NUM (a);
- ALLOCNO_CHEAP_CALLS_CROSSED_NUM (parent_a)
- += ALLOCNO_CHEAP_CALLS_CROSSED_NUM (a);
- ALLOCNO_CROSSED_CALLS_ABIS (parent_a)
- |= ALLOCNO_CROSSED_CALLS_ABIS (a);
- ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS (parent_a)
- |= ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS (a);
+ if (!ira_caller_save_loop_spill_p (parent_a, a, spill_cost))
+ {
+ ALLOCNO_CALL_FREQ (parent_a) += ALLOCNO_CALL_FREQ (a);
+ ALLOCNO_CALLS_CROSSED_NUM (parent_a)
+ += ALLOCNO_CALLS_CROSSED_NUM (a);
+ ALLOCNO_CHEAP_CALLS_CROSSED_NUM (parent_a)
+ += ALLOCNO_CHEAP_CALLS_CROSSED_NUM (a);
+ ALLOCNO_CROSSED_CALLS_ABIS (parent_a)
+ |= ALLOCNO_CROSSED_CALLS_ABIS (a);
+ ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS (parent_a)
+ |= ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS (a);
+ }
ALLOCNO_EXCESS_PRESSURE_POINTS_NUM (parent_a)
+= ALLOCNO_EXCESS_PRESSURE_POINTS_NUM (a);
aclass = ALLOCNO_CLASS (a);
diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 4344ee6689e..1487afc5ef1 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -3597,11 +3597,16 @@ move_spill_restore (void)
propagate_allocno_info will have propagated
the cost of spilling HARD_REGNO in SUBLOOP_NODE.
(ira_subloop_allocnos_can_differ_p must be true
-in that case.) Otherwise, SPILL_COST acted as
-a cap on the propagated register cost, in cases
-where the allocations can differ. */
+in that case.) If HARD_REGNO is a caller-saved
+register, we might have modelled it in the same way.
+
+Otherwise, SPILL_COST acted as a cap on the propagated
+register cost, in cases where the allocations can differ. */
auto conflicts = ira_total_conflict_hard_regs (subloop_allocno);
- if (TEST_HARD_REG_BIT (conflicts, hard_regno))
+ if (TEST_HARD_REG_BIT (conflicts, hard_regno)
+ || (ira_need_caller_save_p (subloop_allocno, hard_regno)
+ && ira_caller_save_loop_spill_p (a, subloop_allocno,
+ spill_cost)))
reg_cost = spill_cost;
else if (ira_subloop_allocno
