public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@orcam.me.uk>
To: Richard Sandiford <richard.sandiford@arm.com>
Cc: YunQiang Su <yunqiang.su@cipunited.com>,
	gcc-patches@gcc.gnu.org,  jiaxun.yang@flygoat.com,
	syq@debian.org
Subject: Re: [PATCH v2] MIPS: add speculation_barrier support
Date: Wed, 3 May 2023 22:04:46 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.2.21.2305031948370.26664@angie.orcam.me.uk> (raw)
In-Reply-To: <mpth6sttgj1.fsf@arm.com>

On Wed, 3 May 2023, Richard Sandiford wrote:

> > speculation_barrier for MIPS needs sync+jr.hb (r2+),
> > so we implement __speculation_barrier in libgcc, like arm32 does.
> 
> Looks reasonable, but do you have a source for the fallback
> pre-r2 handling?  (Thanks for adding that btw, since I realise
> it's not your focus here.)

 Seconded WRT legacy MIPS support, really appreciated.

 I think there may be no authoritative source of information here, this is 
a grey area.  The longest SSNOP sequences I have seen were for the various 
Broadcom implementations and counted 7 instructions.  Both the Linux 
kernel and the CFE firmware has them.

 Also we may not be able to fully enforce ordering for the oldest devices 
that do not implement SYNC, as this is system-specific, e.g. involving 
branching on the CP0 condition with the BC0F instruction, and inventing an 
OS interface for that seems unreasonable at this point of history.

 OTOH Linux emulates any traps on SYNC and even though the handler does 
nothing beyond decoding the instruction the exception handling overhead 
ought to make all the effects from before the exception invocation fully 
visible after its completion.  So for Linux targets I think we ought to 
just emit SYNC unconditionally; glibc has already relied on it for years 
now.

 Overall, oh well, I guess we'll have to live with the limitations.

> > diff --git a/libgcc/config/mips/lib1funcs.S b/libgcc/config/mips/lib1funcs.S
> > new file mode 100644
> > index 00000000000..45d74e2e762
> > --- /dev/null
> > +++ b/libgcc/config/mips/lib1funcs.S
[...]
> > +/* Make ssnop available, ssnop only recognized by GAS since mips32,
> > +   however it's actually available since R5500,
> > +   and it will be decoded as nop on earlier processors */
> > +	.set mips32

 I'm fairly sure `.set mips32' will break with NewABI compilations and may 
cause issues if a higher ISA level with some ASEs enabled has been chosen 
by the user building GCC.  But it's not needed either, as contrary to the 
comment SSNOP is universally accepted by GAS, as it should:

{"ssnop",		"",		0x00000040, 0xffffffff, 0,              	INSN2_ALIAS,	I1,		0,	0 }, /* sll */

Very old versions of GAS indeed required MIPS32+ for SSNOP, but the bug 
was fixed back in 2010, landing in binutils 2.21.  I wouldn't bother 
striving to support such old versions of binutils, i.e. either upgrade or 
patch locally (it might then be worth mentioning in `gcc/doc/install.texi' 
though).

 As to producing SYNC for MIPS I, glibc has this:

#if _MIPS_SIM == _ABIO32 && __mips < 2
#define MIPS_PUSH_MIPS2 ".set	mips2\n\t"
#else
#define MIPS_PUSH_MIPS2
#endif

for the ISA override, so I guess we could take a similar approach.  It 
should be safe, as it doesn't change the ISA unnecessarily.

  Maciej

  reply	other threads:[~2023-05-03 21:04 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-28 12:33 [PATCH] " YunQiang Su
2023-04-28 12:36 ` Jiaxun Yang
2023-04-28 13:07   ` YunQiang Su
2023-04-28 13:12 ` [PATCH v2] " YunQiang Su
2023-05-03 18:29   ` Richard Sandiford
2023-05-03 21:04     ` Maciej W. Rozycki [this message]
2023-05-03 22:12       ` Jiaxun Yang
2023-05-07 17:34         ` Maciej W. Rozycki
2023-05-07 18:47           ` Jiaxun Yang
2023-05-07 19:16             ` Maciej W. Rozycki
2023-05-12 10:03   ` [PATCH v3] " YunQiang Su
2023-05-12 10:30     ` [PATCH v4] " YunQiang Su
2023-05-31  9:43       ` YunQiang Su
2023-05-31 10:35         ` Maciej W. Rozycki
2023-06-01  4:26           ` [PATCH v5] MIPS: Add " YunQiang Su
2023-06-08 12:35             ` Richard Earnshaw (lists)
2023-06-16  7:53               ` YunQiang Su
2023-06-16  8:38                 ` Xi Ruoyao

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=alpine.DEB.2.21.2305031948370.26664@angie.orcam.me.uk \
    --to=macro@orcam.me.uk \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jiaxun.yang@flygoat.com \
    --cc=richard.sandiford@arm.com \
    --cc=syq@debian.org \
    --cc=yunqiang.su@cipunited.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).