From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from angie.orcam.me.uk (angie.orcam.me.uk [78.133.224.34]) by sourceware.org (Postfix) with ESMTP id 441BE3858D28 for ; Wed, 3 May 2023 21:04:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 441BE3858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orcam.me.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orcam.me.uk Received: by angie.orcam.me.uk (Postfix, from userid 500) id 098D592009C; Wed, 3 May 2023 23:04:47 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 0358292009B; Wed, 3 May 2023 22:04:46 +0100 (BST) Date: Wed, 3 May 2023 22:04:46 +0100 (BST) From: "Maciej W. Rozycki" To: Richard Sandiford cc: YunQiang Su , gcc-patches@gcc.gnu.org, jiaxun.yang@flygoat.com, syq@debian.org Subject: Re: [PATCH v2] MIPS: add speculation_barrier support In-Reply-To: Message-ID: References: <20230428123327.686353-1-yunqiang.su@cipunited.com> <20230428131249.713463-1-yunqiang.su@cipunited.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-1169.3 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_INFOUSMEBIZ,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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