RISC V vector predication support intrinsics support #7119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 30 commits
September 16, 2022 19:27
useful for RISC V, but it may be a simpler, better optimized path, for Halide vector operations in general. Add support for a maximum vector size that might be larger than the native vector size. RISC V vector LMUL support is an example of an architecture supporting this.
promotion contexts.
vector predication intrinsics.
improving the calling convention and naming of the new routines to generate the intrinsics.
caveperson programmer habits die hard. Improve comments.
concatenation into one line.
Change TODO(zalman) to TODO(zvookin) uniformly. Few other cleanups.
strided load for dense case. Add some comments.
rootjalex
reviewed
Oct 25, 2022
rootjalex
reviewed
Oct 25, 2022
rootjalex
reviewed
Oct 25, 2022
|
@steven-johnson Do the buildbots test RISC-V cross-compilation? If so, this PR should probably make an addition to simd_op_check, to check for the added vector instructions in this PR. |
Not currently, no. We should definitely add that. |
|
Added an issue for simd_op_check coverage. #7122 |
|
Yeah, I'd rant that not having the rounding mode flags in the intrinsic at
the start is badly broken, but it's sort of how things are with LLVM and
RISC V. Really they ought to be in the opcode, but you know, bits are
expensive. I put in references to a couple PRs for LLVM but it doesn't
look like there is recent movement on this.
…On Tue, Oct 25, 2022 at 2:09 PM Andrew Adams ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/CodeGen_RISCV.cpp
<#7119 (comment)>:
> + llvm::Function *wrapper =
+ llvm::Function::Create(wrapper_ty, llvm::GlobalValue::InternalLinkage,
+ wrapper_name, module.get());
+ llvm::BasicBlock *block =
+ llvm::BasicBlock::Create(module->getContext(), "entry", wrapper);
+ llvm::IRBuilderBase::InsertPoint here = builder->saveIP();
+ builder->SetInsertPoint(block);
+
+ // Set vector fixed-point rounding flag if needed for intrinsic.
+ bool round_down = intrin.flags & RISCVIntrinsic::RoundDown;
+ bool round_up = intrin.flags & RISCVIntrinsic::RoundUp;
+ if (round_down || round_up) {
+ internal_assert(!(round_down && round_up));
+ llvm::Value *rounding_mode = llvm::ConstantInt::get(xlen_type, round_down ? 2 : 0);
+ // TODO: When LLVM finally fixes the instructions to take rounding modes,
+ // this will have to change to passing the rounding mode to the intrinsic.
Is there an existing LLVM issue for it? Having to use inline assembly to
use any of the ops that rely on a rounding flag seems ... unfinished.
—
Reply to this email directly, view it on GitHub
<#7119 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALOUFEADYCC7HI3PIWVT6LWFBEARANCNFSM6AAAAAARNTLZIE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
added 6 commits
October 26, 2022 04:04
std::variant for mask was not handled correctly.
arg, or result type if no vector argument.
intrinsic. (Needed for loads.) Fix result type of vector predication reductions.
|
Had to do a number of bug fixes on the vector predication work as I messed up on the test coverage previously. It works better with the code in this branch... |
|
I believe I have addressed the feedback raised. In a couple cases, by opening tracking issues, but mostly by incorporating the changes suggested or fixing the problems raised. If I've missed anything please let me know. I also did some better testing. Obviously that should get rolled into the real tests, which is effectively the simd_op_check issue. This should be ready to land. |
steven-johnson
approved these changes
Oct 26, 2022
added 2 commits
October 26, 2022 17:06
widths in RISC V intrinsics.
|
I improved naming and comments around the handling of all the integer type widths. |
Vector efficiency improvement oer code review feedback.
ardier
pushed a commit
to ardier/Halide-mutation
that referenced
this pull request
Mar 3, 2024
Turn on vector predication support for RISC V. (First architecture to use this code. Bug fixes included here.) Add architecture specific vector intrinsics support as well. Should not affect anything outside of RISC V.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Have RISC V use LLVM vector predication intrinsics and add support for some architecture specific intrinsics.