Message ID | 93ab62b2b9473733e5118f4265b61804978adfd7.camel@mengyan1223.wang |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 99325385781E for <patchwork@sourceware.org>; Mon, 7 Mar 2022 20:41:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 99325385781E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1646685682; bh=emcSmr93nNUvCaQdeimX3zaSe0Gn0V8fOpirJVUiG4w=; h=Subject:To:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=cDAxvWq31MZc7MwLl1ceRH+M3MwXrNf3lMRMVH3R46qNbmSg9pq5iyhh3fDIBSvtc G/Tz38TFt1fXZahc8SbPxsjsuldlrz6OMqgDGRQHMi1CT4hnflfJr9c0tbkkNEhrR0 uXdrr7azjW2GpqqDgBIUEneaM2y1Gx9Th/9Q1HF8= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mengyan1223.wang (mengyan1223.wang [89.208.246.23]) by sourceware.org (Postfix) with ESMTPS id 3617C385841D; Mon, 7 Mar 2022 20:40:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3617C385841D Received: from [IPv6:240e:358:1188:9c00:dc73:854d:832e:4] (unknown [IPv6:240e:358:1188:9c00:dc73:854d:832e:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature ECDSA (P-384)) (Client did not present a certificate) (Authenticated sender: xry111@mengyan1223.wang) by mengyan1223.wang (Postfix) with ESMTPSA id 0032A66133; Mon, 7 Mar 2022 15:40:46 -0500 (EST) Message-ID: <93ab62b2b9473733e5118f4265b61804978adfd7.camel@mengyan1223.wang> Subject: [PATCH RFC] mips: add TARGET_ZERO_CALL_USED_REGS hook [PR104817, PR104820] To: gcc-patches@gcc.gnu.org Date: Tue, 08 Mar 2022 04:40:34 +0800 Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.42.4 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3039.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, JMQ_SPF_NEUTRAL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Xi Ruoyao <xry111@mengyan1223.wang> Cc: YunQiang Su <yunqiang.su@cipunited.com>, Richard Sandiford <richard.sandiford@arm.com>, Jakub Jelinek <jakub@gcc.gnu.org>, Jeff Law <law@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
[RFC] mips: add TARGET_ZERO_CALL_USED_REGS hook [PR104817, PR104820]
|
|
Commit Message
Xi Ruoyao
March 7, 2022, 8:40 p.m. UTC
Bootstrapped and regtested on mips64el-linux-gnuabi64. I'm not sure if it's "correct" to clobber other registers during the zeroing of scratch registers. But I can't really come up with a better idea: on MIPS there is no simple way to clear one bit in FCSR (i. e. FCC[x]). We can't just use "c.f.s $fccx,$f0,$f0" because it will raise an exception if $f0 contains a sNaN. -- This fixes the ICEs using -fzero-call-used-regs=all for MIPS target. OpenSSH-8.9p1 has started to enable this by default, giving us a reason to fix -fzero-call-used-regs for more targets. gcc/ PR target/104817 PR target/104820 * config/mips/mips.cc (mips_zero_call_used_regs): New function. (TARGET_ZERO_CALL_USED_REGS): Define. * config/mips/mips.md (mips_zero_fcc): New insn. gcc/testsuite * c-c++-common/zero-scratch-regs-8.c: Enable for MIPS. * c-c++-common/zero-scratch-regs-9.c: Likewise. * c-c++-common/zero-scratch-regs-10.c: Likewise. * c-c++-common/zero-scratch-regs-11.c: Likewise. --- gcc/config/mips/mips.cc | 47 +++++++++++++++++++ gcc/config/mips/mips.md | 6 +++ .../c-c++-common/zero-scratch-regs-10.c | 2 +- .../c-c++-common/zero-scratch-regs-11.c | 2 +- .../c-c++-common/zero-scratch-regs-8.c | 2 +- .../c-c++-common/zero-scratch-regs-9.c | 2 +- 6 files changed, 57 insertions(+), 4 deletions(-)
Comments
Xi Ruoyao <xry111@mengyan1223.wang> writes: > Bootstrapped and regtested on mips64el-linux-gnuabi64. > > I'm not sure if it's "correct" to clobber other registers during the > zeroing of scratch registers. But I can't really come up with a better > idea: on MIPS there is no simple way to clear one bit in FCSR (i. e. > FCC[x]). We can't just use "c.f.s $fccx,$f0,$f0" because it will raise > an exception if $f0 contains a sNaN. Yeah, it's a bit of a grey area, but I think it should be fine, provided that the extra clobbers are never used as return registers (which is obviously true for the FCC registers). But on that basis… > +static HARD_REG_SET > +mips_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs) > +{ > + HARD_REG_SET zeroed_hardregs; > + CLEAR_HARD_REG_SET (zeroed_hardregs); > + > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, HI_REGNUM)) > + { > + /* Clear HI and LO altogether. MIPS target treats HILO as a > + double-word register. */ > + machine_mode dword_mode = TARGET_64BIT ? TImode : DImode; > + rtx hilo = gen_rtx_REG (dword_mode, MD_REG_FIRST); > + rtx zero = CONST0_RTX (dword_mode); > + emit_move_insn (hilo, zero); > + > + SET_HARD_REG_BIT (zeroed_hardregs, HI_REGNUM); > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, LO_REGNUM)) > + SET_HARD_REG_BIT (zeroed_hardregs, LO_REGNUM); > + else > + emit_clobber (gen_rtx_REG (word_mode, LO_REGNUM)); …I don't think this conditional LO_REGNUM code is worth it. We might as well just add both registers to zeroed_hardregs. > + } > + > + bool zero_fcc = false; > + for (int i = ST_REG_FIRST; i <= ST_REG_LAST; i++) > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, i)) > + zero_fcc = true; > + > + /* MIPS does not have a simple way to clear one bit in FCC. We just > + clear FCC with ctc1 and clobber all FCC bits. */ > + if (zero_fcc) > + { > + emit_insn (gen_mips_zero_fcc ()); > + for (int i = ST_REG_FIRST; i <= ST_REG_LAST; i++) > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, i)) > + SET_HARD_REG_BIT (zeroed_hardregs, i); > + else > + emit_clobber (gen_rtx_REG (CCmode, i)); > + } Here too I think we should just do: zeroed_hardregs |= reg_class_contents[ST_REGS] & accessible_reg_set; to include all available FCC registers. > + > + need_zeroed_hardregs &= ~zeroed_hardregs; > + return zeroed_hardregs | > + default_zero_call_used_regs (need_zeroed_hardregs); Nit, but: should be formatted as: return (zeroed_hardregs | default_zero_call_used_regs (need_zeroed_hardregs)); > +} > + > > /* Initialize the GCC target structure. */ > #undef TARGET_ASM_ALIGNED_HI_OP > @@ -22919,6 +22964,8 @@ mips_asm_file_end (void) > #undef TARGET_ASM_FILE_END > #define TARGET_ASM_FILE_END mips_asm_file_end > > +#undef TARGET_ZERO_CALL_USED_REGS > +#define TARGET_ZERO_CALL_USED_REGS mips_zero_call_used_regs > > struct gcc_target targetm = TARGET_INITIALIZER; > > diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md > index e0f0a582732..edf58710cdd 100644 > --- a/gcc/config/mips/mips.md > +++ b/gcc/config/mips/mips.md > @@ -96,6 +96,7 @@ (define_c_enum "unspec" [ > ;; Floating-point environment. > UNSPEC_GET_FCSR > UNSPEC_SET_FCSR > + UNSPEC_ZERO_FCC > > ;; HI/LO moves. > UNSPEC_MFHI > @@ -7670,6 +7671,11 @@ (define_insn "*mips_set_fcsr" > "TARGET_HARD_FLOAT" > "ctc1\t%0,$31") > > +(define_insn "mips_zero_fcc" > + [(unspec_volatile [(const_int 0)] UNSPEC_ZERO_FCC)] > + "TARGET_HARD_FLOAT" > + "ctc1\t$0,$25") I've forgotten a lot of MIPS stuff, so: does this clear only the FCC registers, or does it clear other things (such as exception bits) as well? Does it work even for !ISA_HAS_8CC? I think this pattern should explicit clear all eight registers, e.g. using: (set (reg:CC FCC0_REGNUM) (const_int 0)) (set (reg:CC FCC1_REGNUM) (const_int 0)) … which unfortunately means defining 8 new register constants in mips.md. I guess for extra safety there should be a separate !ISA_HAS_8CC version that only sets FCC0_REGNUM. An alternative would be to avoid clearing the FCC registers altogether. I suppose that's less secure, but residual information could leak through the exception bits as well, and it isn't clear whether those should be zeroed at the end of each function. I guess it depends on people's appetite for risk. Both ways are OK with me, just mentioning it in case. Thanks, Richard > + > ;; See tls_get_tp_mips16_<mode> for why this form is used. > (define_insn "mips_set_fcsr_mips16_<mode>" > [(unspec_volatile:SI [(match_operand:P 0 "call_insn_operand" "dS") > diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c > index 96e0b79b328..c23b2ceb391 100644 > --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c > +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c > @@ -1,5 +1,5 @@ > /* { dg-do run } */ > -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* nvptx*-*-* s390*-*-* } } } */ > +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ > /* { dg-options "-O2" } */ > > #include <assert.h> > diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c > index 0714f95a04f..f51f5a2161c 100644 > --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c > +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c > @@ -1,5 +1,5 @@ > /* { dg-do run } */ > -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ > +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all" } */ > > #include "zero-scratch-regs-10.c" > diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c > index aceda7e5cb8..3e5e59b3c79 100644 > --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c > +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c > @@ -1,5 +1,5 @@ > /* { dg-do run } */ > -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ > +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all-arg" } */ > > #include "zero-scratch-regs-1.c" > diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c > index f3152a7a732..d88d61accb2 100644 > --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c > +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c > @@ -1,5 +1,5 @@ > /* { dg-do run } */ > -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ > +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ > /* { dg-options "-O2 -fzero-call-used-regs=all" } */ > > #include "zero-scratch-regs-1.c"
On Wed, 2022-03-09 at 18:25 +0000, Richard Sandiford wrote: > Xi Ruoyao <xry111@mengyan1223.wang> writes: > > Bootstrapped and regtested on mips64el-linux-gnuabi64. > > > > I'm not sure if it's "correct" to clobber other registers during the > > zeroing of scratch registers. But I can't really come up with a > > better > > idea: on MIPS there is no simple way to clear one bit in FCSR (i. e. > > FCC[x]). We can't just use "c.f.s $fccx,$f0,$f0" because it will > > raise > > an exception if $f0 contains a sNaN. > > Yeah, it's a bit of a grey area, but I think it should be fine, > provided > that the extra clobbers are never used as return registers (which is > obviously true for the FCC registers). > > But on that basis… /* snip */ > > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, LO_REGNUM)) > > + SET_HARD_REG_BIT (zeroed_hardregs, LO_REGNUM); > > + else > > + emit_clobber (gen_rtx_REG (word_mode, LO_REGNUM)); > > …I don't think this conditional LO_REGNUM code is worth it. > We might as well just add both registers to zeroed_hardregs. (See below) > > + } > > + > > + bool zero_fcc = false; > > + for (int i = ST_REG_FIRST; i <= ST_REG_LAST; i++) > > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, i)) > > + zero_fcc = true; > > + > > + /* MIPS does not have a simple way to clear one bit in FCC. We just > > + clear FCC with ctc1 and clobber all FCC bits. */ > > + if (zero_fcc) > > + { > > + emit_insn (gen_mips_zero_fcc ()); > > + for (int i = ST_REG_FIRST; i <= ST_REG_LAST; i++) > > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, i)) > > + SET_HARD_REG_BIT (zeroed_hardregs, i); > > + else > > + emit_clobber (gen_rtx_REG (CCmode, i)); > > + } > > Here too I think we should just do: > > zeroed_hardregs |= reg_class_contents[ST_REGS] & accessible_reg_set; > > to include all available FCC registers. I'm afraid that doing so will cause an ICE (triggering an assertion somewhere). Could someone confirm that returning "more" registers than required is allowed? GCC Internal does not say it explicitly, and x86 port is carefully avoiding from clearing registers not requested to be cleared. > > + need_zeroed_hardregs &= ~zeroed_hardregs; > > + return zeroed_hardregs | > > + default_zero_call_used_regs (need_zeroed_hardregs); > > Nit, but: should be formatted as: > > return (zeroed_hardregs > | default_zero_call_used_regs (need_zeroed_hardregs)); > > > +} Will do. > > /* Initialize the GCC target structure. */ > > #undef TARGET_ASM_ALIGNED_HI_OP > > @@ -22919,6 +22964,8 @@ mips_asm_file_end (void) > > #undef TARGET_ASM_FILE_END > > #define TARGET_ASM_FILE_END mips_asm_file_end > > > > +#undef TARGET_ZERO_CALL_USED_REGS > > +#define TARGET_ZERO_CALL_USED_REGS mips_zero_call_used_regs > > > > struct gcc_target targetm = TARGET_INITIALIZER; > > > > diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md > > index e0f0a582732..edf58710cdd 100644 > > --- a/gcc/config/mips/mips.md > > +++ b/gcc/config/mips/mips.md > > @@ -96,6 +96,7 @@ (define_c_enum "unspec" [ > > ;; Floating-point environment. > > UNSPEC_GET_FCSR > > UNSPEC_SET_FCSR > > + UNSPEC_ZERO_FCC > > > > ;; HI/LO moves. > > UNSPEC_MFHI > > @@ -7670,6 +7671,11 @@ (define_insn "*mips_set_fcsr" > > "TARGET_HARD_FLOAT" > > "ctc1\t%0,$31") > > > > +(define_insn "mips_zero_fcc" > > + [(unspec_volatile [(const_int 0)] UNSPEC_ZERO_FCC)] > > + "TARGET_HARD_FLOAT" > > + "ctc1\t$0,$25") > > I've forgotten a lot of MIPS stuff, so: does this clear only the > FCC registers, or does it clear other things (such as exception bits) > as well? Yes, with fs = 25 CTC1 only clear FCCs. > Does it work even for !ISA_HAS_8CC? For !ISA_HAS_8CC targets, ST_REG_FIRST is not added into need_zeroed_hardregs at all. I think it's another bug I didn't catched... > I think this pattern should explicit clear all eight registers, e.g. > using: > > (set (reg:CC FCC0_REGNUM) (const_int 0)) > (set (reg:CC FCC1_REGNUM) (const_int 0)) > … > > which unfortunately means defining 8 new register constants in mips.md. > I guess for extra safety there should be a separate !ISA_HAS_8CC version > that only sets FCC0_REGNUM. Will do. > An alternative would be to avoid clearing the FCC registers altogether. > I suppose that's less secure, but residual information could leak through > the exception bits as well, and it isn't clear whether those should be > zeroed at the end of each function. I guess it depends on people's > appetite for risk.
Changes from v1: * Added all zeroed registers into the return value of TARGET_ZERO_CALL_USED_REGS. **The question: is this allowed?** * Define mips_zero_fcc insn only for ISA_HAS_8CC and mips_isa > MIPS_ISA_MIPS4, because MIPS I-IV and MIPSr6 don't support "ISA_HAS_8CC && mips_isa > MIPS_ISA_MIPS4". * Change mips_zero_fcc to explicit clear all eight registers. * Report an error for MIPS I-IV. -- >8 -- This fixes the ICEs using -fzero-call-used-regs=all for MIPS target. OpenSSH-8.9p1 has started to enable this by default, giving us a reason to fix -fzero-call-used-regs for more targets. gcc/ PR target/xxxxxx (WIP) PR target/xxxxxx (Don't push) * config/mips/mips.cc (mips_zero_call_used_regs): New function. (TARGET_ZERO_CALL_USED_REGS): Define. * config/mips/mips.md (FCC{0..9}_REGNUM): New constants. (mips_zero_fcc): New insn. gcc/testsuite * c-c++-common/zero-scratch-regs-8.c: Enable for MIPS. * c-c++-common/zero-scratch-regs-9.c: Likewise. * c-c++-common/zero-scratch-regs-10.c: Likewise. * c-c++-common/zero-scratch-regs-11.c: Likewise. --- gcc/config/mips/mips.cc | 55 +++++++++++++++++++ gcc/config/mips/mips.md | 20 +++++++ .../c-c++-common/zero-scratch-regs-10.c | 2 +- .../c-c++-common/zero-scratch-regs-11.c | 2 +- .../c-c++-common/zero-scratch-regs-8.c | 2 +- .../c-c++-common/zero-scratch-regs-9.c | 2 +- 6 files changed, 79 insertions(+), 4 deletions(-) diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc index 4f9683e8bf4..59eef515826 100644 --- a/gcc/config/mips/mips.cc +++ b/gcc/config/mips/mips.cc @@ -22611,6 +22611,59 @@ mips_asm_file_end (void) if (NEED_INDICATE_EXEC_STACK) file_end_indicate_exec_stack (); } + +static HARD_REG_SET +mips_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs) +{ + HARD_REG_SET zeroed_hardregs; + CLEAR_HARD_REG_SET (zeroed_hardregs); + + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, HI_REGNUM)) + { + /* Clear HI and LO altogether. MIPS target treats HILO as a + double-word register. */ + machine_mode dword_mode = TARGET_64BIT ? TImode : DImode; + rtx hilo = gen_rtx_REG (dword_mode, MD_REG_FIRST); + rtx zero = CONST0_RTX (dword_mode); + emit_move_insn (hilo, zero); + + SET_HARD_REG_BIT (zeroed_hardregs, HI_REGNUM); + SET_HARD_REG_BIT (zeroed_hardregs, LO_REGNUM); + } + + /* MIPS does not have a simple way to clear one bit in FCC. We just + clear FCC with ctc1 and clobber all FCC bits. */ + HARD_REG_SET fcc = reg_class_contents[ST_REGS] & accessible_reg_set; + if (hard_reg_set_intersect_p (need_zeroed_hardregs, fcc)) + { + static bool issued_error = false; + if (mips_isa <= MIPS_ISA_MIPS4) + { + /* We don't have an easy way to clear FCC on MIPS I, II, III, + and IV. */ + if (!issued_error) + sorry ("%qs not supported on this target", + "-fzero-call-used-regs"); + issued_error = true; + + /* Prevent an ICE. */ + need_zeroed_hardregs &= ~fcc; + } + else + { + /* If the target is MIPSr6, we should not reach here. All other + MIPS targets are ISA_HAS_8CC. */ + gcc_assert (ISA_HAS_8CC); + emit_insn (gen_mips_zero_fcc ()); + zeroed_hardregs |= fcc; + } + } + + need_zeroed_hardregs &= ~zeroed_hardregs; + return (zeroed_hardregs | + default_zero_call_used_regs (need_zeroed_hardregs)); +} + /* Initialize the GCC target structure. */ #undef TARGET_ASM_ALIGNED_HI_OP @@ -22919,6 +22972,8 @@ mips_asm_file_end (void) #undef TARGET_ASM_FILE_END #define TARGET_ASM_FILE_END mips_asm_file_end +#undef TARGET_ZERO_CALL_USED_REGS +#define TARGET_ZERO_CALL_USED_REGS mips_zero_call_used_regs struct gcc_target targetm = TARGET_INITIALIZER; diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index e0f0a582732..36d6a43d67f 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -167,6 +167,14 @@ (define_constants (SET_FCSR_REGNUM 4) (PIC_FUNCTION_ADDR_REGNUM 25) (RETURN_ADDR_REGNUM 31) + (FCC0_REGNUM 67) + (FCC1_REGNUM 68) + (FCC2_REGNUM 69) + (FCC3_REGNUM 70) + (FCC4_REGNUM 71) + (FCC5_REGNUM 72) + (FCC6_REGNUM 73) + (FCC7_REGNUM 74) (CPRESTORE_SLOT_REGNUM 76) (GOT_VERSION_REGNUM 79) @@ -7670,6 +7678,18 @@ (define_insn "*mips_set_fcsr" "TARGET_HARD_FLOAT" "ctc1\t%0,$31") +(define_insn "mips_zero_fcc" + [(set (reg:CC FCC0_REGNUM) (const_int 0)) + (set (reg:CC FCC1_REGNUM) (const_int 0)) + (set (reg:CC FCC2_REGNUM) (const_int 0)) + (set (reg:CC FCC3_REGNUM) (const_int 0)) + (set (reg:CC FCC4_REGNUM) (const_int 0)) + (set (reg:CC FCC5_REGNUM) (const_int 0)) + (set (reg:CC FCC6_REGNUM) (const_int 0)) + (set (reg:CC FCC7_REGNUM) (const_int 0))] + "TARGET_HARD_FLOAT && ISA_HAS_8CC && mips_isa > MIPS_ISA_MIPS4" + "ctc1\t$0,$25") + ;; See tls_get_tp_mips16_<mode> for why this form is used. (define_insn "mips_set_fcsr_mips16_<mode>" [(unspec_volatile:SI [(match_operand:P 0 "call_insn_operand" "dS") diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c index 96e0b79b328..c23b2ceb391 100644 --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* nvptx*-*-* s390*-*-* } } } */ +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ /* { dg-options "-O2" } */ #include <assert.h> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c index 0714f95a04f..f51f5a2161c 100644 --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ /* { dg-options "-O2 -fzero-call-used-regs=all" } */ #include "zero-scratch-regs-10.c" diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c index aceda7e5cb8..3e5e59b3c79 100644 --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ /* { dg-options "-O2 -fzero-call-used-regs=all-arg" } */ #include "zero-scratch-regs-1.c" diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c index f3152a7a732..d88d61accb2 100644 --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ /* { dg-options "-O2 -fzero-call-used-regs=all" } */ #include "zero-scratch-regs-1.c"
> On Mar 9, 2022, at 12:25 PM, Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Xi Ruoyao <xry111@mengyan1223.wang> writes: >> Bootstrapped and regtested on mips64el-linux-gnuabi64. >> >> I'm not sure if it's "correct" to clobber other registers during the >> zeroing of scratch registers. But I can't really come up with a better >> idea: on MIPS there is no simple way to clear one bit in FCSR (i. e. >> FCC[x]). We can't just use "c.f.s $fccx,$f0,$f0" because it will raise >> an exception if $f0 contains a sNaN. > > Yeah, it's a bit of a grey area, but I think it should be fine, provided > that the extra clobbers are never used as return registers (which is > obviously true for the FCC registers). > > But on that basis… > >> +static HARD_REG_SET >> +mips_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs) >> +{ >> + HARD_REG_SET zeroed_hardregs; >> + CLEAR_HARD_REG_SET (zeroed_hardregs); >> + >> + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, HI_REGNUM)) >> + { >> + /* Clear HI and LO altogether. MIPS target treats HILO as a >> + double-word register. */ >> + machine_mode dword_mode = TARGET_64BIT ? TImode : DImode; >> + rtx hilo = gen_rtx_REG (dword_mode, MD_REG_FIRST); >> + rtx zero = CONST0_RTX (dword_mode); >> + emit_move_insn (hilo, zero); >> + >> + SET_HARD_REG_BIT (zeroed_hardregs, HI_REGNUM); >> + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, LO_REGNUM)) >> + SET_HARD_REG_BIT (zeroed_hardregs, LO_REGNUM); >> + else >> + emit_clobber (gen_rtx_REG (word_mode, LO_REGNUM)); > > …I don't think this conditional LO_REGNUM code is worth it. > We might as well just add both registers to zeroed_hardregs. If the LO_REGNUM is NOT in “need_zeroed_hardregs”, adding it to “zeroed_hardregs” seems not right to me. What’s you mean by “not worth it”? > >> + } >> + >> + bool zero_fcc = false; >> + for (int i = ST_REG_FIRST; i <= ST_REG_LAST; i++) >> + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, i)) >> + zero_fcc = true; >> + >> + /* MIPS does not have a simple way to clear one bit in FCC. We just >> + clear FCC with ctc1 and clobber all FCC bits. */ >> + if (zero_fcc) >> + { >> + emit_insn (gen_mips_zero_fcc ()); >> + for (int i = ST_REG_FIRST; i <= ST_REG_LAST; i++) >> + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, i)) >> + SET_HARD_REG_BIT (zeroed_hardregs, i); >> + else >> + emit_clobber (gen_rtx_REG (CCmode, i)); >> + } > > Here too I think we should just do: > > zeroed_hardregs |= reg_class_contents[ST_REGS] & accessible_reg_set; > > to include all available FCC registers. What’s the relationship between “ST_REGs” and FCC? (sorry for the stupid question since I am not familiar with the MIPS register set). From the above code, looks like that when any “ST_REGs” is in “need_zeroed_hardregs”,FCC need to be cleared? thanks. Qing > >> + >> + need_zeroed_hardregs &= ~zeroed_hardregs; >> + return zeroed_hardregs | >> + default_zero_call_used_regs (need_zeroed_hardregs); > > Nit, but: should be formatted as: > > return (zeroed_hardregs > | default_zero_call_used_regs (need_zeroed_hardregs)); > >> +} >> + >> >> /* Initialize the GCC target structure. */ >> #undef TARGET_ASM_ALIGNED_HI_OP >> @@ -22919,6 +22964,8 @@ mips_asm_file_end (void) >> #undef TARGET_ASM_FILE_END >> #define TARGET_ASM_FILE_END mips_asm_file_end >> >> +#undef TARGET_ZERO_CALL_USED_REGS >> +#define TARGET_ZERO_CALL_USED_REGS mips_zero_call_used_regs >> >> struct gcc_target targetm = TARGET_INITIALIZER; >> >> diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md >> index e0f0a582732..edf58710cdd 100644 >> --- a/gcc/config/mips/mips.md >> +++ b/gcc/config/mips/mips.md >> @@ -96,6 +96,7 @@ (define_c_enum "unspec" [ >> ;; Floating-point environment. >> UNSPEC_GET_FCSR >> UNSPEC_SET_FCSR >> + UNSPEC_ZERO_FCC >> >> ;; HI/LO moves. >> UNSPEC_MFHI >> @@ -7670,6 +7671,11 @@ (define_insn "*mips_set_fcsr" >> "TARGET_HARD_FLOAT" >> "ctc1\t%0,$31") >> >> +(define_insn "mips_zero_fcc" >> + [(unspec_volatile [(const_int 0)] UNSPEC_ZERO_FCC)] >> + "TARGET_HARD_FLOAT" >> + "ctc1\t$0,$25") > > I've forgotten a lot of MIPS stuff, so: does this clear only the > FCC registers, or does it clear other things (such as exception bits) > as well? Does it work even for !ISA_HAS_8CC? > > I think this pattern should explicit clear all eight registers, e.g. using: > > (set (reg:CC FCC0_REGNUM) (const_int 0)) > (set (reg:CC FCC1_REGNUM) (const_int 0)) > … > > which unfortunately means defining 8 new register constants in mips.md. > I guess for extra safety there should be a separate !ISA_HAS_8CC version > that only sets FCC0_REGNUM. > > An alternative would be to avoid clearing the FCC registers altogether. > I suppose that's less secure, but residual information could leak through > the exception bits as well, and it isn't clear whether those should be > zeroed at the end of each function. I guess it depends on people's > appetite for risk. > > Both ways are OK with me, just mentioning it in case. > > Thanks, > Richard > >> + >> ;; See tls_get_tp_mips16_<mode> for why this form is used. >> (define_insn "mips_set_fcsr_mips16_<mode>" >> [(unspec_volatile:SI [(match_operand:P 0 "call_insn_operand" "dS") >> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c >> index 96e0b79b328..c23b2ceb391 100644 >> --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c >> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c >> @@ -1,5 +1,5 @@ >> /* { dg-do run } */ >> -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* nvptx*-*-* s390*-*-* } } } */ >> +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ >> /* { dg-options "-O2" } */ >> >> #include <assert.h> >> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c >> index 0714f95a04f..f51f5a2161c 100644 >> --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c >> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c >> @@ -1,5 +1,5 @@ >> /* { dg-do run } */ >> -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ >> +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ >> /* { dg-options "-O2 -fzero-call-used-regs=all" } */ >> >> #include "zero-scratch-regs-10.c" >> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c >> index aceda7e5cb8..3e5e59b3c79 100644 >> --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c >> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c >> @@ -1,5 +1,5 @@ >> /* { dg-do run } */ >> -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ >> +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ >> /* { dg-options "-O2 -fzero-call-used-regs=all-arg" } */ >> >> #include "zero-scratch-regs-1.c" >> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c >> index f3152a7a732..d88d61accb2 100644 >> --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c >> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c >> @@ -1,5 +1,5 @@ >> /* { dg-do run } */ >> -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ >> +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ >> /* { dg-options "-O2 -fzero-call-used-regs=all" } */ >> >> #include "zero-scratch-regs-1.c"
On Thu, 2022-03-10 at 20:31 +0000, Qing Zhao wrote: > > > + SET_HARD_REG_BIT (zeroed_hardregs, HI_REGNUM); > > > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, LO_REGNUM)) > > > + SET_HARD_REG_BIT (zeroed_hardregs, LO_REGNUM); > > > + else > > > + emit_clobber (gen_rtx_REG (word_mode, LO_REGNUM)); > > > > …I don't think this conditional LO_REGNUM code is worth it. > > We might as well just add both registers to zeroed_hardregs. > > If the LO_REGNUM is NOT in “need_zeroed_hardregs”, adding it to “zeroed_hardregs” seems not right to me. > What’s you mean by “not worth it”? It's because the MIPS port almost always treat HI as "a subreg of dword HI-LO register". A direct "mthi $0" is possible but MIPS backend does not recognize "emit_move_insn (HI, CONST_0)". In theory it's possible to emit the mthi instruction explicitly here though, but we'll need to clear something NOT in need_zeroed_hardregs for MIPS anyway (see below). > > Here too I think we should just do: > > > > zeroed_hardregs |= reg_class_contents[ST_REGS] & accessible_reg_set; > > > > to include all available FCC registers. > > What’s the relationship between “ST_REGs” and FCC? (sorry for the stupid question since I am not familiar with the MIPS register set). MIPS instruction manual names the 8 one-bit floating condition codes FCC0, ..., FCC7, but GCC MIPS backend code names the condition codes ST_REG0, ..., ST_REG7. Maybe it's better to always use the name "ST_REG" instead of "FCC" then. > From the above code, looks like that when any “ST_REGs” is in “need_zeroed_hardregs”,FCC need to be cleared? Because there is no elegant way to clear one specific FCC bit in MIPS. A "ctc1 $0, $25" instruction will zero them altogether. If we really need to clear only one of them (let's say ST_REG3), we'll have to emit something like mtc1 $0, $0 # zero FPR0 to ensure it won't contain sNaN c.f.s $3, $0, $0 Then we'll still need to clobber FPR0 with zero. So anyway we'll have to clear some registers not specified in need_zeroed_hardregs. And the question is: is it really allowed to return something other than a subset of need_zeroed_hardregs for a TARGET_ZERO_CALL_USED_REGS hook? If yes then we'll happily to do so (like how the v2 of the patch does), otherwise we'd need to clobber those registers NOT in need_zeroed_hardregs explicitly.
> On Mar 10, 2022, at 8:54 PM, Xi Ruoyao <xry111@mengyan1223.wang> wrote: > > On Thu, 2022-03-10 at 20:31 +0000, Qing Zhao wrote: > >>>> + SET_HARD_REG_BIT (zeroed_hardregs, HI_REGNUM); >>>> + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, LO_REGNUM)) >>>> + SET_HARD_REG_BIT (zeroed_hardregs, LO_REGNUM); >>>> + else >>>> + emit_clobber (gen_rtx_REG (word_mode, LO_REGNUM)); >>> >>> …I don't think this conditional LO_REGNUM code is worth it. >>> We might as well just add both registers to zeroed_hardregs. >> >> If the LO_REGNUM is NOT in “need_zeroed_hardregs”, adding it to “zeroed_hardregs” seems not right to me. >> What’s you mean by “not worth it”? > > It's because the MIPS port almost always treat HI as "a subreg of dword > HI-LO register". A direct "mthi $0" is possible but MIPS backend does > not recognize "emit_move_insn (HI, CONST_0)”. Why there is “mthi $0” instruction, but there is NO emit_move_insn(HI, CONST_0)? Is such mismatch a bug? If not, why? > In theory it's possible > to emit the mthi instruction explicitly here though, but we'll need to > clear something NOT in need_zeroed_hardregs for MIPS anyway (see below). One question here, is there situation when only HI is cleared but LO is not cleared? > >>> Here too I think we should just do: >>> >>> zeroed_hardregs |= reg_class_contents[ST_REGS] & accessible_reg_set; >>> >>> to include all available FCC registers. >> >> What’s the relationship between “ST_REGs” and FCC? (sorry for the stupid question since I am not familiar with the MIPS register set). > > MIPS instruction manual names the 8 one-bit floating condition codes > FCC0, ..., FCC7, but GCC MIPS backend code names the condition codes > ST_REG0, ..., ST_REG7. Maybe it's better to always use the name > "ST_REG" instead of "FCC" then. Okay, I see. So, each ST_REGi register is a 1-bit pseudo register? But physically each of them is 1-bit in a physical register? > >> From the above code, looks like that when any “ST_REGs” is in “need_zeroed_hardregs”,FCC need to be cleared? > > Because there is no elegant way to clear one specific FCC bit in MIPS. > A "ctc1 $0, $25" instruction will zero them altogether. If we really > need to clear only one of them (let's say ST_REG3), we'll have to emit > something like > > mtc1 $0, $0 # zero FPR0 to ensure it won't contain sNaN > c.f.s $3, $0, $0 > > Then we'll still need to clobber FPR0 with zero. So anyway we'll have > to clear some registers not specified in need_zeroed_hardregs. So, “c.f.s” instruction can be used to clear ONLY one specific FCC bit? But you have to clear one FPR (floating pointer register?) first to avoid raising exception? My question here is: is there a case when only FCC need to be cleared but no FPR need to be cleared? If NOT, then we can always pick one FPRi before c.f.s to avoid the issue you mentioned (We’ll have to clear some registers not specified in need_zeroed_hardregs). > > And the question is: is it really allowed to return something other than > a subset of need_zeroed_hardregs for a TARGET_ZERO_CALL_USED_REGS hook? Although currently there is no assertion added to force this requirement, I still think that we should keep it. The “need_zeroed_hardregs” is computed based on 1. User’s request from command line option; 2. Data flow info of the routine; 3. Abi info of the target; If zero_call_used_regs target hook return registers out of “need_zeroed_hardregs” set, then it might out of the user’s exception, it should be considered as a bug, I think. Qing > If yes then we'll happily to do so (like how the v2 of the patch does), > otherwise we'd need to clobber those registers NOT in > need_zeroed_hardregs explicitly. > -- > Xi Ruoyao <xry111@mengyan1223.wang> > School of Aerospace Science and Technology, Xidian University
On Fri, 2022-03-11 at 16:08 +0000, Qing Zhao wrote: > Why there is “mthi $0” instruction, but there is NO emit_move_insn(HI, CONST_0)? > Is such mismatch a bug? If not, why? > > > In theory it's possible > > to emit the mthi instruction explicitly here though, but we'll need to > > clear something NOT in need_zeroed_hardregs for MIPS anyway (see below). > > One question here, is there situation when only HI is cleared but LO is not cleared? No, if I interpret the document of -fzero_call_used_regs and attribute((zero_call_used_regs(...))) correctly. A 2-reg multiplication (or division) always set the value of both HI and LO. Richard has added a comment for this in mips.cc: > 12868 /* After a multiplication or division, clobbering HI makes > 1 the value of LO unpredictable, and vice versa. This means > 2 that, for all interesting cases, HI and LO are effectively > 3 a single register. > 4 > 5 We model this by requiring that any value that uses HI > 6 also uses LO. */ This is also why the handling of emit_move_insn(HI, CONST_0) was removed, I guess (the removal happened in the same commit adding this comment). > > > > Okay, I see. So, each ST_REGi register is a 1-bit pseudo register? > But physically each of them is 1-bit in a physical register? Yes. > > > > Because there is no elegant way to clear one specific FCC bit in MIPS. > > A "ctc1 $0, $25" instruction will zero them altogether. If we really > > need to clear only one of them (let's say ST_REG3), we'll have to emit > > something like > > > > mtc1 $0, $0 # zero FPR0 to ensure it won't contain sNaN > > c.f.s $3, $0, $0 > > > > Then we'll still need to clobber FPR0 with zero. So anyway we'll have > > to clear some registers not specified in need_zeroed_hardregs. > > So, “c.f.s” instruction can be used to clear ONLY one specific FCC bit? > But you have to clear one FPR (floating pointer register?) first to avoid raising exception? > My question here is: is there a case when only FCC need to be cleared but no FPR need to be cleared? Yes, for example: double a, b; struct x { double a, b; }; struct x f(void) { struct x x = { .a = a, .b = b }; if (a < b) x.a = x.b; return x; } It does not need to zero the two FPRs, as they contain the return value. But a FCC bit needs to be cleared. > If NOT, then we can always pick one FPRi before c.f.s to avoid the > issue you mentioned (We’ll have to clear some registers not specified > in need_zeroed_hardregs). I'm now thinking: is there always at least one *GPR* which need to be cleared? If it's true, let's say GPR $12, and fcc0 & fcc2 needs to be cleared, we can use something like: cfc1 $12, $25 andi $25, 5 ctc1 $12, $25 move $12, $0 > > And the question is: is it really allowed to return something other than > > a subset of need_zeroed_hardregs for a TARGET_ZERO_CALL_USED_REGS hook? > > Although currently there is no assertion added to force this > requirement, I still think that we should keep it. > > The “need_zeroed_hardregs” is computed based on > > 1. User’s request from command line option; > 2. Data flow info of the routine; > 3. Abi info of the target; > > If zero_call_used_regs target hook return registers out of > “need_zeroed_hardregs” set, then it might out of the user’s exception, > it should be considered as a bug, I think. I have the same concern. But now I'm too sleepy... Will try to improve this tomorrow.
On Sat, 2022-03-12 at 01:29 +0800, Xi Ruoyao via Gcc-patches wrote: > I'm now thinking: is there always at least one *GPR* which need to be > cleared? If it's true, let's say GPR $12, and fcc0 & fcc2 needs to be > cleared, we can use something like: > > cfc1 $12, $25 > andi $25, 5 $12, 5. I can't type. > ctc1 $12, $25 > move $12, $0
Hi, Ruoyao, (I might not be able to reply to this thread till next Wed due to a short vacation). First, some comments on opening bugs against Gcc: I took a look at the bug reports PR104817 and PR104820: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104820 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104817 I didn’t see a testing case and a script to repeat the error, so I cannot repeat the error at my side. So, in order for other people to help to study the bug, you need to provide a testing case and A script or detailed description on how to repeat the bug. In addition to that, it will be helpful if you can provide more details on what’ the root cause of the Issue from your study into the comment part of the bug report. > On Mar 11, 2022, at 11:29 AM, Xi Ruoyao <xry111@mengyan1223.wang> wrote: > > On Fri, 2022-03-11 at 16:08 +0000, Qing Zhao wrote: > >> Why there is “mthi $0” instruction, but there is NO emit_move_insn(HI, CONST_0)? >> Is such mismatch a bug? If not, why? >> >>> In theory it's possible >>> to emit the mthi instruction explicitly here though, but we'll need to >>> clear something NOT in need_zeroed_hardregs for MIPS anyway (see below). >> >> One question here, is there situation when only HI is cleared but LO is not cleared? > > No, if I interpret the document of -fzero_call_used_regs and > attribute((zero_call_used_regs(...))) correctly. A 2-reg multiplication > (or division) always set the value of both HI and LO. Richard has added > a comment for this in mips.cc: > >> 12868 /* After a multiplication or division, clobbering HI makes >> 1 the value of LO unpredictable, and vice versa. This means >> 2 that, for all interesting cases, HI and LO are effectively >> 3 a single register. >> 4 >> 5 We model this by requiring that any value that uses HI >> 6 also uses LO. */ > > This is also why the handling of emit_move_insn(HI, CONST_0) was > removed, I guess (the removal happened in the same commit adding this > comment). Okay. I see. Then the current handling for HI_REGNUM is reasonable. I suggest to add one assertion inside the handling of HI_REGNUM with proper comment: gcc_assertion (TEST_HARD_REG_BIT (need_zeroed_hardregs, LO_REGNUM)); to catch any unexpected bug. Richard, what’s your opinion on this? > > >>>> >> Okay, I see. So, each ST_REGi register is a 1-bit pseudo register? >> But physically each of them is 1-bit in a physical register? > > Yes. > >>> >>> Because there is no elegant way to clear one specific FCC bit in MIPS. >>> A "ctc1 $0, $25" instruction will zero them altogether. If we really >>> need to clear only one of them (let's say ST_REG3), we'll have to emit >>> something like >>> >>> mtc1 $0, $0 # zero FPR0 to ensure it won't contain sNaN >>> c.f.s $3, $0, $0 >>> >>> Then we'll still need to clobber FPR0 with zero. So anyway we'll have >>> to clear some registers not specified in need_zeroed_hardregs. >> >> So, “c.f.s” instruction can be used to clear ONLY one specific FCC bit? >> But you have to clear one FPR (floating pointer register?) first to avoid raising exception? >> My question here is: is there a case when only FCC need to be cleared but no FPR need to be cleared? > > Yes, for example: > > double a, b; > > struct x > { > double a, b; > }; > > struct x > f(void) > { > struct x x = > { > .a = a, > .b = b > }; > if (a < b) > x.a = x.b; > return x; > } > > It does not need to zero the two FPRs, as they contain the return value. > But a FCC bit needs to be cleared. Okay. > >> If NOT, then we can always pick one FPRi before c.f.s to avoid the >> issue you mentioned (We’ll have to clear some registers not specified >> in need_zeroed_hardregs). > > I'm now thinking: is there always at least one *GPR* which need to be > cleared? If it's true, let's say GPR $12, and fcc0 & fcc2 needs to be > cleared, we can use something like: So, you mean, in order to set one FCC bit to zero, we have to set another GPR or FPR to zero first? Otherwise an error might occur? Why? (This is unreasonable to me) do I miss anything here? Qing > cfc1 $12, $25 > andi $25, 5 > ctc1 $12, $25 > move $12, $0 > >>> And the question is: is it really allowed to return something other than >>> a subset of need_zeroed_hardregs for a TARGET_ZERO_CALL_USED_REGS hook? >> >> Although currently there is no assertion added to force this >> requirement, I still think that we should keep it. >> >> The “need_zeroed_hardregs” is computed based on >> >> 1. User’s request from command line option; >> 2. Data flow info of the routine; >> 3. Abi info of the target; >> >> If zero_call_used_regs target hook return registers out of >> “need_zeroed_hardregs” set, then it might out of the user’s exception, >> it should be considered as a bug, I think. > > I have the same concern. But now I'm too sleepy... Will try to improve > this tomorrow. > -- > Xi Ruoyao <xry111@mengyan1223.wang> > School of Aerospace Science and Technology, Xidian University
On Fri, 2022-03-11 at 21:26 +0000, Qing Zhao wrote: > Hi, Ruoyao, > > (I might not be able to reply to this thread till next Wed due to a > short vacation). > > First, some comments on opening bugs against Gcc: > > I took a look at the bug reports PR104817 and PR104820: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104820 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104817 > > I didn’t see a testing case and a script to repeat the error, so I > cannot repeat the error at my side. I've put the test case, but maybe you didn't see it because it is too simple: echo 'int t() {}' | /home/xry111/git-repos/gcc-test-mips/gcc/cc1 -nostdinc -fzero-call-used-regs=all An empty function is enough to break -fzero-call-used-regs=all. And if you append -mips64r2 to the cc1 command line you'll get 104820. I enabled 4 existing tests for MIPS (reported "not work" on MIPS) in the patch so I think it's unnecessary to add new test cases. Richard: can we use MIPS_EPILOGUE_TEMP as a scratch register in the sequence for zeroing the call-used registers, and then zero itself (despite it's not in need_zeroed_hardregs)?
On Sat, 2022-03-12 at 18:48 +0800, Xi Ruoyao via Gcc-patches wrote: > On Fri, 2022-03-11 at 21:26 +0000, Qing Zhao wrote: > > Hi, Ruoyao, > > > > (I might not be able to reply to this thread till next Wed due to a > > short vacation). > > > > First, some comments on opening bugs against Gcc: > > > > I took a look at the bug reports PR104817 and PR104820: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104820 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104817 > > > > I didn’t see a testing case and a script to repeat the error, so I > > cannot repeat the error at my side. > > I've put the test case, but maybe you didn't see it because it is too > simple: > > echo 'int t() {}' | /home/xry111/git-repos/gcc-test-mips/gcc/cc1 - > nostdinc -fzero-call-used-regs=all > > An empty function is enough to break -fzero-call-used-regs=all. And > if > you append -mips64r2 to the cc1 command line you'll get 104820. I > enabled 4 existing tests for MIPS (reported "not work" on MIPS) in the > patch so I think it's unnecessary to add new test cases. > > Richard: can we use MIPS_EPILOGUE_TEMP as a scratch register in the > sequence for zeroing the call-used registers, and then zero itself > (despite it's not in need_zeroed_hardregs)? No, it leads to an ICE at stage 3 bootstrapping :(. Now I think the only rational ways are: (1) allow zeroing more registers than need_zeroed_hardregs. Or (2) allow zeroing less registers than need_zeroed_hardregs (then I'll skip ST_REGS, after all they are just 8 bits in total). If all these are unacceptable, then (3) I'll just call sorry in MIPS target hook to tell not to use this feature on MIPS.
Sorry for the slow response, was out for a few days. Xi Ruoyao <xry111@mengyan1223.wang> writes: > On Sat, 2022-03-12 at 18:48 +0800, Xi Ruoyao via Gcc-patches wrote: >> On Fri, 2022-03-11 at 21:26 +0000, Qing Zhao wrote: >> > Hi, Ruoyao, >> > >> > (I might not be able to reply to this thread till next Wed due to a >> > short vacation). >> > >> > First, some comments on opening bugs against Gcc: >> > >> > I took a look at the bug reports PR104817 and PR104820: >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104820 >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104817 >> > >> > I didn’t see a testing case and a script to repeat the error, so I >> > cannot repeat the error at my side. >> >> I've put the test case, but maybe you didn't see it because it is too >> simple: >> >> echo 'int t() {}' | /home/xry111/git-repos/gcc-test-mips/gcc/cc1 - >> nostdinc -fzero-call-used-regs=all >> >> An empty function is enough to break -fzero-call-used-regs=all. And >> if >> you append -mips64r2 to the cc1 command line you'll get 104820. I >> enabled 4 existing tests for MIPS (reported "not work" on MIPS) in the >> patch so I think it's unnecessary to add new test cases. >> >> Richard: can we use MIPS_EPILOGUE_TEMP as a scratch register in the >> sequence for zeroing the call-used registers, and then zero itself >> (despite it's not in need_zeroed_hardregs)? > > No, it leads to an ICE at stage 3 bootstrapping :(. > > Now I think the only rational ways are: > > (1) allow zeroing more registers than need_zeroed_hardregs. I think this is the way to go. I agree it's a bit hacky, but it seems like the least worst option. A less hacky alternative would be to pass an extra argument to the hook that contains the set of registers that the hook is *allowed* to clobber. For -fzero-call-used-regs=X, this new argument would be the set that would have been chosen for -fzero-call-used-regs=all, regardless of what X actually is. We could then assert that the extra registers we want to clobber are in that set (which will be true for all values of X). > Or > > (2) allow zeroing less registers than need_zeroed_hardregs (then I'll > skip ST_REGS, after all they are just 8 bits in total). Yeah, this is explicitly OK, provided that the target maintainers feel that the contents of the registers in question are not a significant security concern. I don't feel I can make that call though. It's really a question for the userbase. Thanks, Richard > If all these are unacceptable, then > > (3) I'll just call sorry in MIPS target hook to tell not to use this > feature on MIPS.
On Mon, 2022-03-14 at 16:04 +0000, Richard Sandiford wrote: > Xi Ruoyao <xry111@mengyan1223.wang> writes: > > Now I think the only rational ways are: > > > > (1) allow zeroing more registers than need_zeroed_hardregs. > > I think this is the way to go. I agree it's a bit hacky, but it seems > like the least worst option. > > (2) allow zeroing less registers than need_zeroed_hardregs (then I'll > > skip ST_REGS, after all they are just 8 bits in total). > > Yeah, this is explicitly OK, provided that the target maintainers > feel that the contents of the registers in question are not a significant > security concern. I don't feel I can make that call though. It's really > a question for the userbase. To me, I believe nobody will write some security-critical code depending on a floating-point compare result :). Maybe I'm wrong by mixing up the concept of "security" and "safety".
> On Mar 14, 2022, at 11:04 AM, Richard Sandiford <richard.sandiford@arm.com> wrote: > > Sorry for the slow response, was out for a few days. > > Xi Ruoyao <xry111@mengyan1223.wang> writes: >> On Sat, 2022-03-12 at 18:48 +0800, Xi Ruoyao via Gcc-patches wrote: >>> On Fri, 2022-03-11 at 21:26 +0000, Qing Zhao wrote: >>>> Hi, Ruoyao, >>>> >>>> (I might not be able to reply to this thread till next Wed due to a >>>> short vacation). >>>> >>>> First, some comments on opening bugs against Gcc: >>>> >>>> I took a look at the bug reports PR104817 and PR104820: >>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104820 >>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104817 >>>> >>>> I didn’t see a testing case and a script to repeat the error, so I >>>> cannot repeat the error at my side. >>> >>> I've put the test case, but maybe you didn't see it because it is too >>> simple: >>> >>> echo 'int t() {}' | /home/xry111/git-repos/gcc-test-mips/gcc/cc1 - >>> nostdinc -fzero-call-used-regs=all >>> >>> An empty function is enough to break -fzero-call-used-regs=all. And >>> if >>> you append -mips64r2 to the cc1 command line you'll get 104820. I >>> enabled 4 existing tests for MIPS (reported "not work" on MIPS) in the >>> patch so I think it's unnecessary to add new test cases. >>> >>> Richard: can we use MIPS_EPILOGUE_TEMP as a scratch register in the >>> sequence for zeroing the call-used registers, and then zero itself >>> (despite it's not in need_zeroed_hardregs)? >> >> No, it leads to an ICE at stage 3 bootstrapping :(. >> >> Now I think the only rational ways are: >> >> (1) allow zeroing more registers than need_zeroed_hardregs. > > I think this is the way to go. I agree it's a bit hacky, but it seems > like the least worst option. > > A less hacky alternative would be to pass an extra argument to the hook > that contains the set of registers that the hook is *allowed* to clobber. > For -fzero-call-used-regs=X, this new argument would be the set that > would have been chosen for -fzero-call-used-regs=all, regardless of > what X actually is. We could then assert that the extra registers we > want to clobber are in that set (which will be true for all values of X). If we have to go this way, I think it’s better to make the change you suggested above, and then also update the documentation, both internal documentation on how to define the hook and the user level documentation on what the user might expect when using this option (i.e, it’s possible that the compiler might clear more registers than the user requests on some targets due to the implementation limitation). I can make this change if we decide to do this. Thanks. Qing > >> Or >> >> (2) allow zeroing less registers than need_zeroed_hardregs (then I'll >> skip ST_REGS, after all they are just 8 bits in total). > > Yeah, this is explicitly OK, provided that the target maintainers > feel that the contents of the registers in question are not a significant > security concern. I don't feel I can make that call though. It's really > a question for the userbase. > > Thanks, > Richard > >> If all these are unacceptable, then >> >> (3) I'll just call sorry in MIPS target hook to tell not to use this >> feature on MIPS.
> > If we have to go this way, I think it’s better to make the change you > suggested above, > and then also update the documentation, both internal documentation on > how to define > the hook and the user level documentation on what the user might > expect when using > this option (i.e, it’s possible that the compiler might clear more > registers than the user > requests on some targets due to the implementation limitation). > > I can make this change if we decide to do this. I'd vote for this. Richard?
Xi Ruoyao <xry111@mengyan1223.wang> writes: >> >> If we have to go this way, I think it’s better to make the change you >> suggested above, >> and then also update the documentation, both internal documentation on >> how to define >> the hook and the user level documentation on what the user might >> expect when using >> this option (i.e, it’s possible that the compiler might clear more >> registers than the user >> requests on some targets due to the implementation limitation). >> >> I can make this change if we decide to do this. > > I'd vote for this. Richard? Fine by me too, although I don't think this should be mentioned in the user documentation. E.g. used-arg means that non-argument, non-return registers can have any value on return from the function; the compiler doesn't make any guarantees. If the compiler happens to use a temporary register in the implementation of the option, and if that temporary register happens to still be zero on return, then that's OK. It's just an internal implementation detail. The same thing could happen for any part of the epilogue. Thanks, Richard
> On Mar 18, 2022, at 11:09 AM, Richard Sandiford <richard.sandiford@arm.com> wrote: > > Xi Ruoyao <xry111@mengyan1223.wang> writes: >>> >>> If we have to go this way, I think it’s better to make the change you >>> suggested above, >>> and then also update the documentation, both internal documentation on >>> how to define >>> the hook and the user level documentation on what the user might >>> expect when using >>> this option (i.e, it’s possible that the compiler might clear more >>> registers than the user >>> requests on some targets due to the implementation limitation). >>> >>> I can make this change if we decide to do this. >> >> I'd vote for this. Richard? > > Fine by me too, although I don't think this should be mentioned > in the user documentation. E.g. used-arg means that non-argument, > non-return registers can have any value on return from the function; > the compiler doesn't make any guarantees. If the compiler happens to > use a temporary register in the implementation of the option, and if > that temporary register happens to still be zero on return, then > that's OK. It's just an internal implementation detail. The same > thing could happen for any part of the epilogue. This makes good sense to me. I agree. Okay, will just add an extra argument and update the internal documentation. thanks. Qing > > Thanks, > Richard
On Sat, 12 Mar 2022, Xi Ruoyao via Gcc-patches wrote: > I'm now thinking: is there always at least one *GPR* which need to be > cleared? If it's true, let's say GPR $12, and fcc0 & fcc2 needs to be > cleared, we can use something like: > > cfc1 $12, $25 > andi $25, 5 > ctc1 $12, $25 > move $12, $0 There's always $1 ($at) available and we're in a function's epilogue, so there should be plenty of dead temporaries available as well. For legacy ISAs you'd need to use the FCSR instead ($31) and two temporaries would be required as the condition code bits are located in the upper half. FWIW, Maciej
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc index 4f9683e8bf4..f0d6c8f564c 100644 --- a/gcc/config/mips/mips.cc +++ b/gcc/config/mips/mips.cc @@ -22611,6 +22611,51 @@ mips_asm_file_end (void) if (NEED_INDICATE_EXEC_STACK) file_end_indicate_exec_stack (); } + +static HARD_REG_SET +mips_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs) +{ + HARD_REG_SET zeroed_hardregs; + CLEAR_HARD_REG_SET (zeroed_hardregs); + + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, HI_REGNUM)) + { + /* Clear HI and LO altogether. MIPS target treats HILO as a + double-word register. */ + machine_mode dword_mode = TARGET_64BIT ? TImode : DImode; + rtx hilo = gen_rtx_REG (dword_mode, MD_REG_FIRST); + rtx zero = CONST0_RTX (dword_mode); + emit_move_insn (hilo, zero); + + SET_HARD_REG_BIT (zeroed_hardregs, HI_REGNUM); + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, LO_REGNUM)) + SET_HARD_REG_BIT (zeroed_hardregs, LO_REGNUM); + else + emit_clobber (gen_rtx_REG (word_mode, LO_REGNUM)); + } + + bool zero_fcc = false; + for (int i = ST_REG_FIRST; i <= ST_REG_LAST; i++) + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, i)) + zero_fcc = true; + + /* MIPS does not have a simple way to clear one bit in FCC. We just + clear FCC with ctc1 and clobber all FCC bits. */ + if (zero_fcc) + { + emit_insn (gen_mips_zero_fcc ()); + for (int i = ST_REG_FIRST; i <= ST_REG_LAST; i++) + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, i)) + SET_HARD_REG_BIT (zeroed_hardregs, i); + else + emit_clobber (gen_rtx_REG (CCmode, i)); + } + + need_zeroed_hardregs &= ~zeroed_hardregs; + return zeroed_hardregs | + default_zero_call_used_regs (need_zeroed_hardregs); +} + /* Initialize the GCC target structure. */ #undef TARGET_ASM_ALIGNED_HI_OP @@ -22919,6 +22964,8 @@ mips_asm_file_end (void) #undef TARGET_ASM_FILE_END #define TARGET_ASM_FILE_END mips_asm_file_end +#undef TARGET_ZERO_CALL_USED_REGS +#define TARGET_ZERO_CALL_USED_REGS mips_zero_call_used_regs struct gcc_target targetm = TARGET_INITIALIZER; diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index e0f0a582732..edf58710cdd 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -96,6 +96,7 @@ (define_c_enum "unspec" [ ;; Floating-point environment. UNSPEC_GET_FCSR UNSPEC_SET_FCSR + UNSPEC_ZERO_FCC ;; HI/LO moves. UNSPEC_MFHI @@ -7670,6 +7671,11 @@ (define_insn "*mips_set_fcsr" "TARGET_HARD_FLOAT" "ctc1\t%0,$31") +(define_insn "mips_zero_fcc" + [(unspec_volatile [(const_int 0)] UNSPEC_ZERO_FCC)] + "TARGET_HARD_FLOAT" + "ctc1\t$0,$25") + ;; See tls_get_tp_mips16_<mode> for why this form is used. (define_insn "mips_set_fcsr_mips16_<mode>" [(unspec_volatile:SI [(match_operand:P 0 "call_insn_operand" "dS") diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c index 96e0b79b328..c23b2ceb391 100644 --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* nvptx*-*-* s390*-*-* } } } */ +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ /* { dg-options "-O2" } */ #include <assert.h> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c index 0714f95a04f..f51f5a2161c 100644 --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ /* { dg-options "-O2 -fzero-call-used-regs=all" } */ #include "zero-scratch-regs-10.c" diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c index aceda7e5cb8..3e5e59b3c79 100644 --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-8.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ /* { dg-options "-O2 -fzero-call-used-regs=all-arg" } */ #include "zero-scratch-regs-1.c" diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c b/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c index f3152a7a732..d88d61accb2 100644 --- a/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-9.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* } } } */ +/* { dg-skip-if "not implemented" { ! { i?86*-*-* x86_64*-*-* sparc*-*-* aarch64*-*-* arm*-*-* nvptx*-*-* s390*-*-* mips*-*-* } } } */ /* { dg-options "-O2 -fzero-call-used-regs=all" } */ #include "zero-scratch-regs-1.c"