-
Notifications
You must be signed in to change notification settings - Fork 641
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
Load ukernel bitcode as executable_object
at the time of lowering to ukernels.
#19323
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
bjacob
force-pushed
the
users/bjacob/execobj
branch
7 times, most recently
from
December 2, 2024 17:25
0f950fc
to
8cba1a0
Compare
kuhar
reviewed
Dec 2, 2024
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
bjacob
requested review from
MaheshRavishankar,
qedawkins,
Groverkss,
antiagainst and
benvanik
as code owners
December 2, 2024 18:58
kuhar
approved these changes
Dec 2, 2024
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.
LGTM % nits
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
raikonenfnu
reviewed
Dec 2, 2024
raikonenfnu
approved these changes
Dec 2, 2024
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.
Overall refactoring looks great! just minor Q
kuhar
reviewed
Dec 2, 2024
benvanik
requested changes
Dec 3, 2024
compiler/src/iree/compiler/Codegen/LLVMGPU/test/ukernel_pipeline_transform.mlir
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/test/gpu_lower_to_ukernels.mlir
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/test/gpu_lower_to_ukernels.mlir
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/test/gpu_lower_to_ukernels.mlir
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/test/gpu_lower_to_ukernels.mlir
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Codegen/Common/GPU/GPULowerToUKernels.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Benoit Jacob <jacob.benoit.1@gmail.com>
bjacob
force-pushed
the
users/bjacob/execobj
branch
from
December 3, 2024 20:24
402b9c1
to
ea29fad
Compare
Signed-off-by: Benoit Jacob <jacob.benoit.1@gmail.com>
benvanik
approved these changes
Dec 3, 2024
giacs-epic
pushed a commit
to giacs-epic/iree
that referenced
this pull request
Dec 4, 2024
…o ukernels. (iree-org#19323) 1. Moves the time of loading ukernel bitcode from `serializeExecutable` to the `GPULowerToUKernels` pass. 2. The determination of whether an op can lower to a ukernel, is now based on whether the expected bitcode file is found. This allows removing several utility functions that implemented similar logic in different places. 3. The `GPULowerToUKernels` pass searches for existing bitcode in a `hal.executable.objects` attribute, and only loads the embedded ukernel bitcode if that wasn't found, and in either case ensures that that resulting ukernel op has a `hal.executable.objects` attribute containing the necessary IR. This has several nice implications: - The IR becomes completely self-contained: a ukernel op is no longer an opaque interface to some bitcode at-a-distance. - This solves the problem of allowing contributing one's own bitcode from the outside. Users can write their own `hal.executable.objects`. - De-duplication of bitcode is handled by the HoistExecutableObjects pass. - Linking bitcode is handled by generic linker code linking executable objects. - The only useful custom handling of ukernel symbols, was adding `AlwaysInline` function attributes. This PR moves these attributes to the ukernel source code: `[[clang::always_inline]]`. I verified that these result in the expected `alwaysinline` in the bitcode. 4. The ukernel bitcode is part of the ROCM plugin. The `serializeExecutable` implementation, which was the consumer of that data, is also in the ROCM plugin. But the `GPULowerToUKernels` pass, which is the new consumer, is outside of that plugin. So this required creating a mechanism to export such embedded data files from the ROCM plugin to the outside. That is solved by the new `EmbeddedDataDirectory` utility. --------- Signed-off-by: Benoit Jacob <jacob.benoit.1@gmail.com> Signed-off-by: Giacomo Serafini <179146510+giacs-epic@users.noreply.github.com>
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.
serializeExecutable
to theGPULowerToUKernels
pass.GPULowerToUKernels
pass searches for existing bitcode in ahal.executable.objects
attribute, and only loads the embedded ukernel bitcode if that wasn't found, and in either case ensures that that resulting ukernel op has ahal.executable.objects
attribute containing the necessary IR. This has several nice implications:hal.executable.objects
.AlwaysInline
function attributes. This PR moves these attributes to the ukernel source code:[[clang::always_inline]]
. I verified that these result in the expectedalwaysinline
in the bitcode.serializeExecutable
implementation, which was the consumer of that data, is also in the ROCM plugin. But theGPULowerToUKernels
pass, which is the new consumer, is outside of that plugin. So this required creating a mechanism to export such embedded data files from the ROCM plugin to the outside. That is solved by the newEmbeddedDataDirectory
utility.