On 4/19/25 3:29 PM, Denis Chertykov wrote:
Bugfix for PR118591
This bug occurs only with '-Os' option.
The function 'inherit_reload_reg ()' have a wrong condition:
--------------------------------------------------------------------------------
static bool
inherit_reload_reg (bool def_p, int original_regno,
enum reg_class cl, rtx_insn *insn, rtx next_usage_insns)
{
if (optimize_function_for_size_p (cfun))
------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
return false;
--------------------------------------------------------------------------------
It's wrong because we heed an inheritance and we need to undoing it after
unsuccessful pass.
I applied the following patch:
--------------------------------------------------------------------------------
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index 7dbc7fe1e00..af2d2793159 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -5884,7 +5884,11 @@ inherit_reload_reg (bool def_p, int original_regno,
enum reg_class cl, rtx_insn *insn, rtx next_usage_insns)
{
if (optimize_function_for_size_p (cfun))
- return false;
+ {
+ if (lra_dump_file != NULL)
+ fprintf (lra_dump_file,
+ " <<<<<< inheritance for -Os <<<<<<<<<\n");
+ }
--------------------------------------------------------------------------------
Debug output from patched gcc:
----------------------- Fragment from t.c.323r.reload --------------------------
********** Inheritance #1: **********
EBB 2
EBB 5
EBB 3
<<<<<< inheritance for -Os <<<<<<<<<
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Use smallest class of LD_REGS and GENERAL_REGS
Creating newreg=59 from oldreg=43, assigning class LD_REGS to
inheritance r59
Original reg change 43->59 (bb3):
58: r59:SI=r57:SI
Add original<-inheritance after:
60: r43:SI=r59:SI
Inheritance reuse change 43->59 (bb3):
59: r58:SI=r59:SI
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
<<<<<< inheritance for -Os <<<<<<<<<
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Use smallest class of ALL_REGS and GENERAL_REGS
Creating newreg=60 from oldreg=43, assigning class ALL_REGS to
inheritance r60
Original reg change 43->60 (bb3):
56: r56:QI=r60:SI#0
Add inheritance<-original before:
61: r60:SI=r43:SI
Inheritance reuse change 43->60 (bb3):
57: r57:SI=r60:SI
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
<<<<<< inheritance for -Os <<<<<<<<<
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Use smallest class of ALL_REGS and GENERAL_REGS
Creating newreg=61 from oldreg=43, assigning class ALL_REGS to
inheritance r61
Original reg change 43->61 (bb3):
55: r55:QI=r61:SI#1
Add inheritance<-original before:
62: r61:SI=r43:SI
Inheritance reuse change 43->61 (bb3):
61: r60:SI=r61:SI
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
<<<<<< inheritance for -Os <<<<<<<<<
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Use smallest class of ALL_REGS and GENERAL_REGS
Creating newreg=62 from oldreg=43, assigning class ALL_REGS to
inheritance r62
Original reg change 43->62 (bb3):
54: r54:QI=r62:SI#2
Add inheritance<-original before:
63: r62:SI=r43:SI
Inheritance reuse change 43->62 (bb3):
62: r61:SI=r62:SI
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
<<<<<< inheritance for -Os <<<<<<<<<
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Use smallest class of ALL_REGS and GENERAL_REGS
Creating newreg=63 from oldreg=43, assigning class ALL_REGS to
inheritance r63
Original reg change 43->63 (bb3):
53: r53:QI=r63:SI#3
Add inheritance<-original before:
64: r63:SI=r43:SI
Inheritance reuse change 43->63 (bb3):
63: r62:SI=r63:SI
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
EBB 4
********** Pseudo live ranges #1: **********
BB 4
Insn 43: point = 0, n_alt = -1
[...]
Assign 24 to reload r56 (freq=2000)
Reassigning non-reload pseudos
Assign 24 to r43 (freq=3000)
********** Undoing inheritance #1: **********
Inherit 5 out of 5 (100.00%)
********** Local #2: **********
[...]
--------------------------------------------------------------------------------
So, we need 'Inheritance' and we need 'Undoing inheritance'.
It is difficult for me to understand AVR code but I think the reason for
the bug is in something else. And the fix should be different.
Inheritance can increase the code size. In fact the code was added for
PR59535 to solve code size regression for ARM Thumb.
Sorry, I can not approve the patch.
The patch:
PR rtl-optimization/118591
gcc/
* lra-constraints.cc (inherit_reload_reg): Do inheritance for any
optimization.
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index 7dbc7fe1e00..bfdab4adc34 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -5883,8 +5883,6 @@ static bool
inherit_reload_reg (bool def_p, int original_regno,
enum reg_class cl, rtx_insn *insn, rtx next_usage_insns)
{
- if (optimize_function_for_size_p (cfun))
- return false;
enum reg_class rclass = lra_get_allocno_class (original_regno);
rtx original_reg = regno_reg_rtx[original_regno];
