This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: Saving fixed call-saved registers in unwind functions and setjmp callers
- From: Richard Sandiford <richard at codesourcery dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Fri, 07 Sep 2007 17:08:27 +0100
- Subject: Re: RFA: Saving fixed call-saved registers in unwind functions and setjmp callers
- References: <87ir72yxx7.fsf@firetop.home>
Ping^2
Richard Sandiford <richard@codesourcery.com> writes:
> We get better MIPS16 code by marking certain GPRs as fixed. However,
> MIPS16 unwind functions must still save and restore these registers,
> so that the functions can unwind through non-MIPS16 code. MIPS16
> functions that call __builtin_setjmp must likewise save and restore the
> registers because the longjmp may be called indirectly from a non-MIPS16
> function.
>
> At the moment, we don't save the fixed GPRs in these situations.
> The following code usually makes sure that we save & restore all
> call-saved registers:
>
> /* A function that has a nonlocal label that can reach the exit
> block via non-exceptional paths must save all call-saved
> registers. */
> if (current_function_calls_unwind_init
> || (current_function_has_nonlocal_label
> && has_nonexceptional_receiver ()))
> for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> if (! call_used_regs[i] && ! fixed_regs[i] && ! LOCAL_REGNO (i))
> df_set_regs_ever_live (i, true);
>
> but it doesn't consider fixed registers.
>
> Historically, gcc has treated all fixed registers as call-clobbered,
> and I believe it's still the case fixed_regs[i] implies call_used_regs[i]
> for all I, making the fixed_regs check above redundant. However,
> because that assumption was too restrictive, we ended up with an
> optional second array, call_really_used_regs. This array is allowed
> to say that fixed registers aren't call-clobbered after all.
>
> Also, the target-independent parts of gcc have traditionally not messed
> with fixed registers, apart from well-known ones like the stack pointer,
> frame pointer, etc. So there has never been any way of knowing whether
> a given fixed register has just been hidden from the register allocators
> or whether it doesn't exist at all. (We might hide it because of
> efficiency concerns, as for MIPS16, or because the backend needs to
> reserve it for something else.)
>
> All of which makes the "correct" fix less than obvious (at least to me).
> My first attitude was that the traditional "assume fixed registers might
> not exist" behaviour was tied up with the traditional "all fixed registers
> are call-clobbered" behaviour, and that if the backend goes out of its
> way to say that a fixed register isn't in fact call-clobbered, the
> target-independent code can assume that the register exists, and that
> the register needs to be included when saving "all" call-saved registers.
> I'm still persuaded by this argument, and that's what the patch below does.
> However, others might prefer a different fix, such as:
>
> - a new array to say which registers actually exist
> - a new target hook to replace the "for" loop quoted above
>
> or something else. I don't like either of the two approaches above
> personally. Particularly not the second, as I think we do need
> something declarative here. Thoughts?
>
> Pressing on with the patch's fix, the only targets besides MIPS to define
> CALL_REALLY_USED_REGISTERS are: ia64, m32r, rs6000, s390, and sh. I've
> tried to classify the registers that are "call used" but not "call
> really used" below:
>
> ia64 m32r rs6000 s390 sh
> ------------------------------------------------------------------------
> 1. soft frame pointer sfp
> 2. argument pointer ap
> 3. stack pointer r12 r15
> 4. PIC register r12 r12
> 5. thread pointer r13
> 6. constant registers f0 f1
> ------------------------------------------------------------------------
> 7. call-saved p0 r2 r13 macl
> ar.unat r14 mach
> ar.ec a0
> a1
> ------------------------------------------------------------------------
> 8. inaccessible registers f0-f15 misc
> ------------------------------------------------------------------------
>
> (1), (2) and (3) don't matter because they're always live before reload
> anyway. (4) is correct, because if we have a call-saved PIC register,
> we must save and restore it even if the unwind function or setjmp
> caller doesn't need the PIC register itself. (5) and (6) should never
> be restored, even if the function refers to them, and the backend code
> already ensures this. (7) is correct for the same reasons as (4):
> we must save and restore the register even if the function doesn't
> obviously need the register itself.
>
> Thus I think the only problem case is (8), and as I argued above,
> I think we should simply add inaccessible registers to
> call_really_used_regs as well as call_used_regs.
>
> I compiled the attached call-saved-1.c and call-saved-3.c tests for
> the following targets and option combinations:
>
> ia64-linux-gnu:
> default
>
> m32r-elf:
> default
>
> powerpc-linux-gnu:
> default
> -mspe
> -mabi=altivec -maltivec
>
> powerpc64-linux-gnu
> default
> -mspe
> -mabi=altivec -maltivec
>
> s390-linux-gnu
> -m31 -mesa
> -m31 -mzarch -mhard-float
> -m64 -mzarch -mhard-float
> -m31 -mzarch -msoft-float
> -m64 -mzarch -msoft-float
>
> sh-linux-gnu
> default
>
> sh4-linux-gnu
> default
>
> sh64-linux-gnu
> default
>
> There were no differences. I also bootstrapped & regression-tested the
> patch on x86_64-linux-gnu and regression-tested it on mipsisa32r2-sde-elf.
> OK to install? Or should it be done in another way?
>
> Richard
>
>
> gcc/
> * regclass.c (CALL_REALLY_USED_REGNO_P): Move to...
> * hard-reg-set.h: ...here.
> * reload1.c (reload): Check CALL_REALLY_USED_REGNO_P instead of
> call_used_regs and fixed_regs.
> * config/s390/s390.c (s390_conditional_register_usage): Set the
> call_really_used_regs entries for FPRs to 1 if TARGET_SOFT_FLOAT.
> * config/sh/sh.h (CONDITIONAL_REGISTER_USAGE): Set the
> call_really_used_regs entries for invalid registers to 1.
> * config/mips/mips.c (mips_conditional_register_usage): Set the
> call_really_used_regs entries for FPRs to 1 if TARGET_SOFT_FLOAT.
> (mips_global_pointer): Check call_really_used_regs instead
> of call_used_regs.
> (mips_save_reg_p): Likewise. Save $31 if the current function
> calls __builtin_eh_return. Fix indentation. Remove special
> handling for $18.
>
> gcc/testsuite/
> * gcc.target/mips/call-saved-1.c: New test.
> * gcc.target/mips/call-saved-2.c: Likewise.
> * gcc.target/mips/call-saved-3.c: Likewise.
> * gcc.target/mips/mips.exp (setup_mips_tests): Set mips_gp64
> instead of mips_mips64. Set mips_fp64 too.
> (is_gp32_flag): Return true for -mips1 and -mips2.
> (dg-mips-options): Use mips_gp64 instead of mips_mips64.
>
> Index: gcc/regclass.c
> ===================================================================
> --- gcc/regclass.c 2007-08-21 08:17:09.000000000 -0700
> +++ gcc/regclass.c 2007-08-26 10:18:38.000000000 -0700
> @@ -111,13 +111,6 @@ static const char initial_call_used_regs
> char call_really_used_regs[] = CALL_REALLY_USED_REGISTERS;
> #endif
>
> -#ifdef CALL_REALLY_USED_REGISTERS
> -#define CALL_REALLY_USED_REGNO_P(X) call_really_used_regs[X]
> -#else
> -#define CALL_REALLY_USED_REGNO_P(X) call_used_regs[X]
> -#endif
> -
> -
> /* Indexed by hard register number, contains 1 for registers that are
> fixed use or call used registers that cannot hold quantities across
> calls even if we are willing to save and restore them. call fixed
> Index: gcc/hard-reg-set.h
> ===================================================================
> --- gcc/hard-reg-set.h 2007-08-21 08:17:08.000000000 -0700
> +++ gcc/hard-reg-set.h 2007-08-26 03:51:03.000000000 -0700
> @@ -501,6 +501,9 @@ hard_reg_set_empty_p (const HARD_REG_SET
>
> #ifdef CALL_REALLY_USED_REGISTERS
> extern char call_really_used_regs[];
> +#define CALL_REALLY_USED_REGNO_P(X) call_really_used_regs[X]
> +#else
> +#define CALL_REALLY_USED_REGNO_P(X) call_used_regs[X]
> #endif
>
> /* The same info as a HARD_REG_SET. */
> Index: gcc/reload1.c
> ===================================================================
> --- gcc/reload1.c 2007-08-26 03:41:09.000000000 -0700
> +++ gcc/reload1.c 2007-08-26 03:56:30.000000000 -0700
> @@ -746,7 +746,7 @@ reload (rtx first, int global)
> || (current_function_has_nonlocal_label
> && has_nonexceptional_receiver ()))
> for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> - if (! call_used_regs[i] && ! fixed_regs[i] && ! LOCAL_REGNO (i))
> + if (!CALL_REALLY_USED_REGNO_P (i) && !LOCAL_REGNO (i))
> df_set_regs_ever_live (i, true);
>
> /* Find all the pseudo registers that didn't get hard regs
> Index: gcc/config/s390/s390.c
> ===================================================================
> --- gcc/config/s390/s390.c 2007-08-21 08:16:56.000000000 -0700
> +++ gcc/config/s390/s390.c 2007-08-26 10:24:26.000000000 -0700
> @@ -8948,7 +8948,7 @@ s390_conditional_register_usage (void)
> if (TARGET_SOFT_FLOAT)
> {
> for (i = 16; i < 32; i++)
> - call_used_regs[i] = fixed_regs[i] = 1;
> + call_used_regs[i] = call_really_used_regs[i] = fixed_regs[i] = 1;
> }
> }
>
> Index: gcc/config/sh/sh.h
> ===================================================================
> --- gcc/config/sh/sh.h 2007-08-21 08:16:59.000000000 -0700
> +++ gcc/config/sh/sh.h 2007-08-26 10:24:26.000000000 -0700
> @@ -103,7 +103,8 @@ #define CONDITIONAL_REGISTER_USAGE do
> int regno; \
> for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno ++) \
> if (! VALID_REGISTER_P (regno)) \
> - fixed_regs[regno] = call_used_regs[regno] = 1; \
> + fixed_regs[regno] = call_used_regs[regno] \
> + = call_really_used_regs[regno] = 1; \
> /* R8 and R9 are call-clobbered on SH5, but not on earlier SH ABIs. */ \
> if (TARGET_SH5) \
> { \
> Index: gcc/config/mips/mips.c
> ===================================================================
> --- gcc/config/mips/mips.c 2007-08-26 03:41:09.000000000 -0700
> +++ gcc/config/mips/mips.c 2007-08-26 10:47:10.000000000 -0700
> @@ -5978,7 +5978,8 @@ mips_conditional_register_usage (void)
> int regno;
>
> for (regno = FP_REG_FIRST; regno <= FP_REG_LAST; regno++)
> - fixed_regs[regno] = call_used_regs[regno] = 1;
> + fixed_regs[regno] = call_used_regs[regno]
> + = call_really_used_regs[regno] = 1;
> for (regno = ST_REG_FIRST; regno <= ST_REG_LAST; regno++)
> fixed_regs[regno] = call_used_regs[regno] = 1;
> }
> @@ -7006,7 +7007,7 @@ mips_global_pointer (void)
> if (TARGET_CALL_SAVED_GP && current_function_is_leaf)
> for (regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++)
> if (!df_regs_ever_live_p (regno)
> - && call_used_regs[regno]
> + && CALL_REALLY_USED_REGNO_P (regno)
> && !fixed_regs[regno]
> && regno != PIC_FUNCTION_ADDR_REGNUM)
> return regno;
> @@ -7072,43 +7073,32 @@ mips_save_reg_p (unsigned int regno)
> return TARGET_CALL_SAVED_GP && cfun->machine->global_pointer == regno;
>
> /* Check call-saved registers. */
> - if (df_regs_ever_live_p (regno) && !call_used_regs[regno])
> + if (df_regs_ever_live_p (regno) && !CALL_REALLY_USED_REGNO_P (regno))
> return true;
>
> - /* Save both registers in an FPR pair if either one is used. This is
> - needed for the case when MIN_FPRS_PER_FMT == 1, which allows the odd
> - register to be used without the even register. */
> - if (FP_REG_P (regno)
> - && MAX_FPRS_PER_FMT == 2
> - && df_regs_ever_live_p (regno + 1)
> - && !call_used_regs[regno + 1])
> - return true;
> + /* Save both registers in an FPR pair if either one is used. This is
> + needed for the case when MIN_FPRS_PER_FMT == 1, which allows the odd
> + register to be used without the even register. */
> + if (FP_REG_P (regno)
> + && MAX_FPRS_PER_FMT == 2
> + && df_regs_ever_live_p (regno + 1)
> + && !CALL_REALLY_USED_REGNO_P (regno + 1))
> + return true;
>
> /* We need to save the old frame pointer before setting up a new one. */
> if (regno == HARD_FRAME_POINTER_REGNUM && frame_pointer_needed)
> return true;
>
> /* We need to save the incoming return address if it is ever clobbered
> - within the function. */
> - if (regno == GP_REG_FIRST + 31 && df_regs_ever_live_p (regno))
> + within the function, if __builtin_eh_return is being used to set a
> + different return address, or if a stub is being used to return a
> + value in FPRs. */
> + if (regno == GP_REG_FIRST + 31
> + && (df_regs_ever_live_p (regno)
> + || current_function_calls_eh_return
> + || mips16_cfun_returns_in_fpr_p ()))
> return true;
>
> - if (TARGET_MIPS16)
> - {
> - /* $18 is a special case in mips16 code. It may be used to call
> - a function which returns a floating point value, but it is
> - marked in call_used_regs. */
> - if (regno == GP_REG_FIRST + 18 && df_regs_ever_live_p (regno))
> - return true;
> -
> - /* $31 is also a special case. It will be used to copy a return
> - value into the floating point registers if the return value is
> - floating point. */
> - if (regno == GP_REG_FIRST + 31
> - && mips16_cfun_returns_in_fpr_p ())
> - return true;
> - }
> -
> return false;
> }
>
> Index: gcc/testsuite/gcc.target/mips/call-saved-1.c
> ===================================================================
> --- /dev/null 2007-05-10 18:31:20.000000000 -0700
> +++ gcc/testsuite/gcc.target/mips/call-saved-1.c 2007-08-26 03:42:00.000000000 -0700
> @@ -0,0 +1,20 @@
> +/* Check that we save all call-saved GPRs in a MIPS16 __builtin_eh_return
> + function. */
> +/* { dg-mips-options "-mips2 -mips16 -mno-abicalls" } */
> +
> +void bar (void);
> +void
> +foo (int x)
> +{
> + __builtin_unwind_init ();
> + __builtin_eh_return (x, bar);
> +}
> +/* { dg-final { scan-assembler "\\\$16" } } */
> +/* { dg-final { scan-assembler "\\\$17" } } */
> +/* { dg-final { scan-assembler "\\\$18" } } */
> +/* { dg-final { scan-assembler "\\\$19" } } */
> +/* { dg-final { scan-assembler "\\\$20" } } */
> +/* { dg-final { scan-assembler "\\\$21" } } */
> +/* { dg-final { scan-assembler "\\\$22" } } */
> +/* { dg-final { scan-assembler "\\\$23" } } */
> +/* { dg-final { scan-assembler "\\\$(30|fp)" } } */
> Index: gcc/testsuite/gcc.target/mips/call-saved-2.c
> ===================================================================
> --- /dev/null 2007-05-10 18:31:20.000000000 -0700
> +++ gcc/testsuite/gcc.target/mips/call-saved-2.c 2007-08-26 03:42:00.000000000 -0700
> @@ -0,0 +1,18 @@
> +/* Check that we save non-MIPS16 GPRs if they are explicitly clobbered. */
> +/* { dg-mips-options "-mips2 -mips16 -mno-abicalls -O2" } */
> +
> +void
> +foo (void)
> +{
> + asm volatile ("" ::: "$19", "$23", "$24", "$30");
> +}
> +/* { dg-final { scan-assembler-not "\\\$16" } } */
> +/* { dg-final { scan-assembler-not "\\\$17" } } */
> +/* { dg-final { scan-assembler-not "\\\$18" } } */
> +/* { dg-final { scan-assembler "\\\$19" } } */
> +/* { dg-final { scan-assembler-not "\\\$20" } } */
> +/* { dg-final { scan-assembler-not "\\\$21" } } */
> +/* { dg-final { scan-assembler-not "\\\$22" } } */
> +/* { dg-final { scan-assembler "\\\$23" } } */
> +/* { dg-final { scan-assembler-not "\\\$24" } } */
> +/* { dg-final { scan-assembler "\\\$(30|fp)" } } */
> Index: gcc/testsuite/gcc.target/mips/call-saved-3.c
> ===================================================================
> --- /dev/null 2007-05-10 18:31:20.000000000 -0700
> +++ gcc/testsuite/gcc.target/mips/call-saved-3.c 2007-08-26 03:42:38.000000000 -0700
> @@ -0,0 +1,21 @@
> +/* Check that we save all call-saved GPRs in a MIPS16 __builtin_setjmp
> + function. */
> +/* { dg-mips-options "-mips2 -mips16 -mno-abicalls -O2" } */
> +
> +void bar (void);
> +extern int buf[];
> +void
> +foo (int x)
> +{
> + if (__builtin_setjmp (buf) == 0)
> + bar();
> +}
> +/* { dg-final { scan-assembler "\\\$16" } } */
> +/* { dg-final { scan-assembler "\\\$17" } } */
> +/* { dg-final { scan-assembler "\\\$18" } } */
> +/* { dg-final { scan-assembler "\\\$19" } } */
> +/* { dg-final { scan-assembler "\\\$20" } } */
> +/* { dg-final { scan-assembler "\\\$21" } } */
> +/* { dg-final { scan-assembler "\\\$22" } } */
> +/* { dg-final { scan-assembler "\\\$23" } } */
> +/* { dg-final { scan-assembler "\\\$(30|fp)" } } */
> Index: gcc/testsuite/gcc.target/mips/mips.exp
> ===================================================================
> --- gcc/testsuite/gcc.target/mips/mips.exp 2007-08-26 03:41:09.000000000 -0700
> +++ gcc/testsuite/gcc.target/mips/mips.exp 2007-08-26 10:23:37.000000000 -0700
> @@ -31,7 +31,8 @@ load_lib gcc-dg.exp
> # $mips_isa: the ISA level specified by __mips
> # $mips_arch: the architecture specified by _MIPS_ARCH
> # $mips_mips16: true if MIPS16 mode is selected
> -# $mips_mips64: true if 64-bit output is selected
> +# $mips_gp64: true if 64-bit output is selected
> +# $mips_fp64: true if 64-bit FPRs are selected
> # $mips_float: "hard" or "soft"
> #
> # $mips_forced_isa: true if the command line uses -march=* or -mips*
> @@ -41,7 +42,8 @@ proc setup_mips_tests {} {
> global mips_isa
> global mips_arch
> global mips_mips16
> - global mips_mips64
> + global mips_gp64
> + global mips_fp64
> global mips_float
>
> global mips_forced_isa
> @@ -60,7 +62,10 @@ proc setup_mips_tests {} {
> int mips16 = 1;
> #endif
> #ifdef __mips64
> - int mips64 = 1;
> + int gp64 = 1;
> + #endif
> + #if __mips_fpr==64
> + int fp64 = 1;
> #endif
> #ifdef __mips_hard_float
> const char *float = "hard";
> @@ -75,7 +80,8 @@ proc setup_mips_tests {} {
> regexp {isa = ([^;]*)} $output dummy mips_isa
> regexp {arch = "([^"]*)} $output dummy mips_arch
> set mips_mips16 [regexp {mips16 = 1} $output]
> - set mips_mips64 [regexp {mips64 = 1} $output]
> + set mips_gp64 [regexp {gp64 = 1} $output]
> + set mips_fp64 [regexp {fp64 = 1} $output]
> regexp {float = "([^"]*)} $output dummy mips_float
>
> set mips_forced_isa [regexp -- {(-mips|-march)} $compiler_flags]
> @@ -87,6 +93,7 @@ proc setup_mips_tests {} {
> proc is_gp32_flag {flag} {
> switch -glob -- $flag {
> -msmartmips -
> + -mips[12] -
> -march=mips32* -
> -mgp32 { return 1 }
> default { return 0 }
> @@ -124,7 +131,8 @@ proc dg-mips-options {args} {
> global mips_isa
> global mips_arch
> global mips_mips16
> - global mips_mips64
> + global mips_gp64
> + global mips_fp64
> global mips_float
>
> global mips_forced_isa
> @@ -136,13 +144,15 @@ proc dg-mips-options {args} {
>
> # First handle the -mgp* options. Add an architecture option if necessary.
> foreach flag $flags {
> - if {[is_gp32_flag $flag] && $mips_mips64} {
> + if {[is_gp32_flag $flag]
> + && ($mips_gp64
> + || ($mips_fp64 && [lsearch $flags -mfp64] < 0)) } {
> if {$mips_forced_abi} {
> set matches 0
> } else {
> append flags " -mabi=32"
> }
> - } elseif {$flag == "-mgp64" && !$mips_mips64} {
> + } elseif {$flag == "-mgp64" && !$mips_gp64} {
> if {$mips_forced_abi} {
> set matches 0
> } else {