Message ID | alpine.DEB.2.20.2201201459500.11348@tpp.orcam.me.uk |
---|---|
State | Deferred, archived |
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 A14543857405 for <patchwork@sourceware.org>; Thu, 20 Jan 2022 15:44:46 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com [IPv6:2a00:1450:4864:20::12e]) by sourceware.org (Postfix) with ESMTPS id 3343A3858D37 for <gcc-patches@gcc.gnu.org>; Thu, 20 Jan 2022 15:44:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3343A3858D37 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-lf1-x12e.google.com with SMTP id m3so23435707lfu.0 for <gcc-patches@gcc.gnu.org>; Thu, 20 Jan 2022 07:44:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:user-agent:mime-version; bh=lWpEE5W+F3MqUe3AzjXu2GagaMWzReO8yPZCd1/JnOk=; b=ZG9o782SBKJbE3ogJkuzDX1Lj+GoyhBDVlD4xHLaG3X1PG5VjYNft1dqJGAE+yOuK/ 2z1gXPDRVV9FbHjxTznbLUu+JRh4nlUxiixY+82sax1WKAqqJhUD2zsWBkHn7enxlh/T kwnliyUVUQCh+YI+w4qiNkDP36HgEErvqT//9YrvtulT/moWAzHpUqfFxgIr2DTnvZfG 8uJTh7gjsZaFkw3+UuWDIlJ64KKgS45vaa/5UWrV/offw8EYdXCfzUpbrROM+o/UG6pN m+GHN4T3jS+CwG7BxwrGUHKLUmKok9PvC/73n/b8XKC8ZyzXv/xZhoxQsfzWK3ksW0HG F5jg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:user-agent :mime-version; bh=lWpEE5W+F3MqUe3AzjXu2GagaMWzReO8yPZCd1/JnOk=; b=FN9kP+hx8IFthcXFFUbhrLOSyHcHKvTOjhhIx4n4teFdRlsvEPOD/iY6g14U4tBrgx 01ZyGPb3AaYZksY9ZAbkOxaomtjGAYUDXArLcPVhwy5d5VL7dNwU2jpcPLw697OkhOq+ BtnedyetQT5E1WjxouAPC4GKDFjxPmIuTs66fIMzy3lbaeln+yMiFnM7TtVYt46gibmh mLPFkgrCeyvTmOosi0m1GPquCxceByV4SGBw45mEr0YHyuiX08ElTZbnfoHGeU1QqmDX /5oGFQt8qtKzawVn3ZXPAj0AKs+th7B2BcQ5HCHRJvA9lxhPuLczR6vH+DsM8xEG5r2i a35Q== X-Gm-Message-State: AOAM533h3yyX6dg4rez8DetLqIniNw+QJFg8n3NL5Y4YMGHGUV3JkDW2 2JIUrx3uBsZ0hauO9qgMHi95ng2IPcZCD3Rh X-Google-Smtp-Source: ABdhPJxnNg7KFH+6QvkVW2gWBj8bWEAQdy49izugxYSQSUueizmvJFCZ6iNWHFsRO7awuPKxGCkhJw== X-Received: by 2002:a05:651c:a0f:: with SMTP id k15mr12064797ljq.451.1642693468776; Thu, 20 Jan 2022 07:44:28 -0800 (PST) Received: from [192.168.219.3] ([78.8.192.131]) by smtp.gmail.com with ESMTPSA id q3sm394431lfc.81.2022.01.20.07.44.27 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Jan 2022 07:44:28 -0800 (PST) Date: Thu, 20 Jan 2022 15:44:25 +0000 (GMT) From: "Maciej W. Rozycki" <macro@embecosm.com> To: gcc-patches@gcc.gnu.org Subject: [PATCH][GCC13?] RISC-V: Replace `smin'/`smax' RTL patterns with `fmin'/`fmax' Message-ID: <alpine.DEB.2.20.2201201459500.11348@tpp.orcam.me.uk> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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> Cc: Kito Cheng <kito.cheng@gmail.com>, Andrew Waterman <andrew@sifive.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 |
[GCC13?] RISC-V: Replace `smin'/`smax' RTL patterns with `fmin'/`fmax'
|
|
Commit Message
Maciej W. Rozycki
Jan. 20, 2022, 3:44 p.m. UTC
RISC-V FMIN and FMAX machine instructions are IEEE-754-conformant[1]: "For FMIN and FMAX, if at least one input is a signaling NaN, or if both inputs are quiet NaNs, the result is the canonical NaN. If one operand is a quiet NaN and the other is not a NaN, the result is the non-NaN operand." as required by our `fminM3' and `fmaxM3' standard RTL patterns. However we only define `sminM3' and `smaxM3' standard RTL patterns to produce the FMIN and FMAX machine instructions, which in turn causes the `__builtin_fmin' and `__builtin_fmax' family of intrinsics to emit the corresponding libcalls rather than the relevant machine instructions. Rename the `smin<mode>3' and `smax<mode>3' patterns to `fmin<mode>3' and `fmax<mode>3' respectively then, removing the need to use libcalls for IEEE 754 semantics with the minimum and maximum operations. [1] "The RISC-V Instruction Set Manual, Volume I: User-Level ISA", Document Version 2.2, May 7, 2017, Section 8.3 "NaN Generation and Propagation", p. 48 gcc/ * config/riscv/riscv.md (smin<mode>3): Rename pattern to... (fmin<mode>3): ... this. (smax<mode>3): Likewise... (fmax<mode>3): ... this. --- Hi, It's not clear to me how it's been missed or whether there is anything I might be actually missing. It looks to me like a clear oversight however. And in any case this change has passed full GCC regression testing (except for the D frontend, which has stopped being built recently due to a defect in Debian I haven't yet got to getting fixed) with the `riscv64-linux-gnu' target using the HiFive Unmatched (U74 CPU) target board, so it seems to be doing the right thing. Timing might a bit unfortunate for this submission and given that it is not a regression fix I guess this is GCC 13 material. Please let me know otherwise. In any case OK to apply (when the time comes)? Maciej --- gcc/config/riscv/riscv.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) gcc-riscv-fmin-fmax.diff
Comments
On Thu, 20 Jan 2022 07:44:25 PST (-0800), macro@embecosm.com wrote: > RISC-V FMIN and FMAX machine instructions are IEEE-754-conformant[1]: > > "For FMIN and FMAX, if at least one input is a signaling NaN, or if both > inputs are quiet NaNs, the result is the canonical NaN. If one operand > is a quiet NaN and the other is not a NaN, the result is the non-NaN > operand." > > as required by our `fminM3' and `fmaxM3' standard RTL patterns. > > However we only define `sminM3' and `smaxM3' standard RTL patterns to > produce the FMIN and FMAX machine instructions, which in turn causes the > `__builtin_fmin' and `__builtin_fmax' family of intrinsics to emit the > corresponding libcalls rather than the relevant machine instructions. > > Rename the `smin<mode>3' and `smax<mode>3' patterns to `fmin<mode>3' and > `fmax<mode>3' respectively then, removing the need to use libcalls for > IEEE 754 semantics with the minimum and maximum operations. > > [1] "The RISC-V Instruction Set Manual, Volume I: User-Level ISA", > Document Version 2.2, May 7, 2017, Section 8.3 "NaN Generation and > Propagation", p. 48 > > gcc/ > * config/riscv/riscv.md (smin<mode>3): Rename pattern to... > (fmin<mode>3): ... this. > (smax<mode>3): Likewise... > (fmax<mode>3): ... this. > --- > Hi, > > It's not clear to me how it's been missed or whether there is anything I > might be actually missing. It looks to me like a clear oversight however. I'm not really a floating point person, but IIUC It's actually on purpose: earlier versions of the ISA spec didn't have this behavior, and at the time we originally merged the GCC port we decided to play it safe. Pretty sure we discussed this before on the GCC mailing list ,maybe around the time the glibc port was going upstream? I think Jim was the one who figured out how all the specs fit together. I can't find those older discussions, but this definately recently came up in glibc: https://sourceware.org/pipermail/libc-alpha/2021-October/131637.html . Looks like back then nobody knew of any hardware that ran glibc and implemented the old behavior, but there also haven't been patches posted yet so it's not set in stone. It's probably worth repeating the question here since there are a lot of RISC-V users that don't use glibc but do use GCC. I don't know of anyone who implemented the old floating point standards off the top of my head, even in embedded land, but I'm pretty lost when it comes to ISA versioning these days so I might be missing something. One option could be to tie this to the ISA spec version and emit the required emulation routines, but I don't think that's worth bothering to do unless someone knows of an implementation that implements the old behavior. > And in any case this change has passed full GCC regression testing (except > for the D frontend, which has stopped being built recently due to a defect > in Debian I haven't yet got to getting fixed) with the `riscv64-linux-gnu' > target using the HiFive Unmatched (U74 CPU) target board, so it seems to > be doing the right thing. > > Timing might a bit unfortunate for this submission and given that it is > not a regression fix I guess this is GCC 13 material. Please let me know > otherwise. > > In any case OK to apply (when the time comes)? IMO waiting is the right way to go, as if this does uncover any issues they'll be a long-tail sort of thing. That way we'll at least have a whole release cycle for folks to test on their hardware, which is about as good as we can do here. Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # for 13 Someone should probably do the glibc version, too ;) > > Maciej > --- > gcc/config/riscv/riscv.md | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > gcc-riscv-fmin-fmax.diff > Index: gcc/gcc/config/riscv/riscv.md > =================================================================== > --- gcc.orig/gcc/config/riscv/riscv.md > +++ gcc/gcc/config/riscv/riscv.md > @@ -1214,7 +1214,7 @@ > ;; > ;; .................... > > -(define_insn "smin<mode>3" > +(define_insn "fmin<mode>3" > [(set (match_operand:ANYF 0 "register_operand" "=f") > (smin:ANYF (match_operand:ANYF 1 "register_operand" " f") > (match_operand:ANYF 2 "register_operand" " f")))] > @@ -1223,7 +1223,7 @@ > [(set_attr "type" "fmove") > (set_attr "mode" "<UNITMODE>")]) > > -(define_insn "smax<mode>3" > +(define_insn "fmax<mode>3" > [(set (match_operand:ANYF 0 "register_operand" "=f") > (smax:ANYF (match_operand:ANYF 1 "register_operand" " f") > (match_operand:ANYF 2 "register_operand" " f")))]
On Thu, 20 Jan 2022, Maciej W. Rozycki wrote: > RISC-V FMIN and FMAX machine instructions are IEEE-754-conformant[1]: > > "For FMIN and FMAX, if at least one input is a signaling NaN, or if both > inputs are quiet NaNs, the result is the canonical NaN. If one operand > is a quiet NaN and the other is not a NaN, the result is the non-NaN > operand." > > as required by our `fminM3' and `fmaxM3' standard RTL patterns. > > However we only define `sminM3' and `smaxM3' standard RTL patterns to > produce the FMIN and FMAX machine instructions, which in turn causes the > `__builtin_fmin' and `__builtin_fmax' family of intrinsics to emit the > corresponding libcalls rather than the relevant machine instructions. > > Rename the `smin<mode>3' and `smax<mode>3' patterns to `fmin<mode>3' and > `fmax<mode>3' respectively then, removing the need to use libcalls for > IEEE 754 semantics with the minimum and maximum operations. > > [1] "The RISC-V Instruction Set Manual, Volume I: User-Level ISA", > Document Version 2.2, May 7, 2017, Section 8.3 "NaN Generation and > Propagation", p. 48 That's an old version of the instruction set, and an old version of IEEE 754. The C functions fmin and fmax correspond to the IEEE 754-2008 operations minNum and maxNum, which operate as described, and the RISC-V 'F' and 'D' extensions *before* version 2.2 of those extensions also corresponded to minNum and maxNum. IEEE 754-2019 removes minNum and maxNum because they are non-associative in the presence of signaling NaNs, replacing them with new operations minimum, minimumNumber, maximum, maximumNumber. C23 defines new functions fminimum, fminimum_num, fmaximum, fmaximum_num corresponding to those new operations, leaving fmin and fmax unchanged. And the RISC-V 'F' and 'D' extensions version 2.2 change the FMIN and FMAX instructions to correspond to minimumNumber and maximumNumber instead of minNum and maxNum. So, if generating code that might be run on processors implementing versions of 'F' and 'D' older than 2.2, it's not safe to generate FMAX and FMIN instructions for any of those standard C library functions when signaling NaN operands are a possibility (i.e. flag_signaling_nans is set), because code built for older versions might run on newer versions with changed instruction semantics. If generating code that requires version 2.2 or later of 'F' and 'D' (and I don't know if GCC actually supports generating code for older versions), it's OK to generate those instructions as part of expanding calls to the C23 functions fminimum_num and fmaximum_num - but not as part of expanding calls to fmin and fmax (unless !flag_signaling_nans, in which case the differences between those functions aren't relevant). And GCC currently doesn't have built-in functions for fminimum_num and fmaximum_num, and I don't think it has insn patterns corresponding to those functions either.
On Thu, 20 Jan 2022, Joseph Myers wrote: > The C functions fmin and fmax correspond to the IEEE 754-2008 operations > minNum and maxNum, which operate as described, and the RISC-V 'F' and 'D' > extensions *before* version 2.2 of those extensions also corresponded to > minNum and maxNum. And, to make things clear for people confused by the different version numbers involved: RISC-V ISA version 2.2 contains 'F' and 'D' extensions version 2.0. It's version 2.2 of the extensions, *not* of the ISA, that changed the instructions from minNum and maxNum to minimumNumber and maximumNumber.
On Thu, Jan 20, 2022 at 12:30 PM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Thu, 20 Jan 2022 07:44:25 PST (-0800), macro@embecosm.com wrote: > > RISC-V FMIN and FMAX machine instructions are IEEE-754-conformant[1]: > > > > "For FMIN and FMAX, if at least one input is a signaling NaN, or if both > > inputs are quiet NaNs, the result is the canonical NaN. If one operand > > is a quiet NaN and the other is not a NaN, the result is the non-NaN > > operand." > > > > as required by our `fminM3' and `fmaxM3' standard RTL patterns. > > > > However we only define `sminM3' and `smaxM3' standard RTL patterns to > > produce the FMIN and FMAX machine instructions, which in turn causes the > > `__builtin_fmin' and `__builtin_fmax' family of intrinsics to emit the > > corresponding libcalls rather than the relevant machine instructions. > > > > Rename the `smin<mode>3' and `smax<mode>3' patterns to `fmin<mode>3' and > > `fmax<mode>3' respectively then, removing the need to use libcalls for > > IEEE 754 semantics with the minimum and maximum operations. > > > > [1] "The RISC-V Instruction Set Manual, Volume I: User-Level ISA", > > Document Version 2.2, May 7, 2017, Section 8.3 "NaN Generation and > > Propagation", p. 48 > > > > gcc/ > > * config/riscv/riscv.md (smin<mode>3): Rename pattern to... > > (fmin<mode>3): ... this. > > (smax<mode>3): Likewise... > > (fmax<mode>3): ... this. > > --- > > Hi, > > > > It's not clear to me how it's been missed or whether there is anything I > > might be actually missing. It looks to me like a clear oversight however. > > I'm not really a floating point person, but IIUC It's actually on > purpose: earlier versions of the ISA spec didn't have this behavior, and > at the time we originally merged the GCC port we decided to play it > safe. Pretty sure we discussed this before on the GCC mailing list > ,maybe around the time the glibc port was going upstream? I think Jim > was the one who figured out how all the specs fit together. > > I can't find those older discussions, but this definately recently came > up in glibc: > https://sourceware.org/pipermail/libc-alpha/2021-October/131637.html . > Looks like back then nobody knew of any hardware that ran glibc and > implemented the old behavior, but there also haven't been patches posted > yet so it's not set in stone. > > It's probably worth repeating the question here since there are a lot of > RISC-V users that don't use glibc but do use GCC. I don't know of > anyone who implemented the old floating point standards off the top of > my head, even in embedded land, but I'm pretty lost when it comes to ISA > versioning these days so I might be missing something. > > One option could be to tie this to the ISA spec version and emit the > required emulation routines, but I don't think that's worth bothering to > do unless someone knows of an implementation that implements the old > behavior. The old formulation of the instructions were never ratified as a RISC-V standard. I don't think we need to hamstring ourselves here by assuming the possibility of their implementation. > > > And in any case this change has passed full GCC regression testing (except > > for the D frontend, which has stopped being built recently due to a defect > > in Debian I haven't yet got to getting fixed) with the `riscv64-linux-gnu' > > target using the HiFive Unmatched (U74 CPU) target board, so it seems to > > be doing the right thing. > > > > Timing might a bit unfortunate for this submission and given that it is > > not a regression fix I guess this is GCC 13 material. Please let me know > > otherwise. > > > > In any case OK to apply (when the time comes)? > > IMO waiting is the right way to go, as if this does uncover any issues > they'll be a long-tail sort of thing. That way we'll at least have a > whole release cycle for folks to test on their hardware, which is about > as good as we can do here. > > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # for 13 > > Someone should probably do the glibc version, too ;) > > > > > Maciej > > --- > > gcc/config/riscv/riscv.md | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > gcc-riscv-fmin-fmax.diff > > Index: gcc/gcc/config/riscv/riscv.md > > =================================================================== > > --- gcc.orig/gcc/config/riscv/riscv.md > > +++ gcc/gcc/config/riscv/riscv.md > > @@ -1214,7 +1214,7 @@ > > ;; > > ;; .................... > > > > -(define_insn "smin<mode>3" > > +(define_insn "fmin<mode>3" > > [(set (match_operand:ANYF 0 "register_operand" "=f") > > (smin:ANYF (match_operand:ANYF 1 "register_operand" " f") > > (match_operand:ANYF 2 "register_operand" " f")))] > > @@ -1223,7 +1223,7 @@ > > [(set_attr "type" "fmove") > > (set_attr "mode" "<UNITMODE>")]) > > > > -(define_insn "smax<mode>3" > > +(define_insn "fmax<mode>3" > > [(set (match_operand:ANYF 0 "register_operand" "=f") > > (smax:ANYF (match_operand:ANYF 1 "register_operand" " f") > > (match_operand:ANYF 2 "register_operand" " f")))]
On Thu, 20 Jan 2022, Andrew Waterman wrote: > The old formulation of the instructions were never ratified as a > RISC-V standard. I don't think we need to hamstring ourselves here by > assuming the possibility of their implementation. If you ignore the old version, then the instructions can unconditionally be used for the fminimum_num / fmaximum_num functions (which GCC doesn't support as built-in functions at present) - but can't be used for the fmin / fmax functions if flag_signaling_nans.
On Thu, 20 Jan 2022, Joseph Myers wrote: > > The old formulation of the instructions were never ratified as a > > RISC-V standard. I don't think we need to hamstring ourselves here by > > assuming the possibility of their implementation. > > If you ignore the old version, then the instructions can unconditionally > be used for the fminimum_num / fmaximum_num functions (which GCC doesn't > support as built-in functions at present) - but can't be used for the fmin > / fmax functions if flag_signaling_nans. Fair enough, thank you for your detailed explanation of the requirements set by the individual revisions of the relevant standards, very valuable. I think we have consensus that we can ignore pre-r2.2 hardware. What we actually support is `-misa-spec=<2.2|20190608|20191213>', so we can assume r2.2 semantics as the absolute minimum, matching the description in my submission. Also GCC documents IEEE 754-2008 semantics for the `fmin' and `fmax' RTL operations: "'fminM3', 'fmaxM3' IEEE-conformant minimum and maximum operations. If one operand is a quiet 'NaN', then the other operand is returned. If both operands are quiet 'NaN', then a quiet 'NaN' is returned. In the case when gcc supports signaling 'NaN' (-fsignaling-nans) an invalid floating point exception is raised and a quiet 'NaN' is returned." and I think they need to stay as they are for the sake of IEEE 754-2008 hardware and the `minNum' and `maxNum' (`fmin'/`fmax') operations. Looking further r20190608 of the RISC-V ISA specification then has[1]: "Defined the signed-zero behavior of FMIN.fmt and FMAX.fmt, and changed their behavior on signaling-NaN inputs to conform to the minimumNumber and maximumNumber operations in the proposed IEEE 754-201x specification." and specifically[2]: "Floating-point minimum-number and maximum-number instructions FMIN.S and FMAX.S write, respectively, the smaller or larger of rs1 and rs2 to rd. For the purposes of these instructions only, the value -0.0 is considered to be less than the value +0.0. If both inputs are NaNs, the result is the canonical NaN. If only one operand is a NaN, the result is the non-NaN operand. Signaling NaN inputs set the invalid operation exception flag, even when the result is not NaN." So for forwards compatibility of software built for r2.2 (such that it continues producing correct results with r20190608+ hardware) I think we need to provide `fmin'/`fmax' insns iff !HONOR_SNANS while keeping `smin'/`smax' insns intact. Then once we have IEEE 754-2019 support in place, which will require new RTL standard pattern names, say `fminimum'/`fmaximum', we will provide them iff (!HONOR_SNANS || riscv_isa_spec >= ISA_SPEC_CLASS_20190608). It may be a good idea to start adding IEEE 754-2019 support, including the relevant `__builtin_fminimum' and `__builtin_fmaximum' intrinsics, once the GCC 13 development cycle has started. I'll post v2 then according to this consideration, once it's gone through regression testing. Thanks for your input again. References: [1] "The RISC-V Instruction Set Manual, Volume I: Unprivileged ISA", Document Version 20190608-Base-Ratified, June 8, 2019, "Preface", p. ii [2] same, Section 11.6 "Single-Precision Floating-Point Computational Instructions", p. 66 Maciej
On Sat, 22 Jan 2022, Maciej W. Rozycki wrote: > I think we have consensus that we can ignore pre-r2.2 hardware. What we > actually support is `-misa-spec=<2.2|20190608|20191213>', so we can assume > r2.2 semantics as the absolute minimum, matching the description in my > submission. Where, to repeat the point about possibly confusing version numbers, that's saying we can ignore hardware from before *ISA* revision 2.2 (which contained 'F' and 'D' extension version 2.0) - not that we can ignore hardware from before 'F' and 'D' extension version 2.2 (which changed the semantics of the FMIN and FMAX instructions). > Then once we have IEEE 754-2019 support in place, which will require new > RTL standard pattern names, say `fminimum'/`fmaximum', we will provide > them iff (!HONOR_SNANS || riscv_isa_spec >= ISA_SPEC_CLASS_20190608). It > may be a good idea to start adding IEEE 754-2019 support, including the > relevant `__builtin_fminimum' and `__builtin_fmaximum' intrinsics, once > the GCC 13 development cycle has started. The newer instruction semantics correspond to the functions fminimum_num and fmaximum_num, not fminimum and fmaximum (which are functions that treat both quiet and signaling NaNs like most libm functions do - any argument a NaN means the result is NaN).
On Mon, 24 Jan 2022, Joseph Myers wrote: > > I think we have consensus that we can ignore pre-r2.2 hardware. What we > > actually support is `-misa-spec=<2.2|20190608|20191213>', so we can assume > > r2.2 semantics as the absolute minimum, matching the description in my > > submission. > > Where, to repeat the point about possibly confusing version numbers, > that's saying we can ignore hardware from before *ISA* revision 2.2 (which > contained 'F' and 'D' extension version 2.0) - not that we can ignore > hardware from before 'F' and 'D' extension version 2.2 (which changed the > semantics of the FMIN and FMAX instructions). OK, I'll try to make it clear in the change description of v2. > > Then once we have IEEE 754-2019 support in place, which will require new > > RTL standard pattern names, say `fminimum'/`fmaximum', we will provide > > them iff (!HONOR_SNANS || riscv_isa_spec >= ISA_SPEC_CLASS_20190608). It > > may be a good idea to start adding IEEE 754-2019 support, including the > > relevant `__builtin_fminimum' and `__builtin_fmaximum' intrinsics, once > > the GCC 13 development cycle has started. > > The newer instruction semantics correspond to the functions fminimum_num > and fmaximum_num, not fminimum and fmaximum (which are functions that > treat both quiet and signaling NaNs like most libm functions do - any > argument a NaN means the result is NaN). I got these wrong then, sorry. I got lost too: what is the difference between `fmin'/`fmax' and `fminimum'/`fmaximum' then? It looks to me like we have a zoo of selection functions now with very subtle differences between each other. Maciej
On Wed, 26 Jan 2022, Maciej W. Rozycki wrote: > > The newer instruction semantics correspond to the functions fminimum_num > > and fmaximum_num, not fminimum and fmaximum (which are functions that > > treat both quiet and signaling NaNs like most libm functions do - any > > argument a NaN means the result is NaN). > > I got these wrong then, sorry. I got lost too: what is the difference > between `fmin'/`fmax' and `fminimum'/`fmaximum' then? It looks to me like > we have a zoo of selection functions now with very subtle differences > between each other. fmin and fmax: * Treat quiet NaN as missing data and return the other argument (if a number). * Treat signaling NaN like most functions (raise invalid, return quiet NaN). fminimum and fmaximum: * Treat quiet NaN like most functions (return quiet NaN, no exceptions raised unless the other argument is signaling NaN). * Treat signaling NaN like most functions (raise invalid, return quiet NaN). fminimum_num and fmaximum_num: * Treat both quiet and signaling NaN as missing data and return the other argument (if a number). "invalid" is still raised if one argument is a signaling NaN (contrary to the normal rule that raising "invalid" means also returning a quiet NaN).
On Wed, 26 Jan 2022, Joseph Myers wrote: > fmin and fmax: > > * Treat quiet NaN as missing data and return the other argument (if a > number). > > * Treat signaling NaN like most functions (raise invalid, return quiet > NaN). > > fminimum and fmaximum: > > * Treat quiet NaN like most functions (return quiet NaN, no exceptions > raised unless the other argument is signaling NaN). > > * Treat signaling NaN like most functions (raise invalid, return quiet > NaN). > > fminimum_num and fmaximum_num: > > * Treat both quiet and signaling NaN as missing data and return the other > argument (if a number). "invalid" is still raised if one argument is a > signaling NaN (contrary to the normal rule that raising "invalid" means > also returning a quiet NaN). Thanks! Maciej
On Wed, 26 Jan 2022, Joseph Myers wrote: > fmin and fmax: Also: * If the arguments are +0 and -0 in some order, it's unspecified what sign the result has. > fminimum and fmaximum: > fminimum_num and fmaximum_num: Also: * These four functions are required to treat -0 as less than +0.
Index: gcc/gcc/config/riscv/riscv.md =================================================================== --- gcc.orig/gcc/config/riscv/riscv.md +++ gcc/gcc/config/riscv/riscv.md @@ -1214,7 +1214,7 @@ ;; ;; .................... -(define_insn "smin<mode>3" +(define_insn "fmin<mode>3" [(set (match_operand:ANYF 0 "register_operand" "=f") (smin:ANYF (match_operand:ANYF 1 "register_operand" " f") (match_operand:ANYF 2 "register_operand" " f")))] @@ -1223,7 +1223,7 @@ [(set_attr "type" "fmove") (set_attr "mode" "<UNITMODE>")]) -(define_insn "smax<mode>3" +(define_insn "fmax<mode>3" [(set (match_operand:ANYF 0 "register_operand" "=f") (smax:ANYF (match_operand:ANYF 1 "register_operand" " f") (match_operand:ANYF 2 "register_operand" " f")))]