[PATCH] D133063: [docs] Add a RISC-V Usage page

[Philip Reames via Phabricator via llvm-commits]

reames marked 2 inline comments as done.
reames added inline comments.


================
Comment at: llvm/docs/RISCVUsage.rst:18
+
+The specification defines three base instruction sets: RV32I, RV64I, and RV128I.
+Currently, LLVM supports RV32I, and RV64I, but not RV128I.
----------------
asb wrote:
> I think RV32E is typically described as a base instruction set too (albeit one that is actually a subset of RV32I).
Do we support RV32E?  You're right that this seems to be a base instruction set.  If we do support it, how do we name that argument wise?

For now, I updated the text to indicate we didn't support RV32E, but I'm unsure if this is correct.


================
Comment at: llvm/docs/RISCVUsage.rst:28-29
+     ============ ==============================================================
+     ``riscv32``   RISC-V with ELEN=32
+     ``riscv64``   RISC-V with ELEN=64
+     ============ ==============================================================
----------------
jrtc27 wrote:
> Too much time spent with RVV?...
Apparently, good catch.


================
Comment at: llvm/docs/RISCVUsage.rst:51
+     ``M``          Supported
+     ``Q``          Supported
+     ``V``          Supported
----------------
jrtc27 wrote:
> I don't believe so?
I'm unclear.  I thought it was supported, but hadn't investigated in detail.  Removed for now, will move to future patch.  


================
Comment at: llvm/docs/RISCVUsage.rst:53
+     ``V``          Supported
+     ``Zicsr``      Supported
+     ``Zifencei``   Supported
----------------
asb wrote:
> jrtc27 wrote:
> > .. ish, the CSRs are there but we're still at I2P0 so Zicsr doesn't really exist
> Plus Zicbop, Zicboz, Zicbom
@jrtc27 I don't follow your comment here.  For the moment, I'm going to remove these.  

@asb I'd deliberately left those out for the moment as I wasn't sure if the C intrinsics were implemented.  I'll add in a follow on patch once I've double checked.  


================
Comment at: llvm/docs/RISCVUsage.rst:54
+     ``Zicsr``      Supported
+     ``Zifencei``   Supported
+     ``Zba``        Supported
----------------
jrtc27 wrote:
> ditto
@jrtc27 Moved to follow up.  I'd checked this one and thought we did support IFENCE, but maybe I was mistaken.


================
Comment at: llvm/docs/RISCVUsage.rst:58
+     ``Zbc``        Supported
+     ``Zbs``        Supported
+     ``Zve32x``     Unsupported
----------------
asb wrote:
> Also Zbkb, Zbkc, Zbkx. MC layer support for Zfinx/Zdinx too
> 
> Plus various Zk* extensions.
Let's do these in follow up patches.


================
Comment at: llvm/docs/RISCVUsage.rst:64
+     ``Zve64d``     Supported
+     ``Zvl32b``     Unsupported
+     ``Zvl64b``     Supported
----------------
kito-cheng wrote:
> Maybe mark as MC-layer only?
Is it even supported by assembler?  It could be, but I hadn't thought to check.  


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

https://reviews.llvm.org/D133063