From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by sourceware.org (Postfix) with ESMTPS id E8E963858002 for ; Tue, 8 Feb 2022 02:00:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E8E963858002 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-x1035.google.com with SMTP id h14-20020a17090a130e00b001b88991a305so767721pja.3 for ; Mon, 07 Feb 2022 18:00:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.com; s=20210112; h=date:subject:in-reply-to:cc:from:to:message-id; bh=GFSdR+Jy/7nmFam5G8n1hnrEvN7beYLe4HowMVCilqw=; b=iSPR1lt17zyqf5O2uczw/EKHUHGpldAlNROB/3VVlMK6+ZB5bUJQv0d7EEwpXGX1Cy nF7J5kSvFZfwDPiA6vu5ROr0hAjB3KlifIHDGvrr9AzoT5MbXtQSvYzA5auUHQEDy4cb B+kBvrO8UFjmmAcXGLc8ERlUKxknEJJAUgSvl3L5gKPHj57zwraF9FsJyYzwjFqFVJse s7ILaetot9fnCyM2lnqgD8CxxMnQ/c5f71L9Rd0tl1lRrUBIsA2s0eSjI0FRaNiv0tUw 72/hlXVweRtPlm/b2q1SisKoreT385e6n76SO3Wymmbyaz5No7qu8MCh2JsrOhmHuzIR 5n1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id; bh=GFSdR+Jy/7nmFam5G8n1hnrEvN7beYLe4HowMVCilqw=; b=6omEt+1bQ3gNF95/oCxZtkxplDXOKzQzGzPMNUvl8sXpnaLbmXki+MGB+SjifZU4Nk GIhC8K1WRZe/oN0+5TyyYRY1xCXxcPGdb8xIVVxi2sOHEEsbY5lkF8IfQhWkLmnA7fvH 9NWxPyHZ+7cLidTqCTxJE7Qc5G9UYjJZIUAw5B6cIzo1qxxi3Hlgv+q3IHrqgqMlkFuA /bRgPv7F0Gkvzll0aFmF9cVezHyFfa0wv4I5dAfrjdbOofDCBKFNQaRUUT3tPGK9tFH7 V7qgKZ4YVBvQLdg+To2sOi0HeJp8ysm3AkZo7yhjfvC3TewsKKoI5wiIUj/7SemC1uod mubQ== X-Gm-Message-State: AOAM530d5y4M7hH0jf6qrC9GE+Zxt+S4rotNZM1puL9vnYQWlVaCYvvJ SjAO1BWdZnbLhdmjTnHKaOpdiH/IpwNvCQ== X-Google-Smtp-Source: ABdhPJx+Iz20jL0GPhBwq3xGgx59M4wwoxbCR7M0xsGoYGUTnNryILzhIrtmiQAuEH5mQPLk58v/Cw== X-Received: by 2002:a17:902:7083:: with SMTP id z3mr2373833plk.133.1644285655589; Mon, 07 Feb 2022 18:00:55 -0800 (PST) Received: from localhost ([12.3.194.138]) by smtp.gmail.com with ESMTPSA id a29sm9349310pgl.24.2022.02.07.18.00.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Feb 2022 18:00:55 -0800 (PST) Date: Mon, 07 Feb 2022 18:00:55 -0800 (PST) X-Google-Original-Date: Mon, 07 Feb 2022 16:17:40 PST (-0800) Subject: Re: [RFC PATCH 0/2] RISC-V: Zfinx fixes/enhancements: Part 2A In-Reply-To: CC: research_trasio@irq.a4lg.com, binutils@sourceware.org From: Palmer Dabbelt To: binutils@sourceware.org Message-ID: X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, KAM_LOTSOFHASH, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Tue, 08 Feb 2022 02:00:59 -0000 On Tue, 01 Feb 2022 05:51:12 PST (-0800), binutils@sourceware.org wrote: > [About this patchset] > > This is the Part 2A (part 2 option A) of my Z[fdq]inx fixes and > enhancements. See Part 1 for general background. > > > > There is also Part 2B (part 2 option B) I will submit later to fix the > same bug but I prefer option A. See below for details. > > > > > [Bugfix in Part 2] > > On RV32_Zdinx and RV[32|64]_Zqinx, there is a bug that allows invalid > Zdinx/Zqinx instructions to be emitted and/or disassembled. > > GNU Assembler accepts all register index (as long as encodable) but > on Zdinx extension + RV32, all FP64 register numbers must be even > (encoding with odd register numbers is reserved). > > For instance, this is valid on RV64_Zdinx but not on RV32_Zdinx. > > fadd.d a1, a3, a5 > > On the other hand, this is valid both on RV[32|64]_Zdinx. > > fadd.d a0, a2, a4 > > Although not ratified, Zqinx would require similar constraints (register > pairs and quad-register groups) as the ISA Manual draft, > section 24.5 says: > >> In the future, an RV64Zqinx quad-precision extension could be defined >> analogously to RV32Zdinx. An RV32Zqinx extension could also be >> defined but would require quad-register groups. > > > > > [Option A and B] > > To address this issue (at least in the disassembler), we need to modify > instruction match functions to reject invalid encodings. However, > because whether given Zdinx/Zqinx instruction is valid depends on XLEN > and matching extension (D/Q or Zdinx/Zqinx), current `match_func' with > current instruction list cannot handle this situation. > > We have two options (as I came up with): > > > ** PART 2A (OPTION A) ** > 1. Separate D/Q and Zdinx/Zqinx completely and split Zdinx/Zqinx per > XLEN (because whether given Zdinx/Zqinx instruction is valid depends > on the XLEN). > > For instance, > > {"fadd.d", 0, INSN_CLASS_D_OR_ZDINX, ... , 0 }, > > in riscv_opcodes may be splitted to: > > {"fadd.d", 0, INSN_CLASS_D, ... , 0 }, > {"fadd.d", 32, INSN_CLASS_ZDINX, ... , 0 }, > {"fadd.d", 64, INSN_CLASS_ZDINX, ... , 0 }, > > to preserve current `match_func' interface simplicity. > > We also need 16 new matching functions: > > - match_opcode_zdinx_rtype_g2 > - match_opcode_zdinx_rtype_g4 > For regular arithmetic instructions (rd, rs1 and rs2 are checked) > - match_rs1_eq_rs2_zdinx_rtype_g2 > - match_rs1_eq_rs2_zdinx_rtype_g4 > For fmv, fneg and fabs pseudoinstructions > (rd and rs1[==rs2] are checked). > - match_opcode_zdinx_r4type_g2 > - match_opcode_zdinx_r4type_g4 > For FMA instructions (rd and rs1..3 are checked) > - match_opcode_zdinx_itype_g1_2 > - match_opcode_zdinx_itype_g1_4 > - match_opcode_zdinx_itype_g2_1 > - match_opcode_zdinx_itype_g2_4 > - match_opcode_zdinx_itype_g4_1 > - match_opcode_zdinx_itype_g4_2 > Mainly for fcvt instructions (rd and rs1 are checked). > - match_opcode_zdinx_itype_g2_2 > - match_opcode_zdinx_itype_g4_4 > For fsqrt instructions (rd and rs1 are checked). > - match_opcode_zdinx_cmp_g2 > - match_opcode_zdinx_cmp_g4 > For compare instructions (rs1 and rs2 are checked) > > Downside of this is: > > - It almost triples D/Q riscv_opcodes entries. This is bloat. > - We need separate Zdinx/Zqinx entries per XLEN (new riscv_opcodes > entries will be required for RV128_Z[dq]inx). > - We cannot give proper diagnostics as an assembler error > (other than simple "illegal operands") because we purely handle this > issue with general-purpose matching functions. > > > ** PART 2B (OPTION B) ** > 2. Pass additional information > > If we prefer to give proper diagnostics in the assembler, we definitely > need additional flags holding floating type information on each > instruction and we need to validate all floating point register operands > using those information. > > For assembler part, this is done in the Part 2B but... this part doesn't > handle disassembler issue (unlike Part 2A). > > To address disassembler issue, we ADDITIONALLY need to add some > arguments to `match_func' (RISC-V subset [riscv_parse_subset_t] and > XLEN) and add some matching functions (like Part 2A but the number of > new matching functions would be less) for Zdinx/Zqinx instructions. > > The big problem of this approach is bloat (in an other way than Part 2A) > and redundant design. > > Why do we need to pass riscv_rps_dis/riscv_rps_as just for Zdinx/Zqinx > instructions? Why do we need to check xlen twice? That's why I stopped > before tackling with the disassembler issue. > > > > > [Opinion (submitter prefers option A)] > > So my opinion is..., unless RISC-V maintainers prefer option B, option A > would be better and complete. I would like to hear thoughts of RISC-V > people. I'm fine with the version you have here. At least a bit of the complexity could be removed by dropping Zqinx, but given that it's been written it doesn't seem so bad to me if we decide to keep it around. > > > > > Tsukasa OI (2): > RISC-V: Prepare D/Q and Zdinx/Zqinx separation > RISC-V: Validate Zdinx/Zqinx register pairs > > bfd/elfxx-riscv.c | 10 +- > include/opcode/riscv.h | 4 +- > opcodes/riscv-opc.c | 541 +++++++++++++++++++++++++++++++++-------- > 3 files changed, 441 insertions(+), 114 deletions(-) > > > base-commit: e327c35ef5768789d3ba41a629f178f5eec32790 > prerequisite-patch-id: 9e408f2e6186c8956aae077daf95f38b9ad98675 > prerequisite-patch-id: 32ea143f7662a3297a7cf809cec6454e788f2916 > prerequisite-patch-id: 25d5aa65f72b1b4f1f52c92aa0f8ac30d218cc9c > prerequisite-patch-id: 6599ccdcc15585db285c30e14528f905327fd638 > prerequisite-patch-id: a5f3689afda87a68d4faae698c438aa3211521e0