Message ID | 20221118042706.10725-1-palmer@rivosinc.com |
---|---|
State | Committed |
Commit | df049cb2153839fdf6c6bdc27acf5a5151f74a99 |
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 88BB738432D6 for <patchwork@sourceware.org>; Fri, 18 Nov 2022 04:29:28 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) by sourceware.org (Postfix) with ESMTPS id 4ED15385223E for <gcc-patches@gcc.gnu.org>; Fri, 18 Nov 2022 04:29:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4ED15385223E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-pj1-x102f.google.com with SMTP id w15-20020a17090a380f00b0021873113cb4so3526315pjb.0 for <gcc-patches@gcc.gnu.org>; Thu, 17 Nov 2022 20:29:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.com; s=20210112; h=to:from:cc:content-transfer-encoding:mime-version:message-id:date :subject:from:to:cc:subject:date:message-id:reply-to; bh=sHFXelnsTHRpE/a/ck4dfU9tzcgt/HK2cUQnZxrIrZ0=; b=U/5DZQdFiy6EdexTIZ/tgrSrsH19HU4QoappjwrkSeKSXxic161aW1Tn9dQCxchHnp G4HAKEIWNUTfwOO7tCMrKryLn5h4aIWXdwWUiC0HyULck82aZEc1lv0sLlNv4Fz1cFqz DuzHHUuITQmB11zwI9IVX/50evsJUfxinVzPhq8DQ0yFKwy4e/iDbDj6yZ3Dqa/KHKFp UzWUfH6Yan62yUcJeF5BjACEsfCp3Psvsx7qm3gqXtfwAsHlz02VLAhYz4VQ2WurZnQe hi9QCTdWYUns1OizxVLF1ToJ+c5dnTSXcQFTNZyk+gUwtJ/S/TbRmwj+u0i00/aHmleP vHUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:from:cc:content-transfer-encoding:mime-version:message-id:date :subject:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=sHFXelnsTHRpE/a/ck4dfU9tzcgt/HK2cUQnZxrIrZ0=; b=tbUyspigBVo6TYxmQOzDY5AftoSDSAaCfZ0bmHJCS0vybXlDt2btXwNuSz7aU9bjg3 O70FRy4dV3uuiznc3+erb2D4PudsO2+n/1Pz2JadaE38mp5v72DVu+9RiS8SIfhqcbYy /Ek1mxrg/yWyas+0sjUJetV+hL0AHwCjsp4S3WAPAdMsJyn6dfODLo3n+Nfzc7eT/AX4 A1tm9JV6vpiPWu+Qvcsr4wGwvGllMY/aCrCcWhMO0xB2I318JAMZAKvu1+4U/xrFaILZ anoSEiajteZNqum6zPiFLigWi54Sgmbx1B7OE4+3Pn+nWGpmmWrYJcpv5GFHVgB6Cc9X z7Fg== X-Gm-Message-State: ANoB5pnip6W9lSmf1dqtb6/pyHJSAmC0pP9XZfanQfrMJf4KgbMXaOXz Te/Gna4YvWOrmycLcjpxypIbaA== X-Google-Smtp-Source: AA0mqf5fxPRSTGDWIFX1HTkr5fABu98KjMk+u2PAwJzrkosl8moDs0865Lvgqnu2HlTNpTHOUvgdFA== X-Received: by 2002:a17:903:3246:b0:188:f1b4:cf35 with SMTP id ji6-20020a170903324600b00188f1b4cf35mr5318707plb.54.1668745749141; Thu, 17 Nov 2022 20:29:09 -0800 (PST) Received: from localhost ([50.221.140.188]) by smtp.gmail.com with ESMTPSA id l6-20020a170903120600b0018157b415dbsm2415256plh.63.2022.11.17.20.29.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Nov 2022 20:29:08 -0800 (PST) Subject: [PATCH] RISC-V: Note that __builtin_riscv_pause() implies Xgnuzihintpausestate Date: Thu, 17 Nov 2022 20:27:06 -0800 Message-Id: <20221118042706.10725-1-palmer@rivosinc.com> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Cc: Palmer Dabbelt <palmer@rivosinc.com> From: Palmer Dabbelt <palmer@rivosinc.com> To: gcc-patches@gcc.gnu.org X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
RISC-V: Note that __builtin_riscv_pause() implies Xgnuzihintpausestate
|
|
Commit Message
Palmer Dabbelt
Nov. 18, 2022, 4:27 a.m. UTC
gcc/ChangeLog: * doc/extend.texi (__builtin_riscv_pause): Imply Xgnuzihintpausestate. --- gcc/doc/extend.texi | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Comments
Wait, what's Xgnuzihintpausestate??? On Fri, Nov 18, 2022 at 12:30 PM Palmer Dabbelt <palmer@rivosinc.com> wrote: > > gcc/ChangeLog: > > * doc/extend.texi (__builtin_riscv_pause): Imply > Xgnuzihintpausestate. > --- > gcc/doc/extend.texi | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index b1dd39e64b8..26f14e61bc8 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -21103,7 +21103,9 @@ Returns the value that is currently set in the @samp{tp} register. > @end deftypefn > > @deftypefn {Built-in Function} void __builtin_riscv_pause (void) > -Generates the @code{pause} (hint) machine instruction. > +Generates the @code{pause} (hint) machine instruction. This implies the > +Xgnuzihintpausestate extension, which redefines the @code{pause} instruction to > +change architectural state. > @end deftypefn > > @node RX Built-in Functions > -- > 2.38.1 >
On Thu, 17 Nov 2022 22:59:08 PST (-0800), Kito Cheng wrote: > Wait, what's Xgnuzihintpausestate??? I just made it up, it's defined right next to the name like those profile extensions are. I figured that's the most RISC-V way to define something like this, but we could just drop it and run with the definition -- IIRC we just stuck a comment in for Linux and QEMU, I doubt anyone is actually going to implement the "doesn't touch PC" version of pause. > On Fri, Nov 18, 2022 at 12:30 PM Palmer Dabbelt <palmer@rivosinc.com> wrote: >> >> gcc/ChangeLog: >> >> * doc/extend.texi (__builtin_riscv_pause): Imply >> Xgnuzihintpausestate. >> --- >> gcc/doc/extend.texi | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >> index b1dd39e64b8..26f14e61bc8 100644 >> --- a/gcc/doc/extend.texi >> +++ b/gcc/doc/extend.texi >> @@ -21103,7 +21103,9 @@ Returns the value that is currently set in the @samp{tp} register. >> @end deftypefn >> >> @deftypefn {Built-in Function} void __builtin_riscv_pause (void) >> -Generates the @code{pause} (hint) machine instruction. >> +Generates the @code{pause} (hint) machine instruction. This implies the >> +Xgnuzihintpausestate extension, which redefines the @code{pause} instruction to >> +change architectural state. >> @end deftypefn >> >> @node RX Built-in Functions >> -- >> 2.38.1 >>
On Fri, 18 Nov 2022 09:01:08 PST (-0800), Palmer Dabbelt wrote: > On Thu, 17 Nov 2022 22:59:08 PST (-0800), Kito Cheng wrote: >> Wait, what's Xgnuzihintpausestate??? > > I just made it up, it's defined right next to the name like those > profile extensions are. I figured that's the most RISC-V way to define > something like this, but we could just drop it and run with the > definition -- IIRC we just stuck a comment in for Linux and QEMU, I > doubt anyone is actually going to implement the "doesn't touch PC" > version of pause. Just checking up on this one. I don't care a ton about the name, just that we document where we're intentionally violating the specs. > >> On Fri, Nov 18, 2022 at 12:30 PM Palmer Dabbelt <palmer@rivosinc.com> wrote: >>> >>> gcc/ChangeLog: >>> >>> * doc/extend.texi (__builtin_riscv_pause): Imply >>> Xgnuzihintpausestate. >>> --- >>> gcc/doc/extend.texi | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >>> index b1dd39e64b8..26f14e61bc8 100644 >>> --- a/gcc/doc/extend.texi >>> +++ b/gcc/doc/extend.texi >>> @@ -21103,7 +21103,9 @@ Returns the value that is currently set in the @samp{tp} register. >>> @end deftypefn >>> >>> @deftypefn {Built-in Function} void __builtin_riscv_pause (void) >>> -Generates the @code{pause} (hint) machine instruction. >>> +Generates the @code{pause} (hint) machine instruction. This implies the >>> +Xgnuzihintpausestate extension, which redefines the @code{pause} instruction to >>> +change architectural state. >>> @end deftypefn >>> >>> @node RX Built-in Functions >>> -- >>> 2.38.1 >>>
On Mon, 28 Nov 2022 10:45:51 PST (-0800), Palmer Dabbelt wrote: > On Fri, 18 Nov 2022 09:01:08 PST (-0800), Palmer Dabbelt wrote: >> On Thu, 17 Nov 2022 22:59:08 PST (-0800), Kito Cheng wrote: >>> Wait, what's Xgnuzihintpausestate??? >> >> I just made it up, it's defined right next to the name like those >> profile extensions are. I figured that's the most RISC-V way to define >> something like this, but we could just drop it and run with the >> definition -- IIRC we just stuck a comment in for Linux and QEMU, I >> doubt anyone is actually going to implement the "doesn't touch PC" >> version of pause. > > Just checking up on this one. I don't care a ton about the name, just > that we document where we're intentionally violating the specs. I'm just committing this one, no big deal if you want to change the wording. I just want it out of my queue. > >> >>> On Fri, Nov 18, 2022 at 12:30 PM Palmer Dabbelt <palmer@rivosinc.com> wrote: >>>> >>>> gcc/ChangeLog: >>>> >>>> * doc/extend.texi (__builtin_riscv_pause): Imply >>>> Xgnuzihintpausestate. >>>> --- >>>> gcc/doc/extend.texi | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >>>> index b1dd39e64b8..26f14e61bc8 100644 >>>> --- a/gcc/doc/extend.texi >>>> +++ b/gcc/doc/extend.texi >>>> @@ -21103,7 +21103,9 @@ Returns the value that is currently set in the @samp{tp} register. >>>> @end deftypefn >>>> >>>> @deftypefn {Built-in Function} void __builtin_riscv_pause (void) >>>> -Generates the @code{pause} (hint) machine instruction. >>>> +Generates the @code{pause} (hint) machine instruction. This implies the >>>> +Xgnuzihintpausestate extension, which redefines the @code{pause} instruction to >>>> +change architectural state. >>>> @end deftypefn >>>> >>>> @node RX Built-in Functions >>>> -- >>>> 2.38.1 >>>>
It took me a few minutes to understand the purpose of this chicanery, but there's indeed a contradiction in the ISA spec. HINT instructions _do_ affect architectural state in a limited fashion--namely, updating the PC. So, it's incorrect to say that PAUSE changes no architectural state. Because these statements are contradictory, a common-sense reading should parse this as "PAUSE changes no architectural state in the same informal sense as other HINTs". Otherwise, PAUSE wouldn't actually be a HINT. I'm just going to delete the erroneous text. This eliminates the contradiction and makes the spec consistent with both the de facto and de jure golden models, which behave in the common-sense manner Palmer's Xgnuzihintpausestate extension would suggest. To avoid confusion, I strongly suggest deleting all references to Xgnuzihintpausestate, since its existence invites a question that no longer needs to be answered. cc'ing Greg since AFAICS he merged in the erroneous text. On Fri, Dec 16, 2022 at 8:48 AM Palmer Dabbelt <palmer@rivosinc.com> wrote: > On Mon, 28 Nov 2022 10:45:51 PST (-0800), Palmer Dabbelt wrote: > > On Fri, 18 Nov 2022 09:01:08 PST (-0800), Palmer Dabbelt wrote: > >> On Thu, 17 Nov 2022 22:59:08 PST (-0800), Kito Cheng wrote: > >>> Wait, what's Xgnuzihintpausestate??? > >> > >> I just made it up, it's defined right next to the name like those > >> profile extensions are. I figured that's the most RISC-V way to define > >> something like this, but we could just drop it and run with the > >> definition -- IIRC we just stuck a comment in for Linux and QEMU, I > >> doubt anyone is actually going to implement the "doesn't touch PC" > >> version of pause. > > > > Just checking up on this one. I don't care a ton about the name, just > > that we document where we're intentionally violating the specs. > > I'm just committing this one, no big deal if you want to change the > wording. I just want it out of my queue. > > > > >> > >>> On Fri, Nov 18, 2022 at 12:30 PM Palmer Dabbelt <palmer@rivosinc.com> > wrote: > >>>> > >>>> gcc/ChangeLog: > >>>> > >>>> * doc/extend.texi (__builtin_riscv_pause): Imply > >>>> Xgnuzihintpausestate. > >>>> --- > >>>> gcc/doc/extend.texi | 4 +++- > >>>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > >>>> index b1dd39e64b8..26f14e61bc8 100644 > >>>> --- a/gcc/doc/extend.texi > >>>> +++ b/gcc/doc/extend.texi > >>>> @@ -21103,7 +21103,9 @@ Returns the value that is currently set in > the @samp{tp} register. > >>>> @end deftypefn > >>>> > >>>> @deftypefn {Built-in Function} void __builtin_riscv_pause (void) > >>>> -Generates the @code{pause} (hint) machine instruction. > >>>> +Generates the @code{pause} (hint) machine instruction. This implies > the > >>>> +Xgnuzihintpausestate extension, which redefines the @code{pause} > instruction to > >>>> +change architectural state. > >>>> @end deftypefn > >>>> > >>>> @node RX Built-in Functions > >>>> -- > >>>> 2.38.1 > >>>> >
On Dez 17 2022, Andrew Waterman wrote: > It took me a few minutes to understand the purpose of this chicanery, but > there's indeed a contradiction in the ISA spec. HINT instructions _do_ > affect architectural state in a limited fashion--namely, updating the PC. How can an insn _not_ affect the PC? (Other than the trivial infinite loop.)
On Sat, Dec 17, 2022 at 2:10 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Dez 17 2022, Andrew Waterman wrote: > > > It took me a few minutes to understand the purpose of this chicanery, but > > there's indeed a contradiction in the ISA spec. HINT instructions _do_ > > affect architectural state in a limited fashion--namely, updating the PC. > > How can an insn _not_ affect the PC? (Other than the trivial infinite > loop.) Heh, yeah, that's roughly what I meant by "common-sense reading" (and that's my rationale for simply clarifying the spec and nuking this Xgnuzihintpausestate extension). > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different."
On Dez 17 2022, Andrew Waterman wrote: > On Sat, Dec 17, 2022 at 2:10 AM Andreas Schwab <schwab@linux-m68k.org> wrote: >> >> On Dez 17 2022, Andrew Waterman wrote: >> >> > It took me a few minutes to understand the purpose of this chicanery, but >> > there's indeed a contradiction in the ISA spec. HINT instructions _do_ >> > affect architectural state in a limited fashion--namely, updating the PC. >> >> How can an insn _not_ affect the PC? (Other than the trivial infinite >> loop.) > > Heh, yeah, that's roughly what I meant by "common-sense reading" (and > that's my rationale for simply clarifying the spec and nuking this > Xgnuzihintpausestate extension). My point is that the implicit update of the PC cannot be part of the architectural state in the first place. Even the trivial infinite loop has this, before the actual state change (setting PC back) is performed.
On Sat, Dec 17, 2022 at 2:21 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Dez 17 2022, Andrew Waterman wrote: > > > On Sat, Dec 17, 2022 at 2:10 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > >> > >> On Dez 17 2022, Andrew Waterman wrote: > >> > >> > It took me a few minutes to understand the purpose of this chicanery, but > >> > there's indeed a contradiction in the ISA spec. HINT instructions _do_ > >> > affect architectural state in a limited fashion--namely, updating the PC. > >> > >> How can an insn _not_ affect the PC? (Other than the trivial infinite > >> loop.) > > > > Heh, yeah, that's roughly what I meant by "common-sense reading" (and > > that's my rationale for simply clarifying the spec and nuking this > > Xgnuzihintpausestate extension). > > My point is that the implicit update of the PC cannot be part of the > architectural state in the first place. Even the trivial infinite loop > has this, before the actual state change (setting PC back) is performed. It's just a definitional issue. By analogy, this is why we have the concept of "explicit memory access" (the thing a load or store is trying to do) and "implicit memory access" (all of the other memory accesses, like the instruction fetch or page-table walk). The PC update is very much an architectural-state change, but it would be fair to call it an "implicit architectural-state change" to contrast with e.g. writing an x-register being an "explicit architectural state change". Anyway, I don't think we're disagreeing with each other (and I still think there's no problem to be solved here). > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different."
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index b1dd39e64b8..26f14e61bc8 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -21103,7 +21103,9 @@ Returns the value that is currently set in the @samp{tp} register. @end deftypefn @deftypefn {Built-in Function} void __builtin_riscv_pause (void) -Generates the @code{pause} (hint) machine instruction. +Generates the @code{pause} (hint) machine instruction. This implies the +Xgnuzihintpausestate extension, which redefines the @code{pause} instruction to +change architectural state. @end deftypefn @node RX Built-in Functions