Re: [PATCH 0/3] RISC-V: Prepare Privileged Architecture 1.12

[Nelson Chu <nelson.chu@sifive.com>]

Hi Tsukasa,

I also rewriting the csr testcases in these days, and doing the
similar things as yours, but there is a little difference.  Anyway, I
will leave the priv 1.12 updates to you, hope your copyright can be
finished soon :)

Besides, there are some TODOs you or others may interesting,

* In the csr-version-xxx.l, I think these warnings have a lot of room
for improvement.  For now, I just report "invalid CSR `xxx' for the
current ISA", but it would be great if we can report more details to
users.  For example, "invalid CSR `xxx', need to enable xxx extension"
or "invalid rv32-only CSR `xxx'".   Besides, we probably will have to
check multiple extensions for the CSR, like CSR_CLASS_H_AND_SVINVAL,
so the error messages may be more complicated.

* For now we are using PRIV_SPEC_CLASS_XXX in the DECLARE_CSR, to
describe when the csr was(is) defined, and when the csr was(is)
dropped.  But according to the link,
https://wiki.riscv.org/display/TECH/Recently+Ratified+Extensions, I
see that the extensions Sm1-12 and Ss1-12 are ratified..., and we will
also have an extension name for hypersior in the arch string.
Therefore, I suppose all csrs will be controlled by the extension
versions, rather than the privileged spec versions, so we will need to
rewrite the whole current code in the future.

BTW, the Sm1-12 and Ss1-12 extension names look weird to me, since I
expect they should be Sm and Ss, with the different versions 1p12 or
<major>p<minor>p<revision> in the arch string.  Anyway, this is a
different issue in fact.

Thanks
Nelson

