Home Original page

[PATCH v3 4/4] RISC-V: Disable the CSR checking by default.

Jim Wilson jimw@sifive.com
Sat Feb 1 03:08:00 GMT 2020 
On Sun, Dec 15, 2019 at 9:22 PM Nelson Chu <nelson.chu@sifive.com> wrote:
>         gas/
>         * config/tc-riscv.c: Add new .option and GAS options to enbale/disable
>         the CSR checking.  We disable the CSR checking by default.
>         (riscv_ip, reg_lookup_internal): Check the `riscv_opts.csrcheck`
>         before we doing the CSR checking.
>         * doc/c-riscv.texi: Add description for the new .option and assembler
>         options.
>         * testsuite/gas/riscv/priv-reg-fail-fext.d: Add `-mcsrcheck` to enable
>         the CSR checking.
>         * testsuite/gas/riscv/priv-reg-fail-read-only-01.d: Likewise.
>         * testsuite/gas/riscv/priv-reg-fail-read-only-02.d: Likewise.
>         * testsuite/gas/riscv/priv-reg-fail-rv32-only.d: Likewise.

This patch makes the error message in the previous patch OK.  I should
have looked ahead a bit.  Though now I'm wondering if maybe csrcheck
can choose between an error and a warning.  So we have a choice here
between error/no error or error/warning.  There is a gas option
--fatal-warnings that will turn warnings into errors for people that
want that.  And a --no-warn/-W option that turns warnings off, if
people don't want the warning.

I think -mcsrcheck is a little hard to parse, maybe -mcsr-check
instead?  And similarly for the .option if we do that.

Otherwise this one looks OK, but depends on previous patches not approved yet.

Also, not directly related to this patch, but I noticed that we are
missing doc entries for the -march-attr/-mno-arch-attr command line
options added a year ago.  That would be at least partly my fault for
not asking for that during the review.  Maybe something you can add to
your to do list.

This fixes the csr-dw-regnums rv64 testsuite failure from the second
patch by accident, because you didn't turn on csrchecking for that
test.  But it should still be fixed anyways, in case someone turns
this on by default in their copy of binutils.

Jim
More information about the Binutils mailing list