From: Tsukasa OI <research_trasio@irq.a4lg.com>
To: Nelson Chu <nelson@rivosinc.com>
Cc: binutils@sourceware.org
Subject: Re: [PATCH 0/3] RISC-V: Fix CSR accessibility and implications
Date: Wed, 7 Sep 2022 18:08:24 +0900 [thread overview]
Message-ID: <34e7540c-982c-0791-c526-f9bac7604f70@irq.a4lg.com> (raw)
In-Reply-To: <CAPpQWtBkERCevMDVseFCKb5HnJdb6DG=eNkuVsEsJGebMJ+Mjg@mail.gmail.com>
On 2022/09/07 17:49, Nelson Chu wrote:
> On Wed, Sep 7, 2022 at 1:54 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>>
>> Tracker on GitHub:
>> <https://github.com/a4lg/binutils-gdb/wiki/riscv_fix_csr_access_and_imply>
>>
>> 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
>>
>
prev parent reply other threads:[~2022-09-07 9:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-07 5:53 Tsukasa OI
2022-09-07 5:53 ` [PATCH 1/3] RISC-V: Fix vector CSR requirements and imply Tsukasa OI
2022-09-07 5:53 ` [PATCH 2/3] RISC-V: Imply 'Zicsr' from 'Zkr' Tsukasa OI
2022-09-07 5:53 ` [PATCH 3/3] RISC-V: Imply 'Zicsr' from some privileged extensions Tsukasa OI
2022-09-07 6:21 ` [PATCH v2 0/3] RISC-V: Fix CSR accessibility and implications Tsukasa OI
2022-09-07 6:21 ` [PATCH v2 1/3] RISC-V: Fix vector CSR requirements and imply Tsukasa OI
2022-09-07 9:34 ` Kito Cheng
2022-09-09 11:10 ` Tsukasa OI
2022-09-07 6:21 ` [PATCH v2 2/3] RISC-V: Imply 'Zicsr' from 'Zkr' Tsukasa OI
2022-09-07 6:21 ` [PATCH v2 3/3] RISC-V: Imply 'Zicsr' from some privileged extensions Tsukasa OI
2022-09-08 6:53 ` [PATCH 0/1] RISC-V: Fix CSR accessibility on vectors Tsukasa OI
2022-09-08 6:53 ` [PATCH 1/1] RISC-V: Fix vector CSR requirements Tsukasa OI
2022-09-08 7:12 ` Nelson Chu
2022-09-08 7:15 ` Tsukasa OI
2022-09-08 7:20 ` Tsukasa OI
2022-09-07 8:49 ` [PATCH 0/3] RISC-V: Fix CSR accessibility and implications Nelson Chu
2022-09-07 9:08 ` Tsukasa OI [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=34e7540c-982c-0791-c526-f9bac7604f70@irq.a4lg.com \
--to=research_trasio@irq.a4lg.com \
--cc=binutils@sourceware.org \
--cc=nelson@rivosinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).