public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED
@ 2024-01-10 16:28 hjl.tools at gmail dot com
  2024-01-11  1:39 ` [Bug target/113312] " hpa at zytor dot com
                   ` (26 more replies)
  0 siblings, 27 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-10 16:28 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

            Bug ID: 113312
           Summary: Update __attribute__((interrupt)) for Intel FRED
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hjl.tools at gmail dot com
                CC: crazylht at gmail dot com, hpa at zytor dot com
  Target Milestone: ---
            Target: x86-64

__attribute__((interrupt)) should be updated for Intel Flexible Return and
Event
Delivery (FRED):

1. FRED was never intended to be used without an assembly stub, as the
distinction
between ERETU and ERETS comes from the entry point chosen, and the FRED entry
points
are designed to be large enough to have an initial assembly stub to save and
restore
whatever registers the user wishes.
2. Event info layout on incoming stack is the same for all events.  Event type
is
encoded in the event info.

The FRED attribute:

1. Don't preserve ANY registers. Assembly stubs are responsible to save and
restore any registers if needed.
2. Use the normal RET to go to the assembly stub which will restore registers
and return with ERETU or ERETS as appropriate.
3. Assuming the assembly stubs save registers on incoming stack, the assembly
stub must set up RDI pointing to the event info on incoming stack.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
@ 2024-01-11  1:39 ` hpa at zytor dot com
  2024-01-11  1:48 ` hjl.tools at gmail dot com
                   ` (25 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-11  1:39 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #1 from H. Peter Anvin <hpa at zytor dot com> ---
This is actually a specific use case of the feature requested in bug 103503.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
  2024-01-11  1:39 ` [Bug target/113312] " hpa at zytor dot com
