From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id 176653858C2D for ; Sat, 30 Jul 2022 03:21:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 176653858C2D Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 96617300089; Sat, 30 Jul 2022 03:21:56 +0000 (UTC) Message-ID: <83a4a879-c9be-5b25-256e-eed97441688f@irq.a4lg.com> Date: Sat, 30 Jul 2022 12:21:54 +0900 Mime-Version: 1.0 Subject: Re: [PATCH v4 0/4] RISC-V: Combined floating point enhancements Content-Language: en-US To: Nelson Chu , Kito Cheng , Palmer Dabbelt Cc: binutils@sourceware.org References: From: Tsukasa OI In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, 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: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2022 03:22:01 -0000 Additional Report - Support on LLVM: Functional differences between my patchset and the latest LLVM is whether requirements to "fmv.[sdq]" instructions are relaxed. As I noted earlier, supporting "fmv.[sdq]" on Zfinx environment seems more natural. Register pairs of RV32_Zdinx (note that LLVM does not support Zqinx) are supported on LLVM >= 14. See GPRPF64 for register pairs on LLVM. In fact, all zdinx-32-regpair-fail.s testcases generated errors (total: 110) and zdinx-32-regpair.s only generated one compiler error ("fmv.d" is not supported on LLVM but this is not related to register pairs). Recap - Existing Reviews: Although that the patchset itself changes multiple times, its functionality is mostly reviewed. It's nearly 6 months ago so it's not a bad timing to remind you all. Relaxing fmv.[sdq] requirements (PATCH 2/4): - Reviewed by Palmer Dabbelt Zfinx register pairs (PATCH 3/4): - This is when I had two ideas to fix this issue but the version I implemented in combined patchset v4 seems fine to Palmer. Note that there's no "Reviewed-by" tag, though. I hope they are useful to review my changes. On 2022/07/25 3:58, Tsukasa OI wrote: > Before combining patchsets: > (Zfh/Zfhmin v2) > (Zfinx v2) > Combined v1: > > Combined v2: > > Combined v3: > > > Tracker on GitHub: > > > This is a rebased and fixed version for Zfinx-related issues. > > First, let me recap the changes. > > > > > 1. Enhance Zfinx/Zdinx/Zqinx Testcases (PATCH 1/4) > > I enhanced Zfinx/Zdinx/Zqinx testcases based on Jiawei's Zhinx support > patch. I also... > > - Made indentation / coding style consistent and clean > - Started to use valid register number on Zqinx > (as I explain below [PATCH 3-4/4]) > - Started to use different register per operand > > > 2. Relax Requirements to fmv.[sdq] instructions (PATCH 2/4) > > On Zfinx/Zdinx/Zqinx, fmv instructions seem redundant but actually not. > On RV32_Z[dq]inx and RV64_Zqinx, it requires register pair. > > That means, single... > fmv.d x10, x12 > is equivalent to 2 regular instructions on RV32_Zdinx (with 32-bit GPRs): > mv x10, x12 > mv x11, x13 > > Since fsgnj.[sdq] (base instruction of fmv.[sdq]) are a part of Z[fdq]inx > extensions, it's safe to implement this pseudo-instructions. > > This patch makes fmv.[sdq] available to Zfinx/Zdinx/Zqinx environments. > > > 3. Validate Register pairs on Zdinx/Zqinx (PATCH 3-4/4) > > On RV32_Zdinx and RV64_Zqinx, all registers holding a FP64 value must be even > (x0, x2, x4... are valid, x1, x3, x5... are invalid). On RV32_Zqinx, it would > be that all registers holding a FP128 value will be required to have a multiple > of 4 (x0, x4, x8... are valid, x1, x2, x3, x5... are not). > > On RV32_Zdinx, this is valid. > fadd.d x10, x12, x14 > On the other hand, this is not valid (on RV32_Zdinx). > fadd.d x11, x13, x15 > > Current Binutils can generate invalid instructions with odd register numbers > (or register number x % 4 != 0). PATCH 3/4 makes those invalid. > > Testcases (PATCH 4/4) are separate only because it is quite large. > > > > > [Changes: v3 -> v4] > > - Rebased > - Fixed `fcvt.q.l{,u}' testcases (we don't need rounding anymore) > Clearly, I forgot to test the patchset. > This time, I confirmed that it passes the test. > > > [Changes: v2 -> v3] > > - Rebased > - Postponed non-functional (coding style-related) changes > for another patchset > - Started to use `.insn r OP_FP, ...' on Z[dq]inx disassembler tests > - Fixed `fcvt.q.l{,u}' instruction match values > > > > > Tsukasa OI (4): > RISC-V: Reorganize and enhance Zfinx tests > RISC-V: Relax `fmv.[sdq]' requirements > RISC-V: Validate Zdinx/Zqinx register pairs > RISC-V: Add testcases for Z[dq]inx register pairs > > bfd/elfxx-riscv.c | 8 + > gas/config/tc-riscv.c | 21 +- > .../gas/riscv/zdinx-32-regpair-dis.d | 11 + > .../gas/riscv/zdinx-32-regpair-dis.s | 5 + > .../gas/riscv/zdinx-32-regpair-fail.d | 3 + > .../gas/riscv/zdinx-32-regpair-fail.l | 111 ++++ > .../gas/riscv/zdinx-32-regpair-fail.s | 116 ++++ > gas/testsuite/gas/riscv/zdinx-32-regpair.d | 65 +++ > gas/testsuite/gas/riscv/zdinx-32-regpair.s | 62 ++ > gas/testsuite/gas/riscv/zdinx.d | 27 +- > gas/testsuite/gas/riscv/zdinx.s | 46 +- > gas/testsuite/gas/riscv/zfinx.d | 24 +- > gas/testsuite/gas/riscv/zfinx.s | 42 +- > .../gas/riscv/zqinx-32-regpair-dis.d | 12 + > .../gas/riscv/zqinx-32-regpair-dis.s | 7 + > .../gas/riscv/zqinx-32-regpair-fail.d | 3 + > .../gas/riscv/zqinx-32-regpair-fail.l | 212 +++++++ > .../gas/riscv/zqinx-32-regpair-fail.s | 218 +++++++ > gas/testsuite/gas/riscv/zqinx-32-regpair.d | 66 +++ > gas/testsuite/gas/riscv/zqinx-32-regpair.s | 64 +++ > .../gas/riscv/zqinx-64-regpair-dis.d | 11 + > .../gas/riscv/zqinx-64-regpair-dis.s | 5 + > .../gas/riscv/zqinx-64-regpair-fail.d | 3 + > .../gas/riscv/zqinx-64-regpair-fail.l | 133 +++++ > .../gas/riscv/zqinx-64-regpair-fail.s | 138 +++++ > gas/testsuite/gas/riscv/zqinx-64-regpair.d | 87 +++ > gas/testsuite/gas/riscv/zqinx-64-regpair.s | 84 +++ > gas/testsuite/gas/riscv/zqinx.d | 84 +-- > gas/testsuite/gas/riscv/zqinx.s | 87 +-- > include/opcode/riscv.h | 10 +- > opcodes/riscv-opc.c | 541 ++++++++++++++---- > 31 files changed, 2097 insertions(+), 209 deletions(-) > create mode 100644 gas/testsuite/gas/riscv/zdinx-32-regpair-dis.d > create mode 100644 gas/testsuite/gas/riscv/zdinx-32-regpair-dis.s > create mode 100644 gas/testsuite/gas/riscv/zdinx-32-regpair-fail.d > create mode 100644 gas/testsuite/gas/riscv/zdinx-32-regpair-fail.l > create mode 100644 gas/testsuite/gas/riscv/zdinx-32-regpair-fail.s > create mode 100644 gas/testsuite/gas/riscv/zdinx-32-regpair.d > create mode 100644 gas/testsuite/gas/riscv/zdinx-32-regpair.s > create mode 100644 gas/testsuite/gas/riscv/zqinx-32-regpair-dis.d > create mode 100644 gas/testsuite/gas/riscv/zqinx-32-regpair-dis.s > create mode 100644 gas/testsuite/gas/riscv/zqinx-32-regpair-fail.d > create mode 100644 gas/testsuite/gas/riscv/zqinx-32-regpair-fail.l > create mode 100644 gas/testsuite/gas/riscv/zqinx-32-regpair-fail.s > create mode 100644 gas/testsuite/gas/riscv/zqinx-32-regpair.d > create mode 100644 gas/testsuite/gas/riscv/zqinx-32-regpair.s > create mode 100644 gas/testsuite/gas/riscv/zqinx-64-regpair-dis.d > create mode 100644 gas/testsuite/gas/riscv/zqinx-64-regpair-dis.s > create mode 100644 gas/testsuite/gas/riscv/zqinx-64-regpair-fail.d > create mode 100644 gas/testsuite/gas/riscv/zqinx-64-regpair-fail.l > create mode 100644 gas/testsuite/gas/riscv/zqinx-64-regpair-fail.s > create mode 100644 gas/testsuite/gas/riscv/zqinx-64-regpair.d > create mode 100644 gas/testsuite/gas/riscv/zqinx-64-regpair.s > > > base-commit: ea892bdc4b6da5782c6ea6273aff499fb5fd5e6f