On 14/10/2025 16:26, Matt Parks wrote:
> The update is the same code as you have seen before, but I re-mailed it with
> changes to webmail settings to be plain text instead of html and hopefully
> fixing the formatting issues you had before. I have not gotten any e-mail
> from Richard either via the gcc-patches list or directly; do you know the
> nature of these concerns?
>
> Thanks,
> Matt
>
>> On 10/13/2025 5:17 PM EDT Christophe Lyon <[email protected]> wrote:
>>
>>
>> On Mon, 13 Oct 2025 at 21:00, Matt Parks <[email protected]> wrote:
>>>
>>> This patch fixes PR117366:
>>> arm thumb1 epilogue size optimizer violates -ffixed-r4.
>>>
>>> gcc/ChangeLog:
>>> PR target/117366
>>> * arm.cc (thumb1_extra_regs_pushed): Take fixed regs into account.
>>>
>> Thanks for the update.
>> I think Richard has some worries / comments?
>>
Apologies for the delay responding to this; I haven't forgotten about it.
In fact, I did start looking at the general problem of changing -ffixed-reg and
-fcall-used-reg etc, earlier this year, but it showed up a number of problems
in the thumb1 code and I wasn't able to finish off my changes. I'll try to get
to this during stage3.
On the specific patch, the change to thumb1_extra_regs_pushed is certainly on
the right lines, but the test is too fragile to use in its current form. The
problem with it is that r7 is the thumb1 frame pointer, so trying expecting a
specific behaviour from the compiler in terms of saving or restoring the value
is going to lead to instability in the test output if the implicit compilation
options (or options passed to the test suite) are changed. For example,
running the test with -fno-omit-frame-pointer will cause this test to fail.
R.
>> Christophe
>>
>>
>> Christophe
>>
>>> ---
>>> gcc/config/arm/arm.cc | 5 +++--
>>> gcc/testsuite/gcc.target/arm/pr117366.c | 14 ++++++++++++++
>>> 2 files changed, 17 insertions(+), 2 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.target/arm/pr117366.c
>>>
>>> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
>>> index bde06f3fa86..6c3000f0d25 100644
>>> --- a/gcc/config/arm/arm.cc
>>> +++ b/gcc/config/arm/arm.cc
>>> @@ -26746,8 +26746,9 @@ thumb1_extra_regs_pushed (arm_stack_offsets
>>> *offsets, bool for_prologue)
>>> live_regs_mask >>= reg_base;
>>> }
>>>
>>> - while (reg_base + n_free < 8 && !(live_regs_mask & 1)
>>> - && (for_prologue || call_used_or_fixed_reg_p (reg_base + n_free)))
>>> + while (reg_base + n_free <= LAST_LO_REGNUM && !(live_regs_mask & 1)
>>> + && (for_prologue || (call_used_or_fixed_reg_p (reg_base + n_free)
>>> + && !fixed_regs[reg_base + n_free])))
>>> {
>>> live_regs_mask >>= 1;
>>> n_free++;
>>> diff --git a/gcc/testsuite/gcc.target/arm/pr117366.c
>>> b/gcc/testsuite/gcc.target/arm/pr117366.c
>>> new file mode 100644
>>> index 00000000000..b23d114381b
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/pr117366.c
>>> @@ -0,0 +1,14 @@
>>> +
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-Os -ffixed-r4 -ffixed-r5 -ffixed-r6 -ffixed-r7" } */
>>> +/* { dg-require-effective-target arm_arch_v5t_thumb_ok } */
>>> +/* { dg-add-options arm_arch_v5t_thumb } */
>>> +void func(void *, void *, void *, int, int);
>>> +
>>> +int bad_func(void) {
>>> + int a, b, c;
>>> + func(&a, &b, &c, 1, 2);
>>> + return b;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler-not "pop.*r\[4567\]" } } */
>>> --
>>> 2.34.1