On Fri, Dec 24, 2021 at 11:10 AM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> This patchset adds minimal support for the Privileged Architecture
> version 1.12.
>
> This is preparation for my upcoming patchset to add all CSRs as defined
> in recently ratified documents.  Because my copyright assignment to FSF
> is not complete, this patchset is intentionally designed to be
> *not legally significant* (despite large patch size at first glance).
>
>     I haven't even received the assignment form but it's okay because
>     it's the holidays.
>
> It also improves testcases for maintainability.  Those changes are hardly
> functional but can be a foundation of upcoming changes (by Vineet and me,
> at least).
>
> To quote Information for Maintainers of GNU Software
> <https://www.gnu.org/prep/maintain/html_node/Legally-Significant.html>,
>
> > A regular series of repeated changes, such as renaming a symbol, is not
> > legally significant even if the symbol has to be renamed in many places.
>
>
>
>
> [Patch 1/3: Make testcase indentation consistent]
>
> When "[(SPACE)(TAB)]+" pattern (regex) should be tested in RISC-V
> testcases, following 4 files have "[(x*SPACE)(TAB)]+" (x >= 2):
>
> -   gas/testsuite/gas/riscv/option-arch-01a.d
> -   gas/testsuite/gas/riscv/priv-reg-version-1p10.d
> -   gas/testsuite/gas/riscv/priv-reg-version-1p11.d
> -   gas/testsuite/gas/riscv/priv-reg-version-1p9p1.d
>
> They even have different indentation.  For instance, similar
> "priv-reg-version-1p10.d" and "priv-reg-version-1p11.d" have different
> indentation and that makes making testcases tedious (per-file adoptation
> is required).
>
> This commit makes all such instances (except 2 occurrences in
> "option-arch-01a.d", which will be unaligned if I change that) to
> "[(1*SPACE)(TAB)]+".
>
> All those changes are considered non-creative.
>
>
>
> [Patch 2/3: Unify "access all CSRs" testcases]
>
> On some RISC-V testcases, we have to test access to all (or almost all)
> CSRs.  However, we have two similar but separate files exist:
>
> -   gas/testsuite/gas/riscv/priv-reg.s
> -   gas/testsuite/gas/riscv/priv-reg-fail-read-only-01.s
>
> This commit moves main contents of "priv-reg.s" to "priv-reg-access-all.s"
> and makes two files above include that common source file.
>
> As a side effect, many .d files need additional "-I$srcdir/$subdir" option.
>
> This idea is from gas/testsuite/gas/arm/cde.[ds].
> Only two lines (.include directives) are considered minimally creative.
>
>
>
> [Patch 3/3: Add Privileged Architecture version 1.12]
>
> This commit adds minimal support (with help string and testcases) to the
> Privileged Architecture version 1.12.
>
> Unlike Vineet Gupta's patch,
>
> 1.  It also adds help string
> 2.  It also copies "1p11" testcases to "1p12"
>     (with non-creative version adoptation)
>
> (cf.) Vineet Gupta's privileged architecture 1.12 patch:
> <https://sourceware.org/pipermail/binutils/2021-December/118982.html>
>
> Three lines (in total) from three files are considered minimally creative:
>
> -   bfd/cpu-riscv.c
> -   bfd/cpu-riscv.h
> -   gas/config/tc-riscv.c
>
>
>
>
> From my view, only 5 lines are minimally creative and all the rest are
> non-creative changes (regular series of repeated changes), hoping that
> this whole patchset is *not* legally significant.
>
>
>
>
> Tsukasa OI (3):
>   RISC-V: Make testcase indentation consistent
>   RISC-V: Unify "access all CSRs" testcases
>   RISC-V: Add Privileged Architecture version 1.12
>
>  bfd/cpu-riscv.c                               |   1 +
>  bfd/cpu-riscv.h                               |   1 +
>  gas/config/tc-riscv.c                         |   2 +-
>  gas/testsuite/gas/riscv/csr-dw-regnums.d      |   2 +-
>  gas/testsuite/gas/riscv/option-arch-01a.d     |   8 +-
>  gas/testsuite/gas/riscv/priv-reg-access-all.s | 292 ++++++++++
>  gas/testsuite/gas/riscv/priv-reg-fail-fext.d  |   2 +-
>  .../gas/riscv/priv-reg-fail-read-only-01.d    |   2 +-
>  .../gas/riscv/priv-reg-fail-read-only-01.s    | 267 +--------
>  .../gas/riscv/priv-reg-fail-read-only-02.d    |   2 +-
>  .../gas/riscv/priv-reg-fail-rv32-only.d       |   2 +-
>  .../gas/riscv/priv-reg-fail-version-1p10.d    |   2 +-
>  .../gas/riscv/priv-reg-fail-version-1p11.d    |   2 +-
>  .../gas/riscv/priv-reg-fail-version-1p12.d    |  11 +
>  .../gas/riscv/priv-reg-fail-version-1p12.l    |  24 +
>  .../gas/riscv/priv-reg-fail-version-1p9p1.d   |   2 +-
>  gas/testsuite/gas/riscv/priv-reg-fail-zkr.d   |   2 +-
>  .../gas/riscv/priv-reg-version-1p10.d         | 534 +++++++++---------
>  .../gas/riscv/priv-reg-version-1p11.d         | 534 +++++++++---------
>  .../gas/riscv/priv-reg-version-1p12.d         | 275 +++++++++
>  .../gas/riscv/priv-reg-version-1p9p1.d        | 534 +++++++++---------
>  gas/testsuite/gas/riscv/priv-reg.s            | 294 +---------
>  22 files changed, 1421 insertions(+), 1374 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/priv-reg-access-all.s
>  create mode 100644 gas/testsuite/gas/riscv/priv-reg-fail-version-1p12.d
>  create mode 100644 gas/testsuite/gas/riscv/priv-reg-fail-version-1p12.l
>  create mode 100644 gas/testsuite/gas/riscv/priv-reg-version-1p12.d
>
>
> base-commit: d20236e748ab70e9243960850eef64838f1b9721
> --
> 2.32.0
>