Skip to content

CUDA/HIP: optimize mmv paths taken for HIP/CDNA #14324

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 23, 2025
+23 −1
Merged

CUDA/HIP: optimize mmv paths taken for HIP/CDNA #14324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6a414d9
CUDA/HIP: optimize mmv paths taken for HIP/CDNA
IMbackK Jun 22, 2025
66f4c93
CUDA/HIP: use mmv instead of rocblas for batch 3-5 mmv
IMbackK Jun 22, 2025
70774e1
Update ggml/src/ggml-cuda/common.cuh
IMbackK Jun 23, 2025
8bd52b6
restirct to amd
IMbackK Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ggml/src/ggml-cuda/common.cuh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,11 @@ static bool fp16_mma_hardware_available(const int cc) {
}

static bool bf16_mma_hardware_available(const int cc) {
return GGML_CUDA_CC_IS_NVIDIA(cc) && cc >= GGML_CUDA_CC_AMPERE;
return (GGML_CUDA_CC_IS_NVIDIA(cc) && cc >= GGML_CUDA_CC_AMPERE) || GGML_CUDA_CC_IS_CDNA(cc) || cc >= GGML_CUDA_CC_RDNA3;
}

static bool fp32_mma_hardware_available(const int cc) {
return GGML_CUDA_CC_IS_CDNA(cc);
}

// Volta technically had FP16 tensor cores but they work very differently compared to Turing and later.
Expand Down
18 changes: 18 additions & 0 deletions ggml/src/ggml-cuda/mmv.cu
View file
Open in desktop
Copy link
Collaborator

@JohannesGaessler JohannesGaessler Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please encapsulate the AMD-specific logic in explicit GGML_CUDA_CC_IS_AMD checks.

Copy link
Collaborator Author

@IMbackK IMbackK Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think its AMD-specific.
I think its a good heuristic to choose blas over our valu implementation when the device has some form of matrix units, regardless of vendor.

Copy link
Collaborator

@JohannesGaessler JohannesGaessler Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My experience with CUDA has been that even though different GPU generations have the same relevant hardware features the degree of optimization of cuBLAS vs. this kernel was very different. For this reason I would prefer to have the kernel selection logic strictly separated by hardware vendor.

Copy link
Collaborator Author

@IMbackK IMbackK Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really dont think this is sensible, a vendor implementation using matrix units should be expected to perform better than a valu implementation as the default assumption. But i have changed it none the less.

Copy link
Collaborator

@JohannesGaessler JohannesGaessler Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it's a reasonable assumption that the BLAS library released by the hardware vendor is going to optimally utilize the available hardware resources. Empirically I've found though that this is not always the case.

Copy link
Collaborator Author

@IMbackK IMbackK Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well besides being a reasonable assumption our sample size of gpus and datatypes shows that a matrix unit using vendor blas is going to win at batch >= 4 in the vast majority of cases, thus this should be the default.

Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,11 @@ bool ggml_cuda_should_use_mmv(enum ggml_type type, int cc, const int64_t * src0_
return ne11 <= 4;
}
return ne11 <= 3;
} else if (GGML_CUDA_CC_IS_AMD(cc)) {
if (fp32_mma_hardware_available(cc)) {
return ne11 <= 3;
}
return ne11 <= 8;
}
return ne11 <= 8;
case GGML_TYPE_F16:
Expand All @@ -468,6 +473,14 @@ bool ggml_cuda_should_use_mmv(enum ggml_type type, int cc, const int64_t * src0_
return src0_small && ne11 <= 3;
}
return ne11 <= 8;
} else if (GGML_CUDA_CC_IS_AMD(cc)) {
if (fp16_mma_hardware_available(cc)) {
if (GGML_CUDA_CC_IS_RDNA3(cc) || GGML_CUDA_CC_IS_RDNA4(cc)) {
return ne11 <= 5;
}
return ne11 <= 2;
}
return ne11 <= 8;
}
return ne11 <= 8;
case GGML_TYPE_BF16:
Expand All @@ -480,6 +493,11 @@ bool ggml_cuda_should_use_mmv(enum ggml_type type, int cc, const int64_t * src0_
return src0_small && ne11 <= 3;
}
return ne11 <= 8;
} else if (GGML_CUDA_CC_IS_AMD(cc)) {
if (bf16_mma_hardware_available(cc)) {
return ne11 <= 3;
}
return ne11 <= 8;
}
return ne11 <= 8;
default:
Expand Down
Loading