public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Alexander Monakov <amonakov@ispras.ru>
Cc: Michael Matz <matz@suse.de>,
	gcc-patches@gcc.gnu.org, Jan Hubicka <hubicka@ucw.cz>
Subject: Re: [x86-64] RFC: Add nosse abi attribute
Date: Tue, 11 Jul 2023 15:00:54 +0200	[thread overview]
Message-ID: <CAFiYyc3b3p-FoBZADOsqjsMk+XxixPGcLy+o-i4Gbk=uEJpooQ@mail.gmail.com> (raw)
In-Reply-To: <a4cb9f8e-17f9-a9c1-6aaa-ff5999ce6f02@ispras.ru>

On Mon, Jul 10, 2023 at 9:08 PM Alexander Monakov via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> On Mon, 10 Jul 2023, Michael Matz via Gcc-patches wrote:
>
> > Hello,
> >
> > the ELF psABI for x86-64 doesn't have any callee-saved SSE
> > registers (there were actual reasons for that, but those don't
> > matter anymore).  This starts to hurt some uses, as it means that
> > as soon as you have a call (say to memmove/memcpy, even if
> > implicit as libcall) in a loop that manipulates floating point
> > or vector data you get saves/restores around those calls.
> >
> > But in reality many functions can be written such that they only need
> > to clobber a subset of the 16 XMM registers (or do the save/restore
> > themself in the codepaths that needs them, hello memcpy again).
> > So we want to introduce a way to specify this, via an ABI attribute
> > that basically says "doesn't clobber the high XMM regs".
>
> I think the main question is why you're going with this (weak) form
> instead of the (strong) form "may only clobber the low XMM regs":
> as Richi noted, surely for libcalls we'd like to know they preserve
> AVX-512 mask registers as well?
>
> (I realize this is partially answered later)
>
> Note this interacts with anything that interposes between the caller
> and the callee, like the Glibc lazy binding stub (which used to
> zero out high halves of 512-bit arguments in ZMM registers).
> Not an immediate problem for the patch, just something to mind perhaps.
>
> > I've opted to do only the obvious: do something special only for
> > xmm8 to xmm15, without a way to specify the clobber set in more detail.
> > I think such half/half split is reasonable, and as I don't want to
> > change the argument passing anyway (whose regs are always clobbered)
> > there isn't that much wiggle room anyway.
> >
> > I chose to make it possible to write function definitions with that
> > attribute with GCC adding the necessary callee save/restore code in
> > the xlogue itself.
>
> But you can't trivially restore if the callee is sibcalling — what
> happens then (a testcase might be nice)?
>
> > Carefully note that this is only possible for
> > the SSE2 registers, as other parts of them would need instructions
> > that are only optional.
>
> What is supposed to happen on 32-bit x86 with -msse -mno-sse2?
>
> > When a function doesn't contain calls to
> > unknown functions we can be a bit more lenient: we can make it so that
> > GCC simply doesn't touch xmm8-15 at all, then no save/restore is
> > necessary.
>
> What if the source code has a local register variable bound to xmm15,
> i.e. register double x asm("xmm15"); asm("..." : "+x"(x)); ?
> Probably "dont'd do that", i.e. disallow that in the documentation?
>
> > If a function contains calls then GCC can't know which
> > parts of the XMM regset is clobbered by that, it may be parts
> > which don't even exist yet (say until avx2048 comes out), so we must
> > restrict ourself to only save/restore the SSE2 parts and then of course
> > can only claim to not clobber those parts.
>
> Hm, I guess this is kinda the reason a "weak" form is needed. But this
> highlights the difference between the two: the "weak" form will actively
> preserve some state (so it cannot preserve future extensions), while
> the "strong" form may just passively not touch any state, preserving
> any state it doesn't know about.
>
> > To that end I introduce actually two related attributes (for naming
> > see below):
> > * nosseclobber: claims (and ensures) that xmm8-15 aren't clobbered
>
> This is the weak/active form; I'd suggest "preserve_high_sse".

Isn't it the opposite?  "preserves_low_sse", unless you suggest
the name applies to the caller which has to preserve high parts
when calling nosseclobber.

