Repository navigation
[Triton/Gluon] Add Triton-based Conv3D kernels - #5640
saeid-rostami wants to merge 16 commits into
Conversation
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
PR title tags & labels: |
|
@saeid-rostami, as far as I can see your PRing from your fork. Try to publish your feature branch to ROCm repo and then PR the feature branch to Lots of Triton CI test shards are failing with the following error: This happens because CI isn't logging to Docker Hub, with CI team credentials. This step is skipped when PRing from a fork: - name: Docker login
if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork }}
env:
DOCKER_USERNAME: ${{ secrets[needs.ci_config.outputs.docker_username_secret] }}
DOCKER_PASSWORD: ${{ secrets[needs.ci_config.outputs.docker_password_secret] }} |
Postponing this since it requires opening a new PR. We can move on to a new branch / PR at ROCm fork if Docker rate limiting becomes a real issue in the future. |
brunomazzottiamd
left a comment
There was a problem hiding this comment.
More comments on the Conv3D wrapper function and Conv3D unit tests.
| ) | ||
|
|
||
| _current_arch = get_arch() | ||
| if _current_arch not in ALL_SUPPORTED_ARCHS: |
There was a problem hiding this comment.
Please make sure we can run these unit tests in gfx942 and gfx950 AITER CI runners. They will be a guarantee that future commits won't break Conv3D correctness! Remember that we don't have RDNA runners!
You don't need to tune the kernels for CDNA, just pick any reasonable block size and related meta parameters - be conservative in this regard. I'll be happy if everything is green in CDNA CI (no perf promises at all).
There was a problem hiding this comment.
I think this is because this PR coming from my fork and PR cannot access the Rocm CI, I'll fix this part when all the reviews done.
There was a problem hiding this comment.
PLMK when you're able to push to ROCm fork, then I can give it a quick try locally in some CDNA hosts.
brunomazzottiamd
left a comment
There was a problem hiding this comment.
LGTM! Let's wait for @nidal567 last feedback!
nidal567
left a comment
There was a problem hiding this comment.
I think Rule 4 is not fully satisfied. The Conv3D config files are in the expected architecture folders, and they are placed in their correct respective directories, but the exact-shape pins under gfx942 and gfx950 are copied from the gfx1201 tuning data. The loader resolves exact-shape entries before M_LEQ_* buckets and the generic fallback, so production Conv3D workloads can select launch parameters that were not validated for these target CDNA architectures.
I would say to ensure this is validated across the CDNA archs. The safe fix is to remove the conv3d_* directories from gfx942 and gfx950, or replace them with independently measured CDNA configs. Otherwise the fallback safety path is bypassed and Rule 4 is not met.
Other than that concern, LGTM!
Thanks for pointing this out. I only tuned the Conv3D kernels on RDNA hardware. The gfx942 and gfx950 configurations were initialized using the gfx1201 values so the Conv3D paths could run in AITER CI, same approach that we did on Conv2d PR,. Those variables have not been performance-tuned or fully validated for CDNA. I’m happy to help with that work, but I’m on the RDNA team and do not have access to CDNA machines. If someone with access to those systems help run the configurations and share the results, I can make any necessary adjustments. |
|
@nidal567 and @saeid-rostami, for the Conv2D PR I added untuned Take the following two files as an example.
{
"any": {
"BLOCK_M": 64,
"BLOCK_N": 64,
"BLOCK_K": 64,
"GROUP_SIZE_M": 4,
"num_warps": 4,
"num_stages": 1
}
}
{
"any": {
"BLOCK_M": 64,
"BLOCK_N": 64,
"BLOCK_K": 64,
"GROUP_SIZE_M": 4,
"num_warps": 4,
"num_stages": 1
}
}I think this is the best approach because:
We don't need to tune for CDNA, it's out of scope - we won't do this unless we have a clear demand. Does it work IYO? |
|
@brunomazzottiamd |
|
Closing this PR in favor of #5952, which contains the same Triton Conv3D work Thank you, @brunomazzottiamd and @nidal567, for the reviews and guidance. Please continue |
Motivation
This PR adds Triton Conv3D implementation with optimized
routing for common video-model and VAE workloads.
The public API supports FP16 and BF16 inputs, NCDHW and channels-last-3D
execution, optional bias, asymmetric stride/padding/dilation, and activations.
Technical Details
The Conv3D router selects between the following kernel families:
The implementation also adds:
seeded from the gfx1201 values
aiter.ops.tritonThe benchmark reports per-layer correctness, selected Triton kernels,
kernel-only and repack-inclusive performance, aggregate throughput, and
call-count-weighted encoder/decoder totals. It exits nonzero when a requested
shape errors or fails correctness.
Test Plan
The Conv3D test suite covers:
none,relu,relu6, andgeluactivationsTest Result
Tests were run on an AMD Radeon AI PRO R9700 (
gfx1201) after clearing theTriton compilation cache.
Submission Checklist