[PATCH] D153748: [RISCV] Add support for XCValu extension in CV32E40P

[Jeremy Bennett via Phabricator via llvm-commits]

jeremybennett added inline comments.


================
Comment at: llvm/docs/RISCVUsage.rst:278
+``XCValu``
+  LLVM implements `version 1.3.1 of the Core-V ALU custom instructions specification <https://github.com/openhwgroup/cv32e40p/blob/4f024fe4b15a68b76615b0630c07a6745c620da7/docs/source/instruction_set_extensions.rst>`_ by Core-V.  All instructions are prefixed with `cv.mac.` as described in the specification. These instructions are only available for riscv32 at this time.
----------------
liaolucy wrote:
> realqhc wrote:
> > craig.topper wrote:
> > > kito-cheng wrote:
> > > > realqhc wrote:
> > > > > kito-cheng wrote:
> > > > > > @jeremybennett say it's 1.0 here?
> > > > > > 
> > > > > > https://github.com/riscv-non-isa/riscv-toolchain-conventions/blob/master/README.mkd#list-of-vendor-extensions
> > > > > This might need to be updated as if you check https://github.com/openhwgroup/cv32e40p/releases, there has been release of v1.3.2 last week. 
> > > > I saw binutils from openhwgroup still using 1.0[1], and that patch is committed 5 days ago? I am not intend to block this patch, but I would like to figure out what's the right version for those extension.
> > > > 
> > > > [1] https://github.com/openhwgroup/corev-binutils-gdb/commit/9b120787924eed13455de3062523539dc39bc7f8
> > > The cv.mac part was copied from another description and does not apply here.
> > Should I change it to cv. like XCVbitmanip or do you think cv.alu is more appropriate here?
> Thanks, about the version, we have a group meeting on Friday, we will confirm it internally and come back to synchronize the information
The document referred to is the entire specification of the CV32E40P, of which its ISA extensions are just a small section.  The main document is subject to numerous small changes, and bumps its minor version and patch version from time to time.  However the ISA extensions from a software perspective have a single version, 1.0, which is the only version that will be supported by the tool chain.

I'm open to advice on how we should handle this. I note that `XCVbitmanip` and `XCVmac` have both been merged using the full document reference (1.3.1).  It might be clearer to reference the software spec on ISA extension naming, which explicitly notes that versioning is *not* supported for CORE-V: https://github.com/openhwgroup/core-v-sw/blob/master/specifications/corev-isa-extension-naming.md


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153748/new/

https://reviews.llvm.org/D153748