Re: [PATCH] inline-asm, i386: Add "redzone" clobber support
On Thu, Nov 7, 2024 at 9:00 AM Jakub Jelinek 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.. ;) Uros.
Re: [PATCH] inline-asm, i386: Add "redzone" clobber support
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. > > --- gcc/varasm.cc.jj2024-11-06 10:23:21.215728007 +0100 > > +++ gcc/varasm.cc 2024-11-06 12:21:17.891908352 +0100 > > @@ -967,9 +967,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. > > Comma at the end of this part of the sentence. Fixed in my copy. > > --- gcc/config/i386/i386.cc.jj 2024-11-05 08:55:41.661186354 +0100 > > +++ gcc/config/i386/i386.cc 2024-11-06 14:12:52.560168428 +0100 > > @@ -7172,7 +7172,8 @@ ix86_compute_frame_layout (void) > >&& crtl->sp_is_unchanging > >&& crtl->is_leaf > >&& !ix86_pc_thunk_call_expanded > > - && !ix86_current_function_calls_tls_descriptor) > > + && !ix86_current_function_calls_tls_descriptor > > + && !cfun->machine->asm_redzone_clobber_seen) > > Please put the new check just after crtl-> checks. Changed in my copy too. The patch passed bootstrap/regtest on x86_64-linux and i686-linux btw. Jakub
Re: [PATCH] inline-asm, i386: Add "redzone" clobber support
On Wed, Nov 6, 2024 at 2:50 PM Jakub Jelinek wrote:
>
> Hi!
>
> The following patch adds a "redzone" clobber (recognized just
> on targets which choose to recognize it, right now just x86),
> with which one can mark the rare case where inline asm pushes
> something on the stack or uses call instruction without taking
> red zone into account (i.e. addq $-128, %rsp; and addq $128, %rsp
> around that).
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.
If someone adds "redzone" clobber to the target without redzone it
won't hurt if it is simply ignored.
> 2024-11-06 Jakub Jelinek
>
> 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.target/i386/asm-redzone-1.c: New test.
>
> --- gcc/target.def.jj 2024-10-25 10:00:29.525767041 +0200
> +++ gcc/target.def 2024-11-06 13:32:39.376320955 +0100
> @@ -3376,6 +3376,18 @@ 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 shouldn't be recognized or return\n\
> +@code{pc_rtx} if the clobber should be recognized but should not be added\n\
> +to the list of clobbers.\n\
> +\n\
> +The default is not to support the @code{redzone} clobber.",
> + rtx, (),
> + NULL)
> +
> /* Support for named address spaces. */
> #undef HOOK_PREFIX
> #define HOOK_PREFIX "TARGET_ADDR_SPACE_"
> --- gcc/varasm.cc.jj2024-11-06 10:23:21.215728007 +0100
> +++ gcc/varasm.cc 2024-11-06 12:21:17.891908352 +0100
> @@ -967,9 +967,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.
Comma at the end of this part of the sentence.
> + or -5 if ASMSPEC is `redzone' and is not recognized.
> Accept an exact spelling or a decimal number.
> Prefixes such as % are optional. */
>
> @@ -1036,6 +1038,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-05 08:55:41.610187084 +0100
> +++ gcc/cfgexpand.cc2024-11-06 14:26:07.684918717 +0100
> @@ -3193,6 +3193,19 @@ expand_asm_stmt (gasm *stmt)
> j = decode_reg_name_and_count (regname, &nregs);
> if (j < 0)
> {
> + if (j == -5)
> + {
> + rtx x = NULL_RTX;
> + if (targetm.redzone_clobber)
> + x = targetm.redzone_clobber ();
> + if (x == pc_rtx)
> + /* Recognize it but don't add anything to
> + clobber_rvec. */;
> + else if (x)
> + clobber_rvec.safe_push (x);
> + else
> + j = -2;
> + }
> if (j == -2)
> {
> /* ??? Diagnose during gimplification? */
> @@ -3208,8 +3221,9 @@ expand_asm_stmt (gasm *stmt)
> else
> {
> /* Otherwise we should have -1 == empty string
> -or -3 == cc, which is not a register. */
> - gcc_assert (j == -1 || j == -3);
> +or -3 == cc, which is not a register or
> +-5 == redzone, ditto. */
> + gcc_assert (j == -1 || j == -3 || j == -5);
> }
> }
> else
> --- gcc/config/i386/i386.h.jj 2024-11-05 08:55:41.663186325 +0100
> +++ gcc/config/i386/
Re: [PATCH] inline-asm, i386: Add "redzone" clobber support
On Wed, Nov 06, 2024 at 06:04:43PM +0100, Jan Hubicka wrote: > LGTM, though if asm needs temporary memory it can ask for it explicitly. Sure, but the "redzone" is not about needing extra temporary memory, but about asking the compiler not to put any of its own temporaries in the red zone. The temporary memory could be a hack to avoid moving inline asm with call into the prologue or epilogue. > I guess this can be practical to hide actual call in the asm statement > (which will break unwinding I guess). When using .cfi_* directives, inline asm can be written such that it has accurate unwind info. When not using .cfi_* directives, the inline asm could e.g. jump to a subsection, do call there with hand written .eh_frame section for it and then jump back. I guess Linux kernel doesn't care, for whatever reason they hate DWARF. Jakub
Re: [PATCH] inline-asm, i386: Add "redzone" clobber support
> Hi! > > The following patch adds a "redzone" clobber (recognized just > on targets which choose to recognize it, right now just x86), > with which one can mark the rare case where inline asm pushes > something on the stack or uses call instruction without taking > red zone into account (i.e. addq $-128, %rsp; and addq $128, %rsp > around that). > > 2024-11-06 Jakub Jelinek > > 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.target/i386/asm-redzone-1.c: New test. LGTM, though if asm needs temporary memory it can ask for it explicitly. I guess this can be practical to hide actual call in the asm statement (which will break unwinding I guess). Honza
