public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nelson Chu <nelson@rivosinc.com>
To: Christoph Muellner <christoph.muellner@vrull.eu>
Cc: binutils@sourceware.org, Kito Cheng <kito.cheng@sifive.com>,
	 Jim Wilson <jim.wilson.gcc@gmail.com>,
	Philipp Tomsich <philipp.tomsich@vrull.eu>,
	 Palmer Dabbelt <palmer@dabbelt.com>,
	Cooper Qu <cooper.qu@linux.alibaba.com>,
	 Lifang Xia <lifang_xia@linux.alibaba.com>,
	Yunhai Shang <yunhai@linux.alibaba.com>,
	 Zhiwei Liu <zhiwei_liu@linux.alibaba.com>
Subject: Re: [PATCH 00/13] Add support for the T-Head vendor extensions
Date: Fri, 16 Sep 2022 17:36:32 +0800	[thread overview]
Message-ID: <CAPpQWtBiCntsD=443+BZVLT94v0yPDajty-STChLNLfn8WsubQ@mail.gmail.com> (raw)
In-Reply-To: <20220906122213.1243129-1-christoph.muellner@vrull.eu>

Hi Christoph,


On Tue, Sep 6, 2022 at 8:22 PM Christoph Muellner
<christoph.muellner@vrull.eu> wrote:
>
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> This series introduces support for the T-Head vendor extensions,
> which are implemented e.g. in the XuanTie C906 and XuanTie C910
> processors:
> * XTheadBa
> * XTheadBb
> * XTheadBs
> * XTheadCmo
> * XTheadCondMov
> * XTheadFMemIdx
> * XTheadMac
> * XTheadMemIdx
> * XTheadMemPair
> * XTheadSync
>
> The xthead* extensions are documented here:
> https://github.com/T-head-Semi/thead-extension-spec/releases/download/2.0.0/xthead-2022-09-05-2.0.0.pdf
>
> The "th." instruction prefix prevents future conflicts with standard
> extensions and has been documentented in a PR for the RISC-V toolchain
> conventions:
>   https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/19
>
> The goal of this patchset is to provide access to these instruction
> so that compilers/users can optimize SW accordingly.
>
> Note, that the T-Head vendor extensions do not contain all
> vendor-specific
> functionality of the T-Head SoCs (e.g. no vendor-specific CSRs are
> included).
> Instead the extensions cover coherent functionality, that is exposed to
> S and U mode.
>
> To enable the extensions above, the following two methods are possible:
> * add the extension to the arch string (e.g.
> * -march=rv64gc_xtheadcmo_xtheadsync)
> * implicitly select the extensions via CPU selection (e.g.
> * -mcpu=thead-c910)
>
> The patchset attempts to minimize code changes in generic/infrastructure
> code. All patches in this series come with tests to avoid future regressions.
>
> Christoph Müllner (13):
>   RISC-V: Add generic support for vendor extensions
>   RISC-V: Add T-Head CMO vendor extension
>   RISC-V: Add T-Head SYNC vendor extension
>   RISC-V: Add support for arbitrary immediate encoding formats
>   RISC-V: Add T-Head Bitmanip vendor extension
>   RISC-V: Add T-Head CondMov vendor extension
>   RISC-V: Add T-Head MAC vendor extension
>   RISC-V: Add T-Head FMemIdx vendor extension
>   RISC-V: Add T-Head MemIdx vendor extension
>   RISC-V: Add support for literal instruction arguments
>   RISC-V: Add T-Head MemPair vendor extension

There are three minor issues,

1. Probably need to add new extension support in
riscv_multi_subset_supports_ext, although it is just used for
reporting better error messages.
2. The operand L seems only to be used for t-head for now, so it
should be still a vendor operand, named with the X prefix probably
better.
3. I remember Lifang mentioned before that we should add these vendor
stuff based on the vendor infra in the integration branch, and that's
what I asked him to rebase his patches  to there before, since the
previous policy cannot accept vendor stuff on master branch.  But now
I think most of us agree to accept t-head extensions on mainline
directly, so I figured these things cannot be blocked by the vendor
infra and cannot be committed.  That doesn't mean we don't need the
vendor infra, that means we can support the vendor infra later if me
or someone have time to move them back to mainline.

Therefore, these patches look good to me for now, thank you very much
for spending so much time to implement these.  But personally, I think
we should get the approval from the T-Head guys at least.  Btw, If
these implementations are referred to and based on the integration
branch, then I think it would be better to keep at least the
co-authors there in the commit message here; if these patches are
re-write without referring to the implementation from the integration
branch, then I need to apologize to them, because I asked them to
rewrite their patches over there, and cannot be committed to mainline
at that time...

