Thanks for the feedback. If I can change the test to avoid r7 (demonstrate issue with only r4/r5/r6) will that be sufficiently robust, or do you know of any other potential sensitivity to compiler flags?
Matt > On 12/03/2025 5:13 AM EST Richard Earnshaw (foss) <[email protected]> > wrote: > > > 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
