On Thu, Nov 7, 2024 at 11:24 AM Jakub Jelinek <[email protected]> wrote:
>
> On Thu, Nov 07, 2024 at 09:12:34AM +0100, Uros Bizjak wrote:
> > On Thu, Nov 7, 2024 at 9:00 AM Jakub Jelinek <[email protected]> wrote:
> > >
> > > On Thu, Nov 07, 2024 at 08:47:34AM +0100, Uros Bizjak wrote:
> > > > Maybe we should always recognize "redzone", even for targets without
> > > > it. This is the way we recognize "cc" even for targets without CC reg
> > > > (e.g. alpha). This would simplify the definition and processing - if
> > > > the hook returns NULL_RTX (the default), then it (obviously) won't be
> > > > added to the clobber list.
> > >
> > > Dunno, am open to that, but thought it would be just weird if one says
> > > "redzone" on targets which don't have such a concept.
> >
> > Let's look at the situation with x86_32 and x86_64. The "redzone" for
> > the former is just an afterthought, so we can safely say that it
> > doesn't support it. So, the code that targets both targets (e.g. linux
> > kernel) would (in a pedantic way) have to redefine many shared asm
> > defines, one to have clobber and one without it. We don't want that,
> > we want one definition and "let's compiler sort it out".
> >
> > For targets without clobber concept, well - don't add it to the
> > clobber list if it is always ineffective. One *can* add "cc" to all
> > alpha asms, but well.. ;)
>
> Ok, here is a variant of the patch which just ignores "redzone" clobber if
> it doesn't make sense.
>
> 2024-11-07 Jakub Jelinek <[email protected]>
>
> gcc/
> * target.def (redzone_clobber): New target hook.
> * varasm.cc (decode_reg_name_and_count): Return -5 for
> "redzone".
> * cfgexpand.cc (expand_asm_stmt): Handle redzone clobber.
> * config/i386/i386.h (struct machine_function): Add
> asm_redzone_clobber_seen member.
> * config/i386/i386.cc (ix86_compute_frame_layout): Don't
> use red zone if cfun->machine->asm_redzone_clobber_seen.
> (ix86_redzone_clobber): New function.
> (TARGET_REDZONE_CLOBBER): Redefine.
> * doc/extend.texi (Clobbers and Scratch Registers): Document
> the "redzone" clobber.
> * doc/tm.texi.in: Add @hook TARGET_REDZONE_CLOBBER.
> * doc/tm.texi: Regenerate.
> gcc/testsuite/
> * gcc.dg/asm-redzone-1.c: New test.
> * gcc.target/i386/asm-redzone-1.c: New test.
OK for the x86 part, LGTM for other parts.
Thanks,
Uros.
>
> --- gcc/target.def.jj 2024-11-06 18:53:10.836843793 +0100
> +++ gcc/target.def 2024-11-07 10:57:58.697898800 +0100
> @@ -3376,6 +3376,16 @@ to be used.",
> bool, (machine_mode mode),
> NULL)
>
> +DEFHOOK
> +(redzone_clobber,
> + "Define this to return some RTL for the @code{redzone} @code{asm} clobber\n\
> +if target has a red zone and wants to support the @code{redzone} clobber\n\
> +or return NULL if the clobber should be ignored.\n\
> +\n\
> +The default is to ignore the @code{redzone} clobber.",
> + rtx, (),
> + NULL)
> +
> /* Support for named address spaces. */
> #undef HOOK_PREFIX
> #define HOOK_PREFIX "TARGET_ADDR_SPACE_"
> --- gcc/varasm.cc.jj 2024-11-06 18:53:10.838843765 +0100
> +++ gcc/varasm.cc 2024-11-07 10:55:46.858763724 +0100
> @@ -965,9 +965,11 @@ set_user_assembler_name (tree decl, cons
>
> /* Decode an `asm' spec for a declaration as a register name.
> Return the register number, or -1 if nothing specified,
> - or -2 if the ASMSPEC is not `cc' or `memory' and is not recognized,
> + or -2 if the ASMSPEC is not `cc' or `memory' or `redzone' and is not
> + recognized,
> or -3 if ASMSPEC is `cc' and is not recognized,
> - or -4 if ASMSPEC is `memory' and is not recognized.
> + or -4 if ASMSPEC is `memory' and is not recognized,
> + or -5 if ASMSPEC is `redzone' and is not recognized.
> Accept an exact spelling or a decimal number.
> Prefixes such as % are optional. */
>
> @@ -1034,6 +1036,9 @@ decode_reg_name_and_count (const char *a
> }
> #endif /* ADDITIONAL_REGISTER_NAMES */
>
> + if (!strcmp (asmspec, "redzone"))
> + return -5;
> +
> if (!strcmp (asmspec, "memory"))
> return -4;
>
> --- gcc/cfgexpand.cc.jj 2024-11-06 18:53:10.803844259 +0100
> +++ gcc/cfgexpand.cc 2024-11-07 11:00:16.212953571 +0100
> @@ -3205,6 +3205,12 @@ expand_asm_stmt (gasm *stmt)
> rtx x = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode));
> clobber_rvec.safe_push (x);
> }
> + else if (j == -5)
> + {
> + if (targetm.redzone_clobber)
> + if (rtx x = targetm.redzone_clobber ())
> + clobber_rvec.safe_push (x);
> + }
> else
> {
> /* Otherwise we should have -1 == empty string
> --- gcc/config/i386/i386.h.jj 2024-11-06 18:53:10.807844203 +0100
> +++ gcc/config/i386/i386.h 2024-11-07 10:55:46.904763076 +0100
> @@ -2881,6 +2881,9 @@ struct GTY(()) machine_function {
> /* True if red zone is used. */
> BOOL_BITFIELD red_zone_used : 1;
>
> + /* True if inline asm with redzone clobber has been seen. */
> + BOOL_BITFIELD asm_redzone_clobber_seen : 1;
> +
> /* The largest alignment, in bytes, of stack slot actually used. */
> unsigned int max_used_stack_alignment;
>
> --- gcc/config/i386/i386.cc.jj 2024-11-06 18:53:10.807844203 +0100
> +++ gcc/config/i386/i386.cc 2024-11-07 10:55:46.947762468 +0100
> @@ -7171,6 +7171,7 @@ ix86_compute_frame_layout (void)
> if (ix86_using_red_zone ()
> && crtl->sp_is_unchanging
> && crtl->is_leaf
> + && !cfun->machine->asm_redzone_clobber_seen
> && !ix86_pc_thunk_call_expanded
> && !ix86_current_function_calls_tls_descriptor)
> {
> @@ -26268,6 +26269,22 @@ ix86_mode_can_transfer_bits (machine_mod
> return true;
> }
>
> +/* Implement TARGET_REDZONE_CLOBBER. */
> +static rtx
> +ix86_redzone_clobber ()
> +{
> + cfun->machine->asm_redzone_clobber_seen = true;
> + if (ix86_using_red_zone ())
> + {
> + rtx base = plus_constant (Pmode, stack_pointer_rtx,
> + GEN_INT (-RED_ZONE_SIZE));
> + rtx mem = gen_rtx_MEM (BLKmode, base);
> + set_mem_size (mem, RED_ZONE_SIZE);
> + return mem;
> + }
> + return NULL_RTX;
> +}
> +
> /* Target-specific selftests. */
>
> #if CHECKING_P
> @@ -27121,6 +27138,9 @@ ix86_libgcc_floating_mode_supported_p
> #undef TARGET_MODE_CAN_TRANSFER_BITS
> #define TARGET_MODE_CAN_TRANSFER_BITS ix86_mode_can_transfer_bits
>
> +#undef TARGET_REDZONE_CLOBBER
> +#define TARGET_REDZONE_CLOBBER ix86_redzone_clobber
> +
> static bool
> ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED)
> {
> --- gcc/doc/extend.texi.jj 2024-11-06 18:53:10.826843934 +0100
> +++ gcc/doc/extend.texi 2024-11-07 10:57:07.550622297 +0100
> @@ -11827,7 +11827,7 @@ asm volatile ("movc3 %0, %1, %2"
> : "r0", "r1", "r2", "r3", "r4", "r5", "memory");
> @end example
>
> -Also, there are two special clobber arguments:
> +Also, there are three special clobber arguments:
>
> @table @code
> @item "cc"
> @@ -11855,6 +11855,18 @@ Note that this clobber does not prevent
> speculative reads past the @code{asm} statement. To prevent that, you need
> processor-specific fence instructions.
>
> +@item "redzone"
> +The @code{"redzone"} clobber tells the compiler that the assembly code
> +may write to the stack red zone, area below the stack pointer which on
> +some architectures in some calling conventions is guaranteed not to be
> +changed by signal handlers, interrupts or exceptions and so the compiler
> +can store there temporaries in leaf functions. On targets which have
> +no concept of the stack red zone, the clobber is ignored.
> +It should be used e.g.@: in case the assembly code uses call instructions
> +or pushes something to the stack without taking the red zone into account
> +by subtracting red zone size from the stack pointer first and restoring
> +it afterwards.
> +
> @end table
>
> Flushing registers to memory has performance implications and may be
> --- gcc/doc/tm.texi.in.jj 2024-11-06 18:53:10.833843835 +0100
> +++ gcc/doc/tm.texi.in 2024-11-07 10:55:46.998761746 +0100
> @@ -3464,6 +3464,8 @@ stack.
>
> @hook TARGET_MODE_CAN_TRANSFER_BITS
>
> +@hook TARGET_REDZONE_CLOBBER
> +
> @hook TARGET_TRANSLATE_MODE_ATTRIBUTE
>
> @hook TARGET_SCALAR_MODE_SUPPORTED_P
> --- gcc/doc/tm.texi.jj 2024-11-06 18:53:10.831843863 +0100
> +++ gcc/doc/tm.texi 2024-11-07 10:55:47.008761605 +0100
> @@ -4563,6 +4563,14 @@ The default is to assume modes with the
> to be used.
> @end deftypefn
>
> +@deftypefn {Target Hook} rtx TARGET_REDZONE_CLOBBER ()
> +Define this to return some RTL for the @code{redzone} @code{asm} clobber
> +if target has a red zone and wants to support the @code{redzone} clobber
> +or return NULL if the clobber should be ignored.
> +
> +The default is to ignore the @code{redzone} clobber.
> +@end deftypefn
> +
> @deftypefn {Target Hook} machine_mode TARGET_TRANSLATE_MODE_ATTRIBUTE
> (machine_mode @var{mode})
> Define this hook if during mode attribute processing, the port should
> translate machine_mode @var{mode} to another mode. For example, rs6000's
> --- gcc/testsuite/gcc.dg/asm-redzone-1.c.jj 2024-11-07 11:07:52.873493855
> +0100
> +++ gcc/testsuite/gcc.dg/asm-redzone-1.c 2024-11-07 11:07:45.697595356
> +0100
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +void
> +foo (void)
> +{
> + asm ("" : : : "cc", "memory", "redzone");
> +}
> --- gcc/testsuite/gcc.target/i386/asm-redzone-1.c.jj 2024-11-07
> 10:55:47.018761463 +0100
> +++ gcc/testsuite/gcc.target/i386/asm-redzone-1.c 2024-11-07
> 10:55:47.018761463 +0100
> @@ -0,0 +1,38 @@
> +/* { dg-do run { target lp64 } } */
> +/* { dg-options "-O2" } */
> +
> +__attribute__((noipa)) int
> +foo (void)
> +{
> + int a = 1;
> + int b = 2;
> + int c = 3;
> + int d = 4;
> + int e = 5;
> + int f = 6;
> + int g = 7;
> + int h = 8;
> + int i = 9;
> + int j = 10;
> + int k = 11;
> + int l = 12;
> + int m = 13;
> + int n = 14;
> + asm volatile ("" : "+g" (a), "+g" (b), "+g" (c), "+g" (d), "+g" (e));
> + asm volatile ("" : "+g" (f), "+g" (g), "+g" (h), "+g" (i), "+g" (j));
> + asm volatile ("" : "+g" (k), "+g" (l), "+g" (m), "+g" (n));
> + asm volatile ("{pushq %%rax; pushq %%rax; popq %%rax; popq %%rax"
> + "|push rax;push rax;pop rax;pop rax}"
> + : : : "ax", "si", "di", "r10", "r11", "redzone");
> + asm volatile ("" : "+g" (a), "+g" (b), "+g" (c), "+g" (d), "+g" (e));
> + asm volatile ("" : "+g" (f), "+g" (g), "+g" (h), "+g" (i), "+g" (j));
> + asm volatile ("" : "+g" (k), "+g" (l), "+g" (m), "+g" (n));
> + return a + b + c + d + e + f + g + h + i + j + k + l + m + n;
> +}
> +
> +int
> +main ()
> +{
> + if (foo () != 105)
> + __builtin_abort ();
> +}
>
>
> Jakub
>