>   riscv: gas: Add command line option '-mcpu=' to specify the CPU
>   riscv: Add T-Head entries for the -mcpu= flag

Not sure if it is necessary to add -mcpu in assembler, since the
compiler should recognize -mcpu, and will expand it to the correct
extensions to assembler.  If there are no objections for a period of
time, then please go ahead and commit these.

Thank you very much
Nelson

>  bfd/elfxx-riscv.c                             |  39 ++-
>  gas/config/tc-riscv.c                         | 127 +++++++
>  gas/doc/c-riscv.texi                          |  70 ++++
>  gas/testsuite/gas/riscv/mcpu-fail-unknown.d   |   3 +
>  gas/testsuite/gas/riscv/mcpu-fail-unknown.l   |   2 +
>  gas/testsuite/gas/riscv/mcpu-ok-c906.d        |   6 +
>  gas/testsuite/gas/riscv/mcpu-ok-c910.d        |   6 +
>  gas/testsuite/gas/riscv/x-thead-ba-fail.d     |   3 +
>  gas/testsuite/gas/riscv/x-thead-ba-fail.l     |   3 +
>  gas/testsuite/gas/riscv/x-thead-ba-fail.s     |   3 +
>  gas/testsuite/gas/riscv/x-thead-ba.d          |  13 +
>  gas/testsuite/gas/riscv/x-thead-ba.s          |   6 +
>  gas/testsuite/gas/riscv/x-thead-bb-fail.d     |   3 +
>  gas/testsuite/gas/riscv/x-thead-bb-fail.l     |   7 +
>  gas/testsuite/gas/riscv/x-thead-bb-fail.s     |   7 +
>  gas/testsuite/gas/riscv/x-thead-bb.d          |  30 ++
>  gas/testsuite/gas/riscv/x-thead-bb.s          |  22 ++
>  gas/testsuite/gas/riscv/x-thead-bs-fail.d     |   3 +
>  gas/testsuite/gas/riscv/x-thead-bs-fail.l     |   3 +
>  gas/testsuite/gas/riscv/x-thead-bs-fail.s     |   3 +
>  gas/testsuite/gas/riscv/x-thead-bs.d          |  14 +
>  gas/testsuite/gas/riscv/x-thead-bs.s          |   6 +
>  gas/testsuite/gas/riscv/x-thead-cmo-fail.d    |   3 +
>  gas/testsuite/gas/riscv/x-thead-cmo-fail.l    |  22 ++
>  gas/testsuite/gas/riscv/x-thead-cmo-fail.s    |  22 ++
>  gas/testsuite/gas/riscv/x-thead-cmo.d         |  30 ++
>  gas/testsuite/gas/riscv/x-thead-cmo.s         |  22 ++
>  gas/testsuite/gas/riscv/x-thead-condmov.d     |  11 +
>  gas/testsuite/gas/riscv/x-thead-condmov.s     |   3 +
>  .../gas/riscv/x-thead-fmemidx-fail.d          |   3 +
>  .../gas/riscv/x-thead-fmemidx-fail.l          |  18 +
>  .../gas/riscv/x-thead-fmemidx-fail.s          |  17 +
>  gas/testsuite/gas/riscv/x-thead-fmemidx.d     |  25 ++
>  gas/testsuite/gas/riscv/x-thead-fmemidx.s     |  17 +
>  gas/testsuite/gas/riscv/x-thead-mac.d         |  15 +
>  gas/testsuite/gas/riscv/x-thead-mac.s         |   7 +
>  gas/testsuite/gas/riscv/x-thead-memidx-fail.d |   3 +
>  gas/testsuite/gas/riscv/x-thead-memidx-fail.l |  14 +
>  gas/testsuite/gas/riscv/x-thead-memidx-fail.s |  14 +
>  gas/testsuite/gas/riscv/x-thead-memidx.d      |  53 +++
>  gas/testsuite/gas/riscv/x-thead-memidx.s      |  48 +++
>  .../gas/riscv/x-thead-mempair-fail.d          |   3 +
>  .../gas/riscv/x-thead-mempair-fail.l          |  30 ++
>  .../gas/riscv/x-thead-mempair-fail.s          |  30 ++
>  gas/testsuite/gas/riscv/x-thead-mempair.d     |  14 +
>  gas/testsuite/gas/riscv/x-thead-mempair.s     |   6 +
>  gas/testsuite/gas/riscv/x-thead-sync-fail.d   |   3 +
>  gas/testsuite/gas/riscv/x-thead-sync-fail.l   |   6 +
>  gas/testsuite/gas/riscv/x-thead-sync-fail.s   |   6 +
>  gas/testsuite/gas/riscv/x-thead-sync.d        |  14 +
>  gas/testsuite/gas/riscv/x-thead-sync.s        |   6 +
>  include/opcode/riscv-opc.h                    | 326 ++++++++++++++++++
>  include/opcode/riscv.h                        |  27 ++
>  opcodes/riscv-dis.c                           |  44 +++
>  opcodes/riscv-opc.c                           | 155 +++++++++
>  55 files changed, 1394 insertions(+), 2 deletions(-)
>  create mode 100644 gas/testsuite/gas/riscv/mcpu-fail-unknown.d
>  create mode 100644 gas/testsuite/gas/riscv/mcpu-fail-unknown.l
>  create mode 100644 gas/testsuite/gas/riscv/mcpu-ok-c906.d
>  create mode 100644 gas/testsuite/gas/riscv/mcpu-ok-c910.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-ba-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-ba-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-ba-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-ba.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-ba.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bb-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bb-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bb-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bb.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bb.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bs-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bs-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bs-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bs.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-bs.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-cmo-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-cmo-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-cmo-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-cmo.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-cmo.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-condmov.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-condmov.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-fmemidx-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-fmemidx-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-fmemidx-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-fmemidx.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-fmemidx.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-mac.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-mac.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-memidx-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-memidx-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-memidx-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-memidx.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-memidx.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-mempair-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-mempair-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-mempair-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-mempair.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-mempair.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-sync-fail.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-sync-fail.l
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-sync-fail.s
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-sync.d
>  create mode 100644 gas/testsuite/gas/riscv/x-thead-sync.s
>
> --
> 2.37.2
>

  parent reply	other threads:[~2022-09-16  9:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-06 12:22 Christoph Muellner
2022-09-06 12:22 ` [PATCH 01/13] RISC-V: Add generic support for " Christoph Muellner
2022-09-06 12:22 ` [PATCH 02/13] RISC-V: Add T-Head CMO vendor extension Christoph Muellner
2022-09-06 12:22 ` [PATCH 03/13] RISC-V: Add T-Head SYNC " Christoph Muellner
2022-09-06 12:22 ` [PATCH 04/13] RISC-V: Add support for arbitrary immediate encoding formats Christoph Muellner
2022-09-06 12:22 ` [PATCH 05/13] RISC-V: Add T-Head Bitmanip vendor extension Christoph Muellner
2022-09-06 12:22 ` [PATCH 06/13] RISC-V: Add T-Head CondMov " Christoph Muellner
2022-09-06 12:22 ` [PATCH 07/13] RISC-V: Add T-Head MAC " Christoph Muellner
2022-09-06 12:22 ` [PATCH 08/13] RISC-V: Add T-Head FMemIdx " Christoph Muellner
2022-09-06 12:22 ` [PATCH 09/13] RISC-V: Add T-Head MemIdx " Christoph Muellner
2022-09-06 12:22 ` [PATCH 10/13] RISC-V: Add support for literal instruction arguments Christoph Muellner
2022-09-06 12:22 ` [PATCH 11/13] RISC-V: Add T-Head MemPair vendor extension Christoph Muellner
2022-09-06 12:22 ` [PATCH 12/13] riscv: gas: Add command line option '-mcpu=' to specify the CPU Christoph Muellner
2022-09-06 12:22 ` [PATCH 13/13] riscv: Add T-Head entries for the -mcpu= flag Christoph Muellner
2022-09-16  9:36 ` Nelson Chu [this message]
2022-09-16  9:58   ` [PATCH 00/13] Add support for the T-Head vendor extensions Palmer Dabbelt
2022-09-18  6:51     ` Christoph Müllner
2022-09-18  6:50   ` Christoph Müllner
2022-09-22 11:08     ` Philipp Tomsich

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='CAPpQWtBiCntsD=443+BZVLT94v0yPDajty-STChLNLfn8WsubQ@mail.gmail.com' \
    --to=nelson@rivosinc.com \
    --cc=binutils@sourceware.org \
    --cc=christoph.muellner@vrull.eu \
    --cc=cooper.qu@linux.alibaba.com \
    --cc=jim.wilson.gcc@gmail.com \
    --cc=kito.cheng@sifive.com \
    --cc=lifang_xia@linux.alibaba.com \
    --cc=palmer@dabbelt.com \
    --cc=philipp.tomsich@vrull.eu \
    --cc=yunhai@linux.alibaba.com \
    --cc=zhiwei_liu@linux.alibaba.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).