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 6F3C83858D1E for ; Thu, 7 Sep 2023 10:12:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6F3C83858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=irq.a4lg.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=irq.a4lg.com Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 5BA33300089; Thu, 7 Sep 2023 10:11:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=irq.a4lg.com; s=2017s01; t=1694081518; bh=2/6j38iE4rPRKpM4Ayf+/rziejnKylNmdrW74FjOAT8=; h=Message-ID:Date:Mime-Version:Subject:To:References:Cc:From: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=LocRnL53OwrdfX30Dwx/M+/IJpGG0xpKpZP8x78tDNrct721oUa19mB9S6CbWrnIJ jgWWHOsW4w4HrcAGTf9XQzrwsBalhtSCAvrlh3hrOLFHJRQ17ZOU+mhZZL9m277Rwm GxwY1mlhw2YlE5yrs3lTJbPMvkd7KxvQ2vIhqC+8= Message-ID: <3ab4f583-50f3-4ddf-82f9-50476cf6999f@irq.a4lg.com> Date: Thu, 7 Sep 2023 19:11:57 +0900 Mime-Version: 1.0 Subject: Re: [PATCH 0/2] RISC-V: Support CORE-V XCVMAC and XCVALU extensions Content-Language: en-US To: Mary Bennett References: <20230905145300.652455-1-mary.bennett@embecosm.com> Cc: Binutils From: Tsukasa OI In-Reply-To: <20230905145300.652455-1-mary.bennett@embecosm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.0 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 List-Id: Hi, In general, I request one large change to your patch set. > +{"cv.mac", 0, INSN_CLASS_XCVMAC, "d,s,t", MATCH_CV_MAC, MASK_CV_MACMSU, match_opcode, 0}, > +{"cv.msu", 0, INSN_CLASS_XCVMAC, "d,s,t", MATCH_CV_MSU, MASK_CV_MACMSU, match_opcode, 0}, MASK values should be per (non-alias) instruction. Yes, that would make many MASK macros with different names but per-instruction MASK value is riscv-opcodes friendly and makes inspecting the opcodes easier. So, please make MASK values per-instruction as well as MATCH values and order like: - MATCH_CV_MAC - MASK_CV_MAC - MATCH_CV_MSU - MASK_CV_MSU - MATCH_CV_MULSN - MASK_CV_MULSN - MATCH_CV_MULSRN - MASK_CV_MULSRN... in PATCH 1/2 and likewise in PATCH 2/2. Otherwise it looks good (I haven't closely inspected yet though). Thanks, Tsukasa On 2023/09/05 23:52, Mary Bennett wrote: > This patch series presents the comprehensive implementation of the MAC and ALU > extension for CORE-V. > > Tested with riscv-gnu-toolchain on binutils, ld, gas and gcc testsuites to > ensure its correctness and compatibility with the existing codebase. > However, your input, reviews, and suggestions are invaluable in making this > extension even more robust. > > The CORE-V instructions are described in the specification [1] and work can be > found in the OpenHW group's Github repository [2]. > > [1] docs.openhwgroup.org/projects/cv32e40p-user-manual/en/latest/instruction_set_extensions.html > > [2] github.com/openhwgroup/corev-binutils-gdb > > Contributors: > Mary Bennett > Nandni Jamnadas > Pietra Ferreira > Charlie Keaney > Jessica Mills > Craig Blackmore > Simon Cook > Jeremy Bennett > > RISC-V: Add support for XCValu extension in CV32E40P > RISC-V: Add support for XCVmac extension in CV32E40P > > bfd/elfxx-riscv.c | 11 ++ > gas/config/tc-riscv.c | 60 +++++++ > gas/doc/c-riscv.texi | 10 ++ > gas/testsuite/gas/riscv/cv-alu-boundaries.d | 3 + > gas/testsuite/gas/riscv/cv-alu-boundaries.l | 14 ++ > gas/testsuite/gas/riscv/cv-alu-boundaries.s | 27 +++ > gas/testsuite/gas/riscv/cv-alu-fail-march.d | 3 + > gas/testsuite/gas/riscv/cv-alu-fail-march.l | 32 ++++ > gas/testsuite/gas/riscv/cv-alu-fail-march.s | 33 ++++ > .../gas/riscv/cv-alu-fail-operand-01.d | 3 + > .../gas/riscv/cv-alu-fail-operand-01.l | 32 ++++ > .../gas/riscv/cv-alu-fail-operand-01.s | 33 ++++ > .../gas/riscv/cv-alu-fail-operand-02.d | 3 + > .../gas/riscv/cv-alu-fail-operand-02.l | 32 ++++ > .../gas/riscv/cv-alu-fail-operand-02.s | 33 ++++ > .../gas/riscv/cv-alu-fail-operand-03.d | 3 + > .../gas/riscv/cv-alu-fail-operand-03.l | 25 +++ > .../gas/riscv/cv-alu-fail-operand-03.s | 26 +++ > .../gas/riscv/cv-alu-fail-operand-04.d | 3 + > .../gas/riscv/cv-alu-fail-operand-04.l | 3 + > .../gas/riscv/cv-alu-fail-operand-04.s | 4 + > .../gas/riscv/cv-alu-fail-operand-05.d | 3 + > .../gas/riscv/cv-alu-fail-operand-05.l | 9 + > .../gas/riscv/cv-alu-fail-operand-05.s | 10 ++ > .../gas/riscv/cv-alu-fail-operand-06.d | 3 + > .../gas/riscv/cv-alu-fail-operand-06.l | 9 + > .../gas/riscv/cv-alu-fail-operand-06.s | 10 ++ > .../gas/riscv/cv-alu-fail-operand-07.d | 3 + > .../gas/riscv/cv-alu-fail-operand-07.l | 33 ++++ > .../gas/riscv/cv-alu-fail-operand-07.s | 34 ++++ > gas/testsuite/gas/riscv/cv-alu-insns.d | 102 ++++++++++++ > gas/testsuite/gas/riscv/cv-alu-insns.s | 124 ++++++++++++++ > gas/testsuite/gas/riscv/cv-mac-fail-march.d | 3 + > gas/testsuite/gas/riscv/cv-mac-fail-march.l | 23 +++ > gas/testsuite/gas/riscv/cv-mac-fail-march.s | 24 +++ > gas/testsuite/gas/riscv/cv-mac-fail-operand.d | 3 + > gas/testsuite/gas/riscv/cv-mac-fail-operand.l | 147 +++++++++++++++++ > gas/testsuite/gas/riscv/cv-mac-fail-operand.s | 156 ++++++++++++++++++ > gas/testsuite/gas/riscv/cv-mac-insns.d | 87 ++++++++++ > gas/testsuite/gas/riscv/cv-mac-insns.s | 81 +++++++++ > include/opcode/riscv-opc.h | 56 +++++++ > include/opcode/riscv.h | 12 ++ > opcodes/riscv-dis.c | 20 +++ > opcodes/riscv-opc.c | 61 +++++++ > 44 files changed, 1406 insertions(+) > create mode 100644 gas/testsuite/gas/riscv/cv-alu-boundaries.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-boundaries.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-boundaries.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-march.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-march.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-march.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-01.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-01.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-01.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-02.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-02.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-02.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-03.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-03.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-03.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-04.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-04.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-04.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-05.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-05.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-05.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-06.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-06.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-06.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-07.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-07.l > create mode 100644 gas/testsuite/gas/riscv/cv-alu-fail-operand-07.s > create mode 100644 gas/testsuite/gas/riscv/cv-alu-insns.d > create mode 100644 gas/testsuite/gas/riscv/cv-alu-insns.s > create mode 100644 gas/testsuite/gas/riscv/cv-mac-fail-march.d > create mode 100644 gas/testsuite/gas/riscv/cv-mac-fail-march.l > create mode 100644 gas/testsuite/gas/riscv/cv-mac-fail-march.s > create mode 100644 gas/testsuite/gas/riscv/cv-mac-fail-operand.d > create mode 100644 gas/testsuite/gas/riscv/cv-mac-fail-operand.l > create mode 100644 gas/testsuite/gas/riscv/cv-mac-fail-operand.s > create mode 100644 gas/testsuite/gas/riscv/cv-mac-insns.d > create mode 100644 gas/testsuite/gas/riscv/cv-mac-insns.s >