-
Notifications
You must be signed in to change notification settings - Fork 603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce RVV VLS code-gen #5199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. LGTM-ing it.
@asaadaldien FYI. No need to review, but just in case something stands out to you.
Also, want to make sure that as we build on the LLVM backend, there might be regressions on your side that we wont see. If that happens more frequently, we can consider adding a different backend?
@@ -38,7 +38,7 @@ $ cmake -G Ninja -B ../iree-build-host/ -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COM | |||
$ cmake --build ../iree-build-host/ --target install | |||
``` | |||
|
|||
Debugging note: if `IREE_LLVMAOT_LINKER_PATH` is set for targeting Android then the build above will fail, and you should run `unset IREE_LLVMAOT_LINKER_PATH`. | |||
Debugging note: if `IREE_LLVMAOT_LINKER_PATH` is set for targeting RISC-V then the build above will fail, and you should run `unset IREE_LLVMAOT_LINKER_PATH`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change from Android to RISC-V. Presumably this is still the case for Android?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to set risk-v linker path . If unset IREE_LLVMAOT_LINKER_PATH
it will use the default system linker (e.g /usr/bin/ld
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my experience, IREE_LLVMAOT_LINKER_PATH should only be set when iree-translate generates the LLVM backend target. Setting the environment variable would cause both host toolchain and target compilation to fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it occurs in every cross-compiled platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed you need to scope this variable during switching between translation to different targets.
I am not familiar with risk-v tooling, but if there is a canonical toolchain you can consider adding risk-v linker tool, see example here here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. I'll take look at that. Maybe we can have a patch in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A rv-specific linker tool can be useful. Right now it goes through the UnixLinkerTool.cpp
path that also creates clang warning message w.r.t unused compile arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also https://github.com/google/iree/blob/d496bc9d53e918c0362ddd498ded750eba6050f3/iree/compiler/Dialect/HAL/Target/LLVM/internal/LinkerTools.cpp#L37-L45
for where linker tools are selected based on the target triple
2c8e15c
to
4024fec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the extra prebuilt information!
4024fec
to
8918178
Compare
* da64c93 Integrate MLIR-EmitC at iml130/mlir-emitc@dde739f (#5228) * a8d6c2a Fix doc publication by escaping our IR snippets in markdown. (#5227) * bd5e535 Introduce RVV VLS code-gen (#5199) * d496bc9 Avoid allocating temporary buffer for tensors derived from read-only tensors (.. * 1a93557 Tidying up a few pages of documentation. (#5225) * 46e8331 Add a dedicated iree_c_module CMake module (#5214) * 603b208 Merge google -> main (#5219) * c9f3742 Add support for control flow lowering in the VM to emitc target (#5208) * c8a7b2f Sort descriptors as expected by the native module (#5207) * 47183fb Revise compiler tracing to enable statistics aggregation. (#5218) * 500ec7b Set author to match committer for llvm submodule update action. (#5217) * 881de81 Generate and use the iree_hal_executable_library_t metadata. (#5195) * cf8180e Increase K tile size for small matrices. (#5213) * f5804ec Fix problem bug in MobileBert with vectorization enable. (#5211) COPYBARA_INTEGRATE_REVIEW=#5229 from ScottTodd:main-to-google da64c93 PiperOrigin-RevId: 365076337
* da64c93 Integrate MLIR-EmitC at iml130/mlir-emitc@dde739f (#5228) * a8d6c2a Fix doc publication by escaping our IR snippets in markdown. (#5227) * bd5e535 Introduce RVV VLS code-gen (#5199) * d496bc9 Avoid allocating temporary buffer for tensors derived from read-only tensors (.. * 1a93557 Tidying up a few pages of documentation. (#5225) * 46e8331 Add a dedicated iree_c_module CMake module (#5214) * 603b208 Merge google -> main (#5219) * c9f3742 Add support for control flow lowering in the VM to emitc target (#5208) * c8a7b2f Sort descriptors as expected by the native module (#5207) * 47183fb Revise compiler tracing to enable statistics aggregation. (#5218) * 500ec7b Set author to match committer for llvm submodule update action. (#5217) * 881de81 Generate and use the iree_hal_executable_library_t metadata. (#5195) * cf8180e Increase K tile size for small matrices. (#5213) * f5804ec Fix problem bug in MobileBert with vectorization enable. (#5211) PiperOrigin-RevId: 365076337
No description provided.