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 46A4F3850851 for ; Wed, 7 Sep 2022 09:08:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 46A4F3850851 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 18038300089; Wed, 7 Sep 2022 09:08:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=irq.a4lg.com; s=2017s01; t=1662541709; bh=Cp/JpTmKfk1b8c8XxoH6a65qrmONGfaWzeeeAl+b840=; h=Message-ID:Date:Mime-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=GhBa0zUXn4puqxV/K6M6vXwYbr6PjVrFHFkZgwuHiGOrcc+8rdMPxH06wrdPuHs9U yXdH3ulNb8DwyTQrfwhxrqA5YCFTBmE34O3H77bk98WzaeW401wkmfwNT59Mz8UEDR eLrKGU1UztrdTp5q3pQROW1oEQPcsJBwhDZOd2YA= Message-ID: <34e7540c-982c-0791-c526-f9bac7604f70@irq.a4lg.com> Date: Wed, 7 Sep 2022 18:08:24 +0900 Mime-Version: 1.0 Subject: Re: [PATCH 0/3] RISC-V: Fix CSR accessibility and implications Content-Language: en-US To: Nelson Chu 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.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: On 2022/09/07 17:49, Nelson Chu wrote: > On Wed, Sep 7, 2022 at 1:54 PM Tsukasa OI wrote: >> >> Tracker on GitHub: >> >> >> While I'm reorganizing "b-ext*" and "k-ext*" tests (to make sure that they >> are able to be removed since there are neither 'B' nor 'K' extensions), >> I found something interesting. >> >> If we assemble a file with "csrr a0, seed" (uses "seed" CSR from the 'Zkr' >> extension) with "-march=rv32i_zkr", it causes an error to require 'Zicsr'. >> Yes, current GNU Binutils does not imply 'Zicsr' from 'Zkr' so that adding >> 'Zkr' extension alone is not enough to access corresponding CSRs >> (current version requires an option like "-march=rv32i_zicsr_zkr"). >> "k-ext.d" does not give 'Zicsr' either (and does not test the seed CSR) and >> adding the line "csrr a0, seed" to this test file resulted the same. > > According to the ISA spec, Zicsr is the sub-extension of i, and is > used to control the csr instructions, so I don't see any conflict for > the current toolchain. Though it looks weird that we allow csrs but > need to enable csr instruction by another extension. If you still > have problems, you can create and raise an issue maybe in > riscv-isa-manual or riscv-v-spec, to clarify if the zicsr controls > both csr and csr instructions. Maybe it's worth clarifying this, > since LLVM doesn't recognize zicsr in the main branch. I remember I asked something similar to Andrew Waterman already but I forgot where and when. I'll figure it out later. > >> That means something. If an extension adds CSR(s), we should imply 'Zicsr' >> from that extension (just like 'F'). So, we have to imply 'Zicsr' from >> following extensions: >> >> - 'H' >> - 'Zkr' >> - 'Zve32x' (I'll talk later) >> - 'Smstateen' >> - 'Sscofpmf' >> - 'Sstc' >> >> 'Zkr' is fixed in PATCH 2/3 and 'H' and 'S*' are fixed in PATCH 3/3. >> The only remaining extension is 'Zve32x' but I need to talk more to explain >> actual changes (specific to vectors) in PATCH 1/3. >> >> On the current version of GNU Binutils, CSRs with CSR_CLASS_V means they >> require the 'V' extension. However, there are a few vector subextensions >> that implement vector subsets (intended for embedded processors). > > Check zve32x for vcsr should be enough, since LLVM looks to did the > similar thing for vector instructions, > https://github.com/llvm/llvm-project/blob/7167a4207ee2c07cb192da1788f919332f83b456/llvm/lib/Support/RISCVISAInfo.cpp#L708 I agree and that's what a part of PATCH 1/3 is doing. It looks changing vector CSR requirement from 'V' to 'Zve32x' is acceptable to you. If so, I'll submit separate patch for this part. Thanks, Tsukasa > >> - 'Zve64d' (superset of 'Zve64f') >> - 'Zve64f' (superset of 'Zve32f' and 'Zve64x') >> - 'Zve64x' (superset of 'Zve32x') >> - 'Zve32f' (superset of 'Zve32x') >> - 'Zve32x' >> >> : Graph: Dependency graph of some vector/FP extensions >> : >> : +-------> D --------> F -----> Zicsr >> : | ^ ^ >> : | | | >> : V ---> Zve64d ---> Zve64f ---> Zve64x >> : | | >> : V V >> : Zve32f ---> Zve32x >> : | >> : | >> : +---> (Zicsr [added in PATCH 1/3]) >> >> They also require general purpose vector CSRs (vstart, vl, vtype and vlenb). >> So, corresponding CSR_CLASS_V with the 'V' extension is inappropriate >> (they should require 'Zve32x' instead, the minimum vector subset). >> >> Remaining CSRs are: >> >> - vxsat >> - vxrm >> - vcsr >> >> They are related to fixed-point arithmetic and 18.2 "Zve*: Vector Extensions >> for Embedded Processors" says: >> >>> All Zve* extensions support all vector fixed-point arithmetic instructions >>> (Vector Fixed-Point Arithmetic Instructions), except that vsmul.vv and >>> vsmul.vx are not supported for EEW=64 in Zve64*. >> >> So, their minimum requirement shall be also 'Zve32x', not 'V'. >> >> As a consequence, we can conclude that changing requirements of CSR_CLASS_V >> from 'V' to 'Zve32x' is sufficient to avoid CSR accessibility warnings. >> >> Also, 'Zve32x' must imply 'Zicsr' (just like the rest) to avoid CSR >> instruction errors. This is only effective on 'Zve32x' and 'Zve64x' >> because, for instance, 'Zve32f' implies 'F' and 'F' implies 'Zicsr' >> (see the graph above). >> >> I didn't rename CSR_CLASS_V to CSR_CLASS_ZVE32X because the name gets >> difficult and there's already INSN_CLASS_V (effectively requires 'Zve32x' >> with some exceptions). >> >> >> >> >> Tsukasa OI (3): >> RISC-V: Fix vector CSR requirements and imply >> RISC-V: Imply 'Zicsr' from 'Zkr' >> RISC-V: Imply 'Zicsr' from some privileged extensions >> >> bfd/elfxx-riscv.c | 6 +++++ >> gas/config/tc-riscv.c | 2 +- >> gas/testsuite/gas/riscv/csr-version-1p10.l | 28 ++++++++++---------- >> gas/testsuite/gas/riscv/csr-version-1p11.l | 28 ++++++++++---------- >> gas/testsuite/gas/riscv/csr-version-1p12.l | 28 ++++++++++---------- >> gas/testsuite/gas/riscv/csr-version-1p9p1.l | 28 ++++++++++---------- >> gas/testsuite/gas/riscv/march-imply-h.d | 6 +++++ >> gas/testsuite/gas/riscv/vector-csrs-zve32f.d | 21 +++++++++++++++ >> gas/testsuite/gas/riscv/vector-csrs-zve32x.d | 21 +++++++++++++++ >> gas/testsuite/gas/riscv/vector-csrs-zve64d.d | 21 +++++++++++++++ >> gas/testsuite/gas/riscv/vector-csrs-zve64f.d | 21 +++++++++++++++ >> gas/testsuite/gas/riscv/vector-csrs-zve64x.d | 21 +++++++++++++++ >> gas/testsuite/gas/riscv/vector-csrs.s | 12 +++++++++ >> gas/testsuite/gas/riscv/zkr.d | 10 +++++++ >> gas/testsuite/gas/riscv/zkr.s | 2 ++ >> 15 files changed, 198 insertions(+), 57 deletions(-) >> create mode 100644 gas/testsuite/gas/riscv/march-imply-h.d >> create mode 100644 gas/testsuite/gas/riscv/vector-csrs-zve32f.d >> create mode 100644 gas/testsuite/gas/riscv/vector-csrs-zve32x.d >> create mode 100644 gas/testsuite/gas/riscv/vector-csrs-zve64d.d >> create mode 100644 gas/testsuite/gas/riscv/vector-csrs-zve64f.d >> create mode 100644 gas/testsuite/gas/riscv/vector-csrs-zve64x.d >> create mode 100644 gas/testsuite/gas/riscv/vector-csrs.s >> create mode 100644 gas/testsuite/gas/riscv/zkr.d >> create mode 100644 gas/testsuite/gas/riscv/zkr.s >> >> >> base-commit: f555b327d41ed72ffae28caae550f5f86312db43 >> -- >> 2.34.1 >> >