Support for P extension version 0.9.11
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Unit Tests
Event Timeline
| clang/include/clang/Basic/DiagnosticSemaKinds.td | ||
|---|---|---|
| 11170–11175 ↗ | (On Diff #332560) | I seem to recall another patch that includes generalising this to take the extension name rather than adding multiple copies of the same thing |
| clang/lib/AST/ASTContext.cpp | ||
| 4037 ↗ | (On Diff #332560) | Upper-case parameter name |
| clang/test/CodeGen/builtins-riscv-rv32p.c | ||
| 63 ↗ | (On Diff #332560) | One function per test and update_cc_test_checks.py would be a lot nicer IMO |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
| 961–978 | This seems like it will interact poorly with V if both are present | |
| llvm/test/CodeGen/RISCV/intrinsics-rv32p.ll | ||
| 5 ↗ | (On Diff #332560) | This is an awful test. One function per intrinsic, and no unnecessary alloca's. |
Comment Actions
What are we gaining from making the intrinsics use vector types if no vector operations are supported other than the intrinsics? Why can't we just use an xlen integer type?
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 952 | What about bitcast from float/double to any of these vector types? I'm guess that's not legal. | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 961–978 | Indeed. I don't think we've really thought about how both of these could co-exist in llvm. I'm not sure they can. We don't have the context to decide upon how to lower an operation (to V or P) since we're just given the type. Do we have to error if both are enabled simultaneously? Maybe if P only expects to be lowered via intrinsics we can fudge it. It's not pretty though. I came across this issue in a previous architecture where we had a v2i32 (or something) able to be lowered both via "scalar" and "vector" instructions. | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 952 | Yes, it is not legal bitcast from float/double to any of these vector types. | |
| clang/lib/CodeGen/CGBuiltin.cpp | ||
|---|---|---|
| 19834 | I have concern here. It has lots of duplicate code if the code style is the same as B in the top. | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 952 | Oh, it is a issue. I will try to fix it. | |
| clang/lib/CodeGen/CGBuiltin.cpp | ||
|---|---|---|
| 19833 | Please undef the macros when you're done with them. | |
| 19834 | Each assignment to IntrinsicTypes invokes the SmallVector constructor so each one generates a call in the final binary unless the compiler does a good job of merging them. | |
| 19834 | Please put this above the vector extension since the comment says "vector builtins are handled from here" | |
| llvm/include/llvm/IR/IntrinsicsRISCV.td | ||
| 1634 | I haven't followed the P extension closely. Why do we need 2 kinds of intrinsics for most instructions? | |
| 1634 | RISCVUnary is a generic name, but the scalar and vector intrinsic inside make it P extension specific. | |
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
| 950 | You probably need handling for insert_vector_elt and extract_vector_elt or some of the expansions will probably break. | |
Comment Actions
Only define 1 kind of intrinsic for most intructions, which
operand type and result type are the same scalar or vector type.
But s[z]unpkd* instructions have two kinds of intrinsics, extra
one has operand element width is half of result element width but
with more elements.
Refine code for IR codegen in CGBuiltin.cpp to look like B extension's implementation.
| llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
|---|---|---|
| 950 | I will implement it in another patch for codegen. | |
| llvm/test/CodeGen/RISCV/rvp/intrinsics-rv32p.ll | ||
|---|---|---|
| 25 ↗ | (On Diff #338017) | I'm still not clear why we need to have two different ways to do the same operation. Why isn't the C interface all scalar types or all vector types? How do users choose which interface to use? |
Comment Actions
Also can you please explain the vector codegen plan at a high level? Do you intend to support auto vectorization or just using vector_size in C?
| clang/include/clang/Basic/Builtins.def | ||
|---|---|---|
| 41 ↗ | (On Diff #338017) | followd -> followed |
Comment Actions
Currently, it just supports vector type operation (like v4i8+v4i8=>add8) in my local patch.
| llvm/test/CodeGen/RISCV/rvp/intrinsics-rv32p.ll | ||
|---|---|---|
| 25 ↗ | (On Diff #338017) | I will ask P extension intrinsic designer about your concern. |
Comment Actions
Update to P extension spec 0.96
- Remove CLO intrinsics
- Change intrinsic preifx from rv to __rv_
Please undef the macros when you're done with them.