Enable using bare pointers for GPU kernels with static shape, and then use that support #690
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
sjw36
approved these changes
Jul 11, 2022
|
Update: looks like we need to poke at how MIOpen calls us... |
|
We're now |
krzysz00
force-pushed
the
use-bare-pointer-conv
branch
2 times, most recently
from
edc905e to
0cd8182
Compare
krzysz00
force-pushed
the
use-bare-pointer-conv
branch
5 times, most recently
from
7366727 to
232bc39
Compare
krzysz00
force-pushed
the
use-bare-pointer-conv
branch
from
232bc39 to
cd80dc5
Compare
|
(Update, @jungpark-mlir might be the person to tag in for MIGraphX? ) |
krzysz00
force-pushed
the
use-bare-pointer-conv
branch
from
cd80dc5 to
c09ab72
Compare
|
https://github.com/ROCmSoftwarePlatform/AMDMIGraphX/blob/7dcae03776938bf983ef6184a13bb9bcada678b5/src/targets/gpu/mlir.cpp#L623 |
krzysz00
force-pushed
the
use-bare-pointer-conv
branch
from
c09ab72 to
cb840be
Compare
|
Blocked on @zhanglx13 getting his changes to finding MLIR into MIOpen, since we don't want both those changes to be one MIOpen PR ROCm/MIOpen#1673 |
jungpark-mlir
approved these changes
Aug 10, 2022
| @@ -18,6 +18,8 @@ | |||
| extern "C" { | |||
| #endif | |||
|
|
|||
| #define MLIR_MIGRAPHX_DIALECT_API_VERSION 2 | |||
|
|
|||
There was a problem hiding this comment.
Can we have a comment here to keep the features added/deleted per each version?
krzysz00
force-pushed
the
use-bare-pointer-conv
branch
from
0ff9ad5 to
07b8cd3
Compare
turneram
pushed a commit
to ROCm/AMDMIGraphX
that referenced
this pull request
Aug 12, 2022
Once ROCm/rocMLIR#690 lands, the ABI for MLIR-generated kernels will change. This commit prepares MIGraphX for the change by conditionally selecting the new ABI if MLIR reports a sufficiently high API version in its headers.
krzysz00
force-pushed
the
use-bare-pointer-conv
branch
from
07b8cd3 to
15804eb
Compare
junliume
pushed a commit
to ROCm/MIOpen
that referenced
this pull request
Aug 24, 2022
This is a companion PR to ROCm/rocMLIR#690 and should not be merged until that lands. These changes will need to be revisited if the MLIR solver ever supports dynamic shapes.
In the ROCm runtime (and probably CUDA as well), all kernel arguments are aligned. Therefore, enable using bare pointers for memref arguments to kernels when these memrefs have static shape and a trivial layout. This is a substantial optimization to launching kernels that use memrefs with known, static sizes, since it causes the kernel launch packet to no longer include information already known to the kernel, which can enable packing the kernel launch arguments into launch packets instead of having to allocate an entire separate structure to hold unneeded memref information. Reviewed By: ftynse Differential Revision: https://reviews.llvm.org/D130716
Keeping with the external changes that enable using bare pointer memref args in gpu kernels, turn on that option for our pipelines and runners by default, but add options to disable bare pointers for cases (either currently existing or upcoming) where memrefs of unknown shape come in to the picture. Fixes ROCm/rocMLIR-internal#583
krzysz00
force-pushed
the
use-bare-pointer-conv
branch
from
15804eb to
9f35ba7
Compare
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.
The changes needed to make this work upstream are at https://reviews.llvm.org/D130716 and have landed - a copy of that commit is on this branch