fix cutlass_3x_gemm_fp8_blockwise on sm103a - #32224
Conversation
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
There was a problem hiding this comment.
Code Review
This pull request aims to fix an issue with cutlass_3x_gemm_fp8_blockwise on the sm103a architecture by introducing a new preprocessor guard, enable_sm100f_only, which correctly includes sm103a. The change also adds compiler warnings for kernels used on unsupported architectures, which is a good practice. However, the implementation of these warnings is flawed in several places, causing them to be triggered during host code compilation, which can be very noisy. My review includes suggestions to correct this behavior so that warnings are only shown for device code compilation on unsupported architectures.
| CUTLASS_DEVICE void operator()(Args&&... args) { | ||
| #if defined __CUDA_ARCH__ && __CUDA_ARCH__ >= 900 | ||
| Kernel::operator()(std::forward<Args>(args)...); | ||
| #else |
There was a problem hiding this comment.
The use of #else here will cause a compiler warning to be emitted during host-side compilation, as __CUDA_ARCH__ is not defined. This can lead to a large number of unnecessary warnings. To ensure the warning is only triggered for device compilation on an unsupported architecture, you should use #elif defined(__CUDA_ARCH__). This pattern is already correctly used for enable_sm100f_only in this same file.
#elif defined(__CUDA_ARCH__)| CUTLASS_DEVICE void operator()(Args&&... args) { | ||
| #if defined __CUDA_ARCH__ && __CUDA_ARCH__ == 900 | ||
| Kernel::operator()(std::forward<Args>(args)...); | ||
| #else |
There was a problem hiding this comment.
The use of #else here will cause a compiler warning to be emitted during host-side compilation, as __CUDA_ARCH__ is not defined. This can lead to a large number of unnecessary warnings. To ensure the warning is only triggered for device compilation on an unsupported architecture, you should use #elif defined(__CUDA_ARCH__). This pattern is already correctly used for enable_sm100f_only in this same file.
#elif defined(__CUDA_ARCH__)| CUTLASS_DEVICE void operator()(Args&&... args) { | ||
| #if defined __CUDA_ARCH__ && __CUDA_ARCH__ == 1000 | ||
| Kernel::operator()(std::forward<Args>(args)...); | ||
| #else |
There was a problem hiding this comment.
The use of #else here will cause a compiler warning to be emitted during host-side compilation, as __CUDA_ARCH__ is not defined. This can lead to a large number of unnecessary warnings. To ensure the warning is only triggered for device compilation on an unsupported architecture, you should use #elif defined(__CUDA_ARCH__). This pattern is already correctly used for enable_sm100f_only in this same file.
#elif defined(__CUDA_ARCH__)| CUTLASS_DEVICE void operator()(Args&&... args) { | ||
| #if defined __CUDA_ARCH__ && __CUDA_ARCH__ == 1200 | ||
| Kernel::operator()(std::forward<Args>(args)...); | ||
| #else |
There was a problem hiding this comment.
The use of #else here will cause a compiler warning to be emitted during host-side compilation, as __CUDA_ARCH__ is not defined. This can lead to a large number of unnecessary warnings. To ensure the warning is only triggered for device compilation on an unsupported architecture, you should use #elif defined(__CUDA_ARCH__). This pattern is already correctly used for enable_sm100f_only in this same file.
#elif defined(__CUDA_ARCH__)4575a3c to
764db01
Compare
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
764db01 to
42d3ba8
Compare
64b930e to
b859962
Compare
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
| Kernel::operator()(std::forward<Args>(args)...); | ||
| #else | ||
| printf("This kernel only supports sm100a.\n"); | ||
| asm("trap;"); |
There was a problem hiding this comment.
Could you check, in case of real fail, will this print appears?
There was a problem hiding this comment.
Yes. I have tested and the print appears.
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
|
Thanks for the identifying and fixing this issue! |
|
Looks like the ci/cd failed due to some bugs in the |
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com> Co-authored-by: Pavani Majety <pmajety@nvidia.com> Signed-off-by: Pai <416932041@qq.com>
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com> Co-authored-by: Pavani Majety <pmajety@nvidia.com> Signed-off-by: Pai <416932041@qq.com>
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com> Co-authored-by: Pavani Majety <pmajety@nvidia.com> Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com> Co-authored-by: Pavani Majety <pmajety@nvidia.com>
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com> Co-authored-by: Pavani Majety <pmajety@nvidia.com>
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com> Co-authored-by: Pavani Majety <pmajety@nvidia.com>
Purpose
When compiling with sm103a, the output of
cutlass_3x_gemm_fp8_blockwiseis garbage value.Updated the helpers in
csrc/cutlass_extensions/common.hppto include sm103a.Also added a runtime error message when the kernel is executed but not compiled.
Test Plan
pytest tests/kernels/quantization/test_block_fp8.py::test_w8a8_block_fp8_cutlass_matmulTest Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.