> > * noanysseclobber: claims (and ensures) that nothing of any of the
> >   registers overlapping xmm8-15 is clobbered (not even future, as of
> >   yet unknown, parts)
>
> This is the strong/passive form; I'd suggest "only_low_sse".

Likewise.

As for mask registers I understand we'd have to split the 8 register
set into two halves to make the same approach work, otherwise
we'd have no registers left to allocate from.

> > Ensuring the first is simple: potentially add saves/restore in xlogue
> > (e.g. when xmm8 is either used explicitely or implicitely by a call).
> > Ensuring the second comes with more: we must also ensure that no
> > functions are called that don't guarantee the same thing (in addition
> > to just removing all xmm8-15 parts alltogether from the available
> > regsters).
> >
> > See also the added testcases for what I intended to support.
> >
> > I chose to use the new target independend function-abi facility for
> > this.  I need some adjustments in generic code:
> > * the "default_abi" is actually more like a "current" abi: it happily
> >   changes its contents according to conditional_register_usage,
> >   and other code assumes that such changes do propagate.
> >   But if that conditonal_reg_usage is actually done because the current
> >   function is of a different ABI, then we must not change default_abi.
> > * in insn_callee_abi we do look at a potential fndecl for a call
> >   insn (only set when -fipa-ra), but doesn't work for calls through
> >   pointers and (as said) is optional.  So, also always look at the
> >   called functions type (it's always recorded in the MEM_EXPR for
> >   non-libcalls), before asking the target.
> >   (The function-abi accessors working on trees were already doing that,
> >   its just the RTL accessor that missed this)
> >
> > Accordingly I also implement some more target hooks for function-abi.
> > With that it's possible to also move the other ABI-influencing code
> > of i386 to function-abi (ms_abi and friends).  I have not done so for
> > this patch.
> >
> > Regarding the names of the attributes: gah!  I've left them at
> > my mediocre attempts of names in order to hopefully get input on better
> > names :-)
> >
> > I would welcome any comments, about the names, the approach, the attempt
> > at documenting the intricacies of these attributes and anything.
>
> I hope the new attributes are supposed to be usable with function pointers?
> From the code it looks that way, but the documentation doesn't promise that.
>
> > FWIW, this particular patch was regstrapped on x86-64-linux
> > with trunk from a week ago (and sniff-tested on current trunk).
>
> This looks really cool.

I think it's indeed nice for scalar code but since noanysseclobber
will be a pain
to manually use vector code is unlikely to benefit for modern archs.
There might
be the chance to auto-enable (maybe even more fine-grained) these with LTO,
but in the end 8 registers is not much and maybe we want to extend this to
noavxclobber and noavx512clobber, esp. considering glibc vectorized math
routines (maybe the OMP SIMD ABI could have been extended instead).

Otherwise after thinking about this more I don't see how to do better - I'm
still curious about actual use cases (for manually annotation, that is).

Richard.



> Thanks.
> Alexander

  parent reply	other threads:[~2023-07-11 13:01 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-10 15:55 Michael Matz
2023-07-10 17:28 ` Richard Biener
2023-07-10 19:07 ` Alexander Monakov
2023-07-10 20:33   ` Alexander Monakov
2023-07-11  6:42   ` Richard Biener
2023-07-11  8:53     ` Jan Hubicka
2023-07-11  9:07       ` Richard Biener
2023-07-11 13:00   ` Richard Biener [this message]
2023-07-11 13:21     ` Jan Hubicka
2023-07-11 14:00       ` Michael Matz
2023-07-11 13:59     ` Alexander Monakov
2023-07-11 14:57   ` Michael Matz
2023-07-11 15:17     ` Alexander Monakov
2023-07-11 15:34       ` Michael Matz
2023-07-11 16:53         ` Alexander Monakov
2023-07-17 23:00 ` Richard Sandiford

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='CAFiYyc3b3p-FoBZADOsqjsMk+XxixPGcLy+o-i4Gbk=uEJpooQ@mail.gmail.com' \
    --to=richard.guenther@gmail.com \
    --cc=amonakov@ispras.ru \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=matz@suse.de \
    /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).