[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.
----------------
jeremybennett wrote:
> 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
Just to clarify.  The CORE-V ISA extensions do not support versioning, which in a RISC-V context means the version is always 1.0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153748