[mlir][RISCVV][RFC] Initial RISCVV Dialect

[zhanghb97]

Summary

This is the inital patch for RISC-V vector extension (RVV) dialect. This patch include:

• RVV Dialect Definition
  • RVV Scalable Vector Type
  • RVV Operations
  • RVV Intrinsic Operations
• Translation from RVV Dialect to LLVM Dialect

[zhanghb97]

FYI - I wrote the RFC in the LLVM discourse.

[aartbik]

some initial comments, we are also having a discussion at discourse in the RFC thread

mlir/include/mlir/Dialect/RVV/RVV.td

27–29 ↗

(On Diff #368053)

This is the part that will show up on the public doc website. Perhaps you can give a bit more background here, and link to the proper places to find out more onthe RISC-V Vector ops you introduce here.

39 ↗

(On Diff #368053)

this is copy and past from ArmSVE. We should really try to lift this to a shared definition at the proper place

87 ↗

(On Diff #368053)

empty line after this separator for consistency

mlir/lib/Dialect/RVV/Transforms/LegalizeForLLVMExport.cpp

150 ↗

(On Diff #368053)

period at tend

mlir/test/Dialect/RVV/roundtrip.mlir

3 ↗

(On Diff #368053)

here and below, use

CHECK-LABEL:

[rriddle]

nit on the naming, rvv feels a bit too general. I'd rather go with a longer name that spells out riscv: riscvv or something related.

[Joejiong]

mlir/lib/Dialect/RVV/Transforms/LegalizeForLLVMExport.cpp

157 ↗

(On Diff #368053)

double // added

[zhanghb97]

• Add more background about RVV and introduction to operations in this dialect.
• Add CHECK-LABELs for the round trip test.
• Fix some format issues.

[zhanghb97]

In D108536#2963181, @rriddle wrote:

nit on the naming, rvv feels a bit too general. I'd rather go with a longer name that spells out riscv: riscvv or something related.

Some communities tend to use RVV as the name. For example, the rvv-intrinsic-doc and OpenCV wide universal intrinsic definition: intrin_rvv.hpp. So I also use the RVV on the MLIR side at the very beginning.

If the "rvv/RVV" is too general, and to be consistent with MLIR naming convention (arm_sve/ArmSVE, x86vector/X86Vector, etc.), how about using :

• "riscv_vector" for the namespace
• "RISCVVector" for the file name, folder name, etc.

What do you think about these names?

mlir/include/mlir/Dialect/RVV/RVV.td

39 ↗

(On Diff #368053)

I think we can make the vector type support both static and scalable versions, for example:

• static version for SIMD architecture vector<4xi64>
• scalable version for vector architecture vector<?x4xi64>

And different ISAs can have different semantics. What do you think about this method?
I will spend some time figuring out how to implement this idea.

mlir/lib/Dialect/RVV/Transforms/LegalizeForLLVMExport.cpp

150 ↗

(On Diff #368053)

Sorry, I don’t understand what "period at tend" means.

[zhanghb97]

As for the integration test, I currently encounter some problems, and I will continue to find some solutions. At the same time, I will try to move the scalable vector type into the vector dialect in the next step.

And for the names, if the "rvv" is to general, we can use the following names:

• "riscv_vector" for the namespace
• "RISCVVector" for the file name, folder name, etc.

This needs further confirmation.

[Joejiong]

mlir/include/mlir/Dialect/RVV/RVV.td

29 ↗

(On Diff #369225)

a small typo: pleas -> please

[zhanghb97]

• Fix typo.
• Lift the scalable vector type to vector dialect. The SVE and RVV can share the definition now.

[zhanghb97]

mlir/include/mlir/Dialect/RVV/RVV.td

39 ↗

(On Diff #368053)

After further thinking, I think it is better for SVE and RVV to share the current scalable vector type definition. The unified scalable vector type (vector<?x4xi64> or something else) needs more exploration in the future. And I have lifted the definition to make the scalable vector type a shared version.

[zhanghb97]

For the next step, I will try to give better semantics to the type of RVV:

• let the vector type directly map LMUL and SEW.
• add a type conversion when lower the RVV operation to RVV intrinsic operation.

[jurahul]

mlir/lib/Dialect/RVV/Transforms/LegalizeForLLVMExport.cpp

150 ↗

(On Diff #368053)

period at end. https://llvm.org/docs/CodingStandards.html#commenting ("using proper capitalization, punctuation, etc").

[zhanghb97]

• Naming (RVV -> RISCVV)
• Scalable Vector Type (Common Scalable Vector Type + Separate RISC-V Scalable Vector Type)
  • Lift Scalable Vector Type to Vector Dialect
  • Define RVV LMUL Type, RVV Mask Type, and RVV Vector Type
  • Implement Type Mapping Process

[zhanghb97]

mlir/lib/Dialect/RVV/Transforms/LegalizeForLLVMExport.cpp

150 ↗

(On Diff #368053)

Thanks for informing this!

[aartbik]

In D108536#2970377, @zhanghb97 wrote:

As for the integration test, I currently encounter some problems, and I will continue to find some solutions.

Have you made more progress on this? I realize this is a bit of a hazzle, but leaving it as a TODO has a very high risk of not being done at all... :-(

[zhanghb97]

In D108536#3025499, @aartbik wrote:

Have you made more progress on this? I realize this is a bit of a hazzle, but leaving it as a TODO has a very high risk of not being done at all... :-(

I also realize the risk of not having the integration test. If LLVM IR intrinsics are changed (e.g. add a argument like this patch), the unit tests on MLIR will not be affected, but there will be problems when translating to LLVM IR.

I am trying to figure out why the cross-compiled lli and mlir-cpu-runner fail to identify the target with the QEMU, And I will also use a RISC-V machine to have a try.

[zhanghb97]

• Rebase
• Using OpAdaptor. sync to commit b54c72
• Adding vta argument. sync to commit 7d39a8
• Removing context argument from parse function. sync to commit fb093c

WIP: integration test

[zhanghb97]

• Sync to the vector type with scalable dimensions.
• Set the vta as an attribute.
• Add setvl operation.

[zhanghb97]

• Some RISC-V + JIT progress (needed by integration test)

My teammate has sent some patches hoping to support JIT for the RISC-V side.
Here I quote some sentences of his summary to show the point of the challenge:

In RISCV, temporary symbols will be used to generate dwarf, eh_frame sections..., and will be placed in object code's symbol table. However, LLVM does not use names on these temporary symbols.

For more details, please see his patches:

https://reviews.llvm.org/D116475

https://reviews.llvm.org/D116794

[jsetoain]

mlir/include/mlir/Dialect/RISCVV/RISCVV.td

38–44

Since this is not specific to RISC-V Vector, it should go along the other LLVM Dialect constraints (LLVMOpBase.td).

[zhanghb97]

Use the LLVM_AnyPointer constraints in LLVMOpBase.td instead of a local one.

[zhanghb97]

mlir/include/mlir/Dialect/RISCVV/RISCVV.td

38–44

Thanks for the suggestion! When I checked the constraints list in LLVMOpBase.td, I found there is already LLVM_AnyPointer constraint. (I didn't notice it before 😅)

[zhanghb97]

• Add integration test (test-riscvv-arithmetic, test-riscvv-memory, test-riscvv-stripmining).
• Add ConvertPolicyOperandOpToLLVMPattern and ConvertPassthruOperandOpToLLVMPattern for mask/tail policy in the RISC-V LLVM IR intrinsics.

[zhanghb97]

I provide an environment setup document to show how to build the toolchain and perform the integration tests.

[zhanghb97]

Rebase.