public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jim Wilson <jimw@sifive.com>
To: Nelson Chu <nelson.chu@sifive.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH v3 4/4] RISC-V: Disable the CSR checking by default.
Date: Sat, 01 Feb 2020 03:08:00 -0000	[thread overview]
Message-ID: <CAFyWVaakoVcLXsb4GUU_csOXXp9H-P=LoYY4CydDpear5aHN6w@mail.gmail.com> (raw)
In-Reply-To: <1576473701-11072-5-git-send-email-nelson.chu@sifive.com>

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

  parent reply	other threads:[~2020-02-01  3:08 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-16  5:21 [PATCH v3 0/4] RISC-V: Support more rigorous check for CSR, and update them to spec 1.12 Nelson Chu
2019-12-16  5:21 ` [PATCH v3 4/4] RISC-V: Disable the CSR checking by default Nelson Chu
2020-01-03  1:32   ` [PING] " Nelson Chu
2020-02-01  3:08   ` Jim Wilson [this message]
2020-02-04  7:48     ` Nelson Chu
2020-02-04 20:22       ` Jim Wilson
2019-12-16  5:21 ` [PATCH v3 2/4] RISC-V: Support the ISA-dependent CSR checking Nelson Chu
2020-01-03  1:31   ` [PING] " Nelson Chu
2020-02-01  1:27   ` Jim Wilson
2020-02-04  7:15     ` Nelson Chu
2019-12-16  5:21 ` [PATCH v3 3/4] RISC-V: Support the read-only " Nelson Chu
2020-01-03  1:31   ` [PING] " Nelson Chu
2020-02-01  2:38   ` Jim Wilson
2020-02-04  7:31     ` Nelson Chu
2019-12-16  5:21 ` [PATCH v3 1/4] RISC-V: Update the CSR to privilege spec 1.12 Nelson Chu
2020-01-03  1:31   ` [PING] " Nelson Chu
2020-02-01  0:19   ` Jim Wilson
2020-02-01  0:24     ` Andrew Waterman
2020-02-04  7:06       ` Nelson Chu
2020-02-10 18:12       ` Palmer Dabbelt via binutils
2020-02-11  3:23         ` Nelson Chu
2020-01-03  1:29 ` [PING] [PATCH v3 0/4] RISC-V: Support more rigorous check for CSR, and update them to " Nelson Chu
2020-01-22 21:35 ` Palmer Dabbelt via binutils
2020-01-27 23:26   ` Jim Wilson
2020-01-30 18:39   ` Palmer Dabbelt via binutils

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='CAFyWVaakoVcLXsb4GUU_csOXXp9H-P=LoYY4CydDpear5aHN6w@mail.gmail.com' \
    --to=jimw@sifive.com \
    --cc=binutils@sourceware.org \
    --cc=nelson.chu@sifive.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).