@ 2024-01-11  1:48 ` hjl.tools at gmail dot com
  2024-01-11  2:10 ` hpa at zytor dot com
                   ` (24 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-11  1:48 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-01-11
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H. Peter Anvin from comment #1)
> This is actually a specific use case of the feature requested in bug 103503.

This covers #1.  Should FRED handler take a pointer to the event info as the
function argument?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
  2024-01-11  1:39 ` [Bug target/113312] " hpa at zytor dot com
  2024-01-11  1:48 ` hjl.tools at gmail dot com
@ 2024-01-11  2:10 ` hpa at zytor dot com
  2024-01-11  2:10 ` hpa at zytor dot com
                   ` (23 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-11  2:10 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #3 from H. Peter Anvin <hpa at zytor dot com> ---
Created attachment 57032
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57032&action=edit
FRED assembly entry stub (example, slightly modified from the Linux kernel)

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (2 preceding siblings ...)
  2024-01-11  2:10 ` hpa at zytor dot com
@ 2024-01-11  2:10 ` hpa at zytor dot com
  2024-01-11  2:19 ` hjl.tools at gmail dot com
                   ` (22 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-11  2:10 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #4 from H. Peter Anvin <hpa at zytor dot com> ---
(In reply to H.J. Lu from comment #2)
> (In reply to H. Peter Anvin from comment #1)
> > This is actually a specific use case of the feature requested in bug 103503.
> 
> This covers #1.  Should FRED handler take a pointer to the event info as the
> function argument?

This can be passed in from the assembly stub (normally in %rdi).

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (3 preceding siblings ...)
  2024-01-11  2:10 ` hpa at zytor dot com
@ 2024-01-11  2:19 ` hjl.tools at gmail dot com
  2024-01-11  2:35 ` hpa at zytor dot com
                   ` (21 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-11  2:19 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H. Peter Anvin from comment #3)
> Created attachment 57032 [details]
> FRED assembly entry stub (example, slightly modified from the Linux kernel)

Can you do

asm_fred_entry_\type:
        endbr64
        push    %rdi
        push    %rsi
        push    %rdx
        push    %rax
        push    %r8
        push    %r9
        push    %r10
        push    %r11
        push    %rbx
        push    %rbp
        push    %r12
        push    %r13
        push    %r14
        push    %r15
        lea     15*8(%rsp),%rdi <<<<<< RDI will point to the event info after
CALL.
        call    fred_entry_\type        /* call C code */
        endbr64
        pop     %r15
        pop     %r14
        pop     %r13
        pop     %r12
        pop     %rbp
        pop     %rbx
        pop     %r11
        pop     %r10
        pop     %r9
        pop     %r8
        pop     %rax
        pop     %rdx
        pop     %rsi
        pop     %rdi
        \eret

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (4 preceding siblings ...)
  2024-01-11  2:19 ` hjl.tools at gmail dot com
@ 2024-01-11  2:35 ` hpa at zytor dot com
  2024-01-11  4:09 ` hjl.tools at gmail dot com
                   ` (20 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-11  2:35 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #6 from H. Peter Anvin <hpa at zytor dot com> ---
Of course. That's not what we want in the Linux kernel specifically, though.
It's really up to the OS.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (5 preceding siblings ...)
  2024-01-11  2:35 ` hpa at zytor dot com
@ 2024-01-11  4:09 ` hjl.tools at gmail dot com
  2024-01-11  4:10 ` hjl.tools at gmail dot com
                   ` (19 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-11  4:09 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H. Peter Anvin from comment #6)
> Of course. That's not what we want in the Linux kernel specifically, though.
> It's really up to the OS.

no_callee_saved_registers attribute is sufficient.  One can do

void *
__attribute__((no_caller_saved_registers))
fred_handler (struct fred_event_info_and_called_saved_regs *arg)
{
  ....
}

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (6 preceding siblings ...)
  2024-01-11  4:09 ` hjl.tools at gmail dot com
@ 2024-01-11  4:10 ` hjl.tools at gmail dot com
  2024-01-11  4:11 ` hjl.tools at gmail dot com
                   ` (18 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-11  4:10 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H.J. Lu from comment #7)
> (In reply to H. Peter Anvin from comment #6)
> > Of course. That's not what we want in the Linux kernel specifically, though.
> > It's really up to the OS.
> 
> no_callee_saved_registers attribute is sufficient.  One can do
>

void *
__attribute__((no_callee_saved_registers))
fred_handler (struct fred_event_info_and_caller_saved_regs *arg)
{
  ...
}

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (7 preceding siblings ...)
  2024-01-11  4:10 ` hjl.tools at gmail dot com
@ 2024-01-11  4:11 ` hjl.tools at gmail dot com
  2024-01-11  5:29 ` hpa at zytor dot com
                   ` (17 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-11  4:11 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H.J. Lu from comment #8)
> (In reply to H.J. Lu from comment #7)
> > (In reply to H. Peter Anvin from comment #6)
> > > Of course. That's not what we want in the Linux kernel specifically, though.
> > > It's really up to the OS.
> > 
> > no_callee_saved_registers attribute is sufficient.  One can do
> >
> 

void
__attribute__((no_callee_saved_registers))
fred_handler (struct fred_event_info_and_caller_saved_regs *arg)
{
  ...
}

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (8 preceding siblings ...)
  2024-01-11  4:11 ` hjl.tools at gmail dot com
@ 2024-01-11  5:29 ` hpa at zytor dot com
  2024-01-11  5:38 ` liuhongt at gcc dot gnu.org
                   ` (16 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-11  5:29 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #10 from H. Peter Anvin <hpa at zytor dot com> ---
Right, is there such an attribute (that's what I'm asking for in bug 103503)?

All I see in the gcc documentation is no_calle*R*_saved_registers, which,
again, is the exact opposite.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (9 preceding siblings ...)
  2024-01-11  5:29 ` hpa at zytor dot com
@ 2024-01-11  5:38 ` liuhongt at gcc dot gnu.org
  2024-01-11 15:47 ` fw at gcc dot gnu.org
                   ` (15 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: liuhongt at gcc dot gnu.org @ 2024-01-11  5:38 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

Hongtao Liu <liuhongt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |liuhongt at gcc dot gnu.org

--- Comment #11 from Hongtao Liu <liuhongt at gcc dot gnu.org> ---
(In reply to H. Peter Anvin from comment #10)
> Right, is there such an attribute (that's what I'm asking for in bug 103503)?
No, I'll try to support that in GCC15.
> 
> All I see in the gcc documentation is no_calle*R*_saved_registers, which,
> again, is the exact opposite.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (10 preceding siblings ...)
  2024-01-11  5:38 ` liuhongt at gcc dot gnu.org
@ 2024-01-11 15:47 ` fw at gcc dot gnu.org
  2024-01-11 15:55 ` hpa at zytor dot com
                   ` (14 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: fw at gcc dot gnu.org @ 2024-01-11 15:47 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

Florian Weimer <fw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fw at gcc dot gnu.org

--- Comment #12 from Florian Weimer <fw at gcc dot gnu.org> ---
Can you make fred_handler noreturn and use inline assembler to do the exit?
Will FRED restore the processor state in this case?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (11 preceding siblings ...)
  2024-01-11 15:47 ` fw at gcc dot gnu.org
@ 2024-01-11 15:55 ` hpa at zytor dot com
  2024-01-12  1:00 ` hjl.tools at gmail dot com
                   ` (13 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-11 15:55 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #13 from H. Peter Anvin <hpa at zytor dot com> ---
No, it will not. Most OSes flows will want to merge the kernel and user flows
at some point for some handlers, so it isn't clear that that makes sense.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (12 preceding siblings ...)
  2024-01-11 15:55 ` hpa at zytor dot com
@ 2024-01-12  1:00 ` hjl.tools at gmail dot com
  2024-01-12  1:25 ` hpa at zytor dot com
                   ` (12 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-12  1:00 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #14 from H.J. Lu <hjl.tools at gmail dot com> ---
Here is a branch for __attribute__((no_callee_saved_registers)):

https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/pr113312/master

Calling a function with __attribute__((no_callee_saved_registers))
doesn't work since GCC won't restore clobbered caller-saved registers.
Also calling a function with __attribute__((no_callee_saved_registers))
via a function pointer won't work correctly.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (13 preceding siblings ...)
  2024-01-12  1:00 ` hjl.tools at gmail dot com
@ 2024-01-12  1:25 ` hpa at zytor dot com
  2024-01-12  4:31 ` hjl.tools at gmail dot com
                   ` (11 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-12  1:25 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #15 from H. Peter Anvin <hpa at zytor dot com> ---
That should be fine for this use case, obviously.

I should add the following: the reason the assembly stub isn't a problem for
FRED whereas it is a bit of a nuisance for IDT-style delivery is that with
FRED, vector dispatch is done in software, not hardware. This is exactly
because *most* operating systems do need some amount of common entry/exit code
anyway, and having to duplicate it is a severe nuisance.

In the specific case of Linux, the full register set, including saved
registers, are a required part of the exception frame in order for things like
ptrace() and fork() to work correctly, so relying on the compiler to save the
"saved" registers doesn't work for us anyway.

So since there is only *one* instance of the assembly stub needed, it means
there isn't a whole separate stub needed for every handler.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (14 preceding siblings ...)
  2024-01-12  1:25 ` hpa at zytor dot com
@ 2024-01-12  4:31 ` hjl.tools at gmail dot com
  2024-01-13  5:03 ` hjl.tools at gmail dot com
                   ` (10 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-12  4:31 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #16 from H.J. Lu <hjl.tools at gmail dot com> ---
I updated users/hjl/pr113312/master branch to handle function pointers.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (15 preceding siblings ...)
  2024-01-12  4:31 ` hjl.tools at gmail dot com
@ 2024-01-13  5:03 ` hjl.tools at gmail dot com
  2024-01-13 18:25 ` hjl.tools at gmail dot com
                   ` (9 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-13  5:03 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #17 from H.J. Lu <hjl.tools at gmail dot com> ---
Please try users/hjl/pr113312/gcc-13 branch:

https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/pr113312/gcc-13?ref_type=heads

It supports no_callee_saved_registers attribute. It should also avoid saving
registers in noreturn functions in Linux kernel.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (16 preceding siblings ...)
  2024-01-13  5:03 ` hjl.tools at gmail dot com
@ 2024-01-13 18:25 ` hjl.tools at gmail dot com
  2024-01-13 20:02 ` hpa at zytor dot com
                   ` (8 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-13 18:25 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #18 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H.J. Lu from comment #17)
> Please try users/hjl/pr113312/gcc-13 branch:
> 
> https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/pr113312/gcc-
> 13?ref_type=heads
> 
> It supports no_callee_saved_registers attribute. It should also avoid saving
> registers in noreturn functions in Linux kernel.

I updated users/hjl/pr113312/gcc-13 branch. It works on glibc. Please try it
on Linux kernel.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (17 preceding siblings ...)
  2024-01-13 18:25 ` hjl.tools at gmail dot com
@ 2024-01-13 20:02 ` hpa at zytor dot com
  2024-01-13 20:22 ` hjl.tools at gmail dot com
                   ` (7 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-13 20:02 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #19 from H. Peter Anvin <hpa at zytor dot com> ---
I'm away for the long weekend, but I'll try it out on Tuesday.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (18 preceding siblings ...)
  2024-01-13 20:02 ` hpa at zytor dot com
@ 2024-01-13 20:22 ` hjl.tools at gmail dot com
  2024-01-13 20:51 ` hpa at zytor dot com
                   ` (6 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-13 20:22 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #20 from H.J. Lu <hjl.tools at gmail dot com> ---
In Linux kernel 6.7.0 on x86-64, do_exit is changed from

do_exit:
        endbr64
        call   <do_exit+0x9>
        push   %r15
        push   %r14
        push   %r13
        push   %r12
        mov    %rdi,%r12
        push   %rbp
        push   %rbx
        mov    %gs:0x0,%rbx
        sub    $0x28,%rsp
        mov    %gs:0x28,%rax
        mov    %rax,0x20(%rsp)
        xor    %eax,%eax
        call   *0x0(%rip)        # <do_exit+0x39>
        test   $0x2,%ah
        je     <do_exit+0x8d3>

to

do_exit:
        endbr64
        call   <do_exit+0x9>
        sub    $0x28,%rsp
        mov    %rdi,%r12
        mov    %gs:0x28,%rax
        mov    %rax,0x20(%rsp)
        xor    %eax,%eax
        mov    %gs:0x0,%rbx
        call   *0x0(%rip)        # <do_exit+0x2f>
        test   $0x2,%ah
        je     <do_exit+0x8c9>

Kernel seems to work fine.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Update __attribute__((interrupt)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (19 preceding siblings ...)
  2024-01-13 20:22 ` hjl.tools at gmail dot com
@ 2024-01-13 20:51 ` hpa at zytor dot com
  2024-01-18  6:42 ` [Bug target/113312] Add __attribute__((no_callee_saved_registers)) " xin at zytor dot com
                   ` (5 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hpa at zytor dot com @ 2024-01-13 20:51 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #21 from H. Peter Anvin <hpa at zytor dot com> ---
I think this could be a really useful performance improvement in general. The
Linux exception and syscall paths have a fair number of tail calls on the
primary path, and this would make it possible to avoid the register save and
restores for each of the functions in the tail called path.

I have asked Xin Li (FRED maintainer) to try this out when he has the
opportunity, although right now the Linux kernel merge window is open and so
that is necessarily his first priority.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Add __attribute__((no_callee_saved_registers)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (20 preceding siblings ...)
  2024-01-13 20:51 ` hpa at zytor dot com
@ 2024-01-18  6:42 ` xin at zytor dot com
  2024-01-18 13:01 ` hjl.tools at gmail dot com
                   ` (4 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: xin at zytor dot com @ 2024-01-18  6:42 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

Xin Li <xin at zytor dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xin at zytor dot com

--- Comment #22 from Xin Li <xin at zytor dot com> ---
Per Peter's suggestion, I added __attribute__((no_callee_saved_registers)) to a
linux source tree containing FRED patches:
https://github.com/xinli-intel/linux-fred-public/commit/12c38143a5c33e89f2b3d8906629dd4f23f8d79c.
 And I compiled the linux code with a gcc built from
https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/pr113312/gcc-13.

Following are my observations:
1) the generated kernel boots fine on both FRED Simics model and bare metal.
2) the asm code generated for fred_entry_from_{user,kernel}() are the same,
i.e., __attribute__((no_callee_saved_registers)) makes no difference (Peter
said the FRED dispatch points simply do not have significant register pressure
– intentionally).
3) other functions with __attribute__((no_callee_saved_registers)) do get rid
of pushing/popping clobbered registers, and cause no issues because they are
top-level functions, only invoked by tail call, meaning the following case
won't happen:

<func>:
...
mov (%rbx), %r13
call foo
mov %rax, (%r13)
...

otherwise foo() is NOT a top-level function.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Add __attribute__((no_callee_saved_registers)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (21 preceding siblings ...)
  2024-01-18  6:42 ` [Bug target/113312] Add __attribute__((no_callee_saved_registers)) " xin at zytor dot com
@ 2024-01-18 13:01 ` hjl.tools at gmail dot com
  2024-01-19  6:23 ` xin at zytor dot com
                   ` (3 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-18 13:01 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #23 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Xin Li from comment #22)
> Per Peter's suggestion, I added __attribute__((no_callee_saved_registers))
> to a linux source tree containing FRED patches:
> https://github.com/xinli-intel/linux-fred-public/commit/
> 12c38143a5c33e89f2b3d8906629dd4f23f8d79c.  And I compiled the linux code
> with a gcc built from
> https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/pr113312/gcc-13.
> 
> Following are my observations:
> 1) the generated kernel boots fine on both FRED Simics model and bare metal.
> 2) the asm code generated for fred_entry_from_{user,kernel}() are the same,
> i.e., __attribute__((no_callee_saved_registers)) makes no difference (Peter
> said the FRED dispatch points simply do not have significant register
> pressure – intentionally).
> 3) other functions with __attribute__((no_callee_saved_registers)) do get
> rid of pushing/popping clobbered registers, and cause no issues because they
> are top-level functions, only invoked by tail call, meaning the following
> case won't happen:
> 
> <func>:
> ...
> mov (%rbx), %r13
> call foo
> mov %rax, (%r13)
> ...
> 
> otherwise foo() is NOT a top-level function.

Do these mean that __attribute__((no_callee_saved_registers)) works for you?
BTW, my patches also avoid saving callee-saved registers in
__attribute__((noreturn))
functions.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Add __attribute__((no_callee_saved_registers)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (22 preceding siblings ...)
  2024-01-18 13:01 ` hjl.tools at gmail dot com
@ 2024-01-19  6:23 ` xin at zytor dot com
  2024-01-19  6:25 ` xin at zytor dot com
                   ` (2 subsequent siblings)
  26 siblings, 0 replies; 28+ messages in thread
From: xin at zytor dot com @ 2024-01-19  6:23 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #24 from Xin Li <xin at zytor dot com> ---
(In reply to H.J. Lu from comment #23)
> (In reply to Xin Li from comment #22)
> > Per Peter's suggestion, I added __attribute__((no_callee_saved_registers))
> > to a linux source tree containing FRED patches:
> > https://github.com/xinli-intel/linux-fred-public/commit/
> > 12c38143a5c33e89f2b3d8906629dd4f23f8d79c.  And I compiled the linux code
> > with a gcc built from
> > https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/pr113312/gcc-13.
> > 
> > Following are my observations:
> > 1) the generated kernel boots fine on both FRED Simics model and bare metal.
> > 2) the asm code generated for fred_entry_from_{user,kernel}() are the same,
> > i.e., __attribute__((no_callee_saved_registers)) makes no difference (Peter
> > said the FRED dispatch points simply do not have significant register
> > pressure – intentionally).
> > 3) other functions with __attribute__((no_callee_saved_registers)) do get
> > rid of pushing/popping clobbered registers, and cause no issues because they
> > are top-level functions, only invoked by tail call, meaning the following
> > case won't happen:
> > 
> > <func>:
> > ...
> > mov (%rbx), %r13
> > call foo
> > mov %rax, (%r13)
> > ...
> > 
> > otherwise foo() is NOT a top-level function.
> 
> Do these mean that __attribute__((no_callee_saved_registers)) works for you?

Yes, it works as expected.

> BTW, my patches also avoid saving callee-saved registers in
> __attribute__((noreturn))
> functions.

I also checked do_exit, which is the same as what you posted.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Add __attribute__((no_callee_saved_registers)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (23 preceding siblings ...)
  2024-01-19  6:23 ` xin at zytor dot com
@ 2024-01-19  6:25 ` xin at zytor dot com
  2024-01-19 13:40 ` hjl.tools at gmail dot com
  2024-01-27 12:19 ` cvs-commit at gcc dot gnu.org
  26 siblings, 0 replies; 28+ messages in thread
From: xin at zytor dot com @ 2024-01-19  6:25 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #25 from Xin Li <xin at zytor dot com> ---
(In reply to Xin Li from comment #22)
> Per Peter's suggestion, I added __attribute__((no_callee_saved_registers))
> to a linux source tree containing FRED patches:
> https://github.com/xinli-intel/linux-fred-public/commit/
> 12c38143a5c33e89f2b3d8906629dd4f23f8d79c

It's better to refer to branch 'gcc-no_callee_saved_registers' instead:

https://github.com/xinli-intel/linux-fred-public/commits/gcc-no_callee_saved_registers/

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Add __attribute__((no_callee_saved_registers)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (24 preceding siblings ...)
  2024-01-19  6:25 ` xin at zytor dot com
@ 2024-01-19 13:40 ` hjl.tools at gmail dot com
  2024-01-27 12:19 ` cvs-commit at gcc dot gnu.org
  26 siblings, 0 replies; 28+ messages in thread
From: hjl.tools at gmail dot com @ 2024-01-19 13:40 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #26 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Xin Li from comment #25)
> (In reply to Xin Li from comment #22)
> > Per Peter's suggestion, I added __attribute__((no_callee_saved_registers))
> > to a linux source tree containing FRED patches:
> > https://github.com/xinli-intel/linux-fred-public/commit/
> > 12c38143a5c33e89f2b3d8906629dd4f23f8d79c
> 
> It's better to refer to branch 'gcc-no_callee_saved_registers' instead:
> 
> https://github.com/xinli-intel/linux-fred-public/commits/gcc-
> no_callee_saved_registers/

I commented on your commit to add __attribute__((no_callee_saved_registers)).

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Bug target/113312] Add __attribute__((no_callee_saved_registers)) for Intel FRED
  2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
                   ` (25 preceding siblings ...)
  2024-01-19 13:40 ` hjl.tools at gmail dot com
@ 2024-01-27 12:19 ` cvs-commit at gcc dot gnu.org
  26 siblings, 0 replies; 28+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-01-27 12:19 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113312

--- Comment #27 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:a96549dce7636edfc693bf758ef27fcd8adc6161

commit r14-8469-ga96549dce7636edfc693bf758ef27fcd8adc6161
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Jan 23 06:59:50 2024 -0800

    x86: Add no_callee_saved_registers function attribute

    When an interrupt handler is implemented by an assembly stub which does:

    1. Save all registers.
    2. Call a C function.
    3. Restore all registers.
    4. Return from interrupt.

    it is completely unnecessary to save and restore any registers in the C
    function called by the assembly stub, even if they would normally be
    callee-saved.

    Add no_callee_saved_registers function attribute, which is complementary
    to no_caller_saved_registers function attribute, to mark a function which
    doesn't have any callee-saved registers.  Such a function won't save and
    restore any registers.  Classify function call-saved register handling
    type with:

    1. Default call-saved registers.
    2. No caller-saved registers with no_caller_saved_registers attribute.
    3. No callee-saved registers with no_callee_saved_registers attribute.

    Disallow sibcall if callee is a no_callee_saved_registers function
    and caller isn't a no_callee_saved_registers function.  Otherwise,
    callee-saved registers won't be preserved.

    After a no_callee_saved_registers function is called, all registers may
    be clobbered.  If the calling function isn't a no_callee_saved_registers
    function, we need to preserve all registers which aren't used by function
    calls.

    gcc/

            PR target/103503
            PR target/113312
            * config/i386/i386-expand.cc (ix86_expand_call): Replace
            no_caller_saved_registers check with call_saved_registers check.
            Clobber all registers that are not used by the callee with
            no_callee_saved_registers attribute.
            * config/i386/i386-options.cc (ix86_set_func_type): Set
            call_saved_registers to TYPE_NO_CALLEE_SAVED_REGISTERS for
            noreturn function.  Disallow no_callee_saved_registers with
            interrupt or no_caller_saved_registers attributes together.
            (ix86_set_current_function): Replace no_caller_saved_registers
            check with call_saved_registers check.
            (ix86_handle_no_caller_saved_registers_attribute): Renamed to ...
            (ix86_handle_call_saved_registers_attribute): This.
            (ix86_gnu_attributes): Add
            ix86_handle_call_saved_registers_attribute.
            * config/i386/i386.cc (ix86_conditional_register_usage): Replace
            no_caller_saved_registers check with call_saved_registers check.
            (ix86_function_ok_for_sibcall): Don't allow callee with
            no_callee_saved_registers attribute when the calling function
            has callee-saved registers.
            (ix86_comp_type_attributes): Also check
            no_callee_saved_registers.
            (ix86_epilogue_uses): Replace no_caller_saved_registers check
            with call_saved_registers check.
            (ix86_hard_regno_scratch_ok): Likewise.
            (ix86_save_reg): Replace no_caller_saved_registers check with
            call_saved_registers check.  Don't save any registers for
            TYPE_NO_CALLEE_SAVED_REGISTERS.  Save all registers with
            TYPE_DEFAULT_CALL_SAVED_REGISTERS if function with
            no_callee_saved_registers attribute is called.
            (find_drap_reg): Replace no_caller_saved_registers check with
            call_saved_registers check.
            * config/i386/i386.h (call_saved_registers_type): New enum.
            (machine_function): Replace no_caller_saved_registers with
            call_saved_registers.
            * doc/extend.texi: Document no_callee_saved_registers attribute.

    gcc/testsuite/

            PR target/103503
            PR target/113312
            * gcc.dg/torture/no-callee-saved-run-1a.c: New file.
            * gcc.dg/torture/no-callee-saved-run-1b.c: Likewise.
            * gcc.target/i386/no-callee-saved-1.c: Likewise.
            * gcc.target/i386/no-callee-saved-2.c: Likewise.
            * gcc.target/i386/no-callee-saved-3.c: Likewise.
            * gcc.target/i386/no-callee-saved-4.c: Likewise.
            * gcc.target/i386/no-callee-saved-5.c: Likewise.
            * gcc.target/i386/no-callee-saved-6.c: Likewise.
            * gcc.target/i386/no-callee-saved-7.c: Likewise.
            * gcc.target/i386/no-callee-saved-8.c: Likewise.
            * gcc.target/i386/no-callee-saved-9.c: Likewise.
            * gcc.target/i386/no-callee-saved-10.c: Likewise.
            * gcc.target/i386/no-callee-saved-11.c: Likewise.
            * gcc.target/i386/no-callee-saved-12.c: Likewise.
            * gcc.target/i386/no-callee-saved-13.c: Likewise.
            * gcc.target/i386/no-callee-saved-14.c: Likewise.
            * gcc.target/i386/no-callee-saved-15.c: Likewise.
            * gcc.target/i386/no-callee-saved-16.c: Likewise.
            * gcc.target/i386/no-callee-saved-17.c: Likewise.
            * gcc.target/i386/no-callee-saved-18.c: Likewise.

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2024-01-27 12:19 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-10 16:28 [Bug target/113312] New: Update __attribute__((interrupt)) for Intel FRED hjl.tools at gmail dot com
2024-01-11  1:39 ` [Bug target/113312] " hpa at zytor dot com
2024-01-11  1:48 ` hjl.tools at gmail dot com
2024-01-11  2:10 ` hpa at zytor dot com
2024-01-11  2:10 ` hpa at zytor dot com
2024-01-11  2:19 ` hjl.tools at gmail dot com
2024-01-11  2:35 ` hpa at zytor dot com
2024-01-11  4:09 ` hjl.tools at gmail dot com
2024-01-11  4:10 ` hjl.tools at gmail dot com
2024-01-11  4:11 ` hjl.tools at gmail dot com
2024-01-11  5:29 ` hpa at zytor dot com
2024-01-11  5:38 ` liuhongt at gcc dot gnu.org
2024-01-11 15:47 ` fw at gcc dot gnu.org
2024-01-11 15:55 ` hpa at zytor dot com
2024-01-12  1:00 ` hjl.tools at gmail dot com
2024-01-12  1:25 ` hpa at zytor dot com
2024-01-12  4:31 ` hjl.tools at gmail dot com
2024-01-13  5:03 ` hjl.tools at gmail dot com
2024-01-13 18:25 ` hjl.tools at gmail dot com
2024-01-13 20:02 ` hpa at zytor dot com
2024-01-13 20:22 ` hjl.tools at gmail dot com
2024-01-13 20:51 ` hpa at zytor dot com
2024-01-18  6:42 ` [Bug target/113312] Add __attribute__((no_callee_saved_registers)) " xin at zytor dot com
2024-01-18 13:01 ` hjl.tools at gmail dot com
2024-01-19  6:23 ` xin at zytor dot com
2024-01-19  6:25 ` xin at zytor dot com
2024-01-19 13:40 ` hjl.tools at gmail dot com
2024-01-27 12:19 ` cvs-commit at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).