From: "H.J. Lu" <hjl.tools@gmail.com>
To: Hongtao Liu <crazylht@gmail.com>
Cc: gcc-patches@gcc.gnu.org, ubizjak@gmail.com,
hongtao.liu@intel.com, jh@suse.cz
Subject: Re: [PATCH 1/2] x86: Add no_callee_saved_registers function attribute
Date: Mon, 22 Jan 2024 07:47:42 -0800 [thread overview]
Message-ID: <CAMe9rOq-33QasuX5XswHx0jmf2+8ojBpaNMusKbAcBmdK=6PDQ@mail.gmail.com> (raw)
In-Reply-To: <CAMZc-byDn8A5--CZa=V-fS5KzPdCbHO6BYaW2Z8CLJOO08-WLw@mail.gmail.com>
On Sun, Jan 21, 2024 at 8:03 PM Hongtao Liu <crazylht@gmail.com> wrote:
>
> On Sat, Jan 20, 2024 at 10:30 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > 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): Set
> > call_no_callee_saved_registers to true when calling function
> > with no_callee_saved_registers attribute. Replace
> > no_caller_saved_registers check with call_saved_registers check.
> > * 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. Add call_no_callee_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.
> > ---
> > gcc/config/i386/i386-expand.cc | 72 ++++++++++++++++---
> > gcc/config/i386/i386-options.cc | 49 +++++++++----
> > gcc/config/i386/i386.cc | 70 ++++++++++++++----
> > gcc/config/i386/i386.h | 20 +++++-
> > gcc/doc/extend.texi | 8 +++
> > .../gcc.dg/torture/no-callee-saved-run-1a.c | 23 ++++++
> > .../gcc.dg/torture/no-callee-saved-run-1b.c | 59 +++++++++++++++
> > .../gcc.target/i386/no-callee-saved-1.c | 30 ++++++++
> > .../gcc.target/i386/no-callee-saved-10.c | 46 ++++++++++++
> > .../gcc.target/i386/no-callee-saved-11.c | 11 +++
> > .../gcc.target/i386/no-callee-saved-12.c | 10 +++
> > .../gcc.target/i386/no-callee-saved-13.c | 16 +++++
> > .../gcc.target/i386/no-callee-saved-14.c | 16 +++++
> > .../gcc.target/i386/no-callee-saved-15.c | 17 +++++
> > .../gcc.target/i386/no-callee-saved-16.c | 16 +++++
> > .../gcc.target/i386/no-callee-saved-17.c | 16 +++++
> > .../gcc.target/i386/no-callee-saved-18.c | 51 +++++++++++++
> > .../gcc.target/i386/no-callee-saved-2.c | 30 ++++++++
> > .../gcc.target/i386/no-callee-saved-3.c | 8 +++
> > .../gcc.target/i386/no-callee-saved-4.c | 8 +++
> > .../gcc.target/i386/no-callee-saved-5.c | 11 +++
> > .../gcc.target/i386/no-callee-saved-6.c | 12 ++++
> > .../gcc.target/i386/no-callee-saved-7.c | 49 +++++++++++++
> > .../gcc.target/i386/no-callee-saved-8.c | 50 +++++++++++++
> > .../gcc.target/i386/no-callee-saved-9.c | 49 +++++++++++++
> > 25 files changed, 709 insertions(+), 38 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.dg/torture/no-callee-saved-run-1a.c
> > create mode 100644 gcc/testsuite/gcc.dg/torture/no-callee-saved-run-1b.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-1.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-10.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-11.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-12.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-13.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-14.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-15.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-16.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-17.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-18.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-2.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-3.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-4.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-5.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-6.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-7.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-8.c
> > create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-9.c
> >
> > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
> > index 52754e114f4..c0c7c697440 100644
> > --- a/gcc/config/i386/i386-expand.cc
> > +++ b/gcc/config/i386/i386-expand.cc
> > @@ -9739,17 +9739,41 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
> > rtx use = NULL, call;
> > unsigned int vec_len = 0.
> > tree fndecl;
> > + bool call_no_callee_saved_registers = false;
> >
> > if (GET_CODE (XEXP (fnaddr, 0)) == SYMBOL_REF)
> > {
> > fndecl = SYMBOL_REF_DECL (XEXP (fnaddr, 0));
> > - if (fndecl
> > - && (lookup_attribute ("interrupt",
> > - TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))))
> > - error ("interrupt service routine cannot be called directly");
> > + if (fndecl)
> > + {
> > + if (lookup_attribute ("interrupt",
> > + TYPE_ATTRIBUTES (TREE_TYPE (fndecl))))
> > + error ("interrupt service routine cannot be called directly");
> > + else if (lookup_attribute ("no_callee_saved_registers",
> > + TYPE_ATTRIBUTES (TREE_TYPE (fndecl))))
> > + {
> > + cfun->machine->call_no_callee_saved_registers = true;
> > + call_no_callee_saved_registers = true;
> > + }
> > + }
> > }
> > else
> > - fndecl = NULL_TREE;
> > + {
> > + if (MEM_P (fnaddr))
> > + {
> > + tree mem_expr = MEM_EXPR (fnaddr);
> > + if (mem_expr != nullptr
> > + && TREE_CODE (mem_expr) == MEM_REF
> > + && lookup_attribute ("no_callee_saved_registers",
> > + TYPE_ATTRIBUTES (TREE_TYPE (mem_expr))))
> > + {
> > + cfun->machine->call_no_callee_saved_registers = true;
> > + call_no_callee_saved_registers = true;
> > + }
> > + }
> > +
> > + fndecl = NULL_TREE;
> > + }
> >
> > if (pop == const0_rtx)
> > pop = NULL;
> > @@ -9884,13 +9908,18 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
> > vec[vec_len++] = pop;
> > }
> >
> > - if (cfun->machine->no_caller_saved_registers
> > + static const char ix86_call_used_regs[] = CALL_USED_REGISTERS;
> > +
> > + char clobbered_registers[FIRST_PSEUDO_REGISTER];
> > + memset (clobbered_registers, 0, sizeof (clobbered_registers));
> > +
> > + if ((cfun->machine->call_saved_registers
> > + == TYPE_NO_CALLER_SAVED_REGISTERS)
> > && (!fndecl
> > || (!TREE_THIS_VOLATILE (fndecl)
> > && !lookup_attribute ("no_caller_saved_registers",
> > TYPE_ATTRIBUTES (TREE_TYPE (fndecl))))))
> > {
> > - static const char ix86_call_used_regs[] = CALL_USED_REGISTERS;
> > bool is_64bit_ms_abi = (TARGET_64BIT
> > && ix86_function_abi (fndecl) == MS_ABI);
> > char c_mask = CALL_USED_REGISTERS_MASK (is_64bit_ms_abi);
> > @@ -9903,8 +9932,11 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
> > || (ix86_call_used_regs[i] & c_mask))
> > && !STACK_REGNO_P (i)
> > && !MMX_REGNO_P (i))
> > - clobber_reg (&use,
> > - gen_rtx_REG (GET_MODE (regno_reg_rtx[i]), i));
> > + {
> > + clobber_reg (&use,
> > + gen_rtx_REG (GET_MODE (regno_reg_rtx[i]), i));
> > + clobbered_registers[i] = 1;
> > + }
> > }
> > else if (TARGET_64BIT_MS_ABI
> > && (!callarg2 || INTVAL (callarg2) != -2))
> > @@ -9917,6 +9949,7 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
> > machine_mode mode = SSE_REGNO_P (regno) ? TImode : DImode;
> >
> > clobber_reg (&use, gen_rtx_REG (mode, regno));
> > + clobbered_registers[i] = 1;
> > }
> >
> > /* Set here, but it may get cleared later. */
> > @@ -9953,6 +9986,27 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
> > resolver could be used which clobbers R11 and R10. */
> > clobber_reg (&use, gen_rtx_REG (DImode, R11_REG));
> > clobber_reg (&use, gen_rtx_REG (DImode, R10_REG));
> > + clobbered_registers[R11_REG] = 1;
> > + clobbered_registers[R10_REG] = 1;
> > + }
> > +
> > + if (call_no_callee_saved_registers)
> > + {
> > + /* After calling a no_callee_saved_registers function, all
> > + registers may be clobbered. Clobber all registers that are
> > + not clobbered yet and not used by the callee. */
> > + bool is_64bit_ms_abi = (TARGET_64BIT
> > + && ix86_function_abi (fndecl) == MS_ABI);
> > + char c_mask = CALL_USED_REGISTERS_MASK (is_64bit_ms_abi);
> > + for (int i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> > + if (!fixed_regs[i]
> > + && !clobbered_registers[i]
> It seems to me clobbered_registers is only used here which seems
> redundant, remove !clobbered_registers[i] should also be fine?
>
You are right. Here is the v2 patch set:
https://patchwork.sourceware.org/project/gcc/list/?series=30050
Changes in v2:
1. Rebase against commit f9df00340e3
2. Don't add redundant clobbered_registers check in ix86_expand_call.
Thanks.
--
H.J.
next prev parent reply other threads:[~2024-01-22 15:48 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-20 14:27 [PATCH 0/2] x86: Don't save callee-saved registers if not needed H.J. Lu
2024-01-20 14:27 ` [PATCH 1/2] x86: Add no_callee_saved_registers function attribute H.J. Lu
2024-01-22 4:03 ` Hongtao Liu
2024-01-22 15:47 ` H.J. Lu [this message]
2024-01-20 14:27 ` [PATCH 2/2] x86: Don't save callee-saved registers in noreturn functions H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMe9rOq-33QasuX5XswHx0jmf2+8ojBpaNMusKbAcBmdK=6PDQ@mail.gmail.com' \
--to=hjl.tools@gmail.com \
--cc=crazylht@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hongtao.liu@intel.com \
--cc=jh@suse.cz \
--cc=ubizjak@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).