HIP: use hipBLAS for dense prefill on gfx900, keep MMQ for MoE - #24588
Conversation
github-actions
Bot
added
Nvidia GPU
sanmai
left a comment
sanmai
left a comment
There was a problem hiding this comment.
That's pretty cool! I hope this eventually gets reviewed 🤞
There was a problem hiding this comment.
MMQ is not "emulated" when there is no dp4a, its just a slower path. Please remove the misleading comment. Otherwise this is fine, you could try tuneing mmq better for gfx900, but then again im not sure its worth the effort for an architecture this old.
For the record, did you test this on an old rocm version where amd still compiled gfx900, the new preview builds where amd again re-added support for gfx900 or distro/self compiled rocm?
|
Removed comment, thanks! Yeah, I'd agree, so architecture is so old I also don't know if it's worth the effort to explore. I tested using the official AMD 6.3.3 build from their repo, I installed using this on a fresh install of Ubuntu 22.04 LTS: https://repo.radeon.com/rocm/installer/rocm-linux-install-offline/rocm-rel-6.3.3/ubuntu/22.04/ |
|
@IMbackK |
|
@IMbackK I actually gave into curiosity lol, messed around some today and did some fine-tuning, specifically around |
seperate pr, reverting this one if need be. |
|
Is there anything thing else I need to do for this PR to be merged? 👀 @JohannesGaessler Not to bother you (also 100% let me know if I shouldn't ping, I don't mean to be bothersome in the slightest), this is approved, could we get this merged in so I can open up my next optimization PR? |
|
Tested against Rocm 6.2, 7.2, 7.14 works fine give a nice speedup wanna have |
|
My current personal situation is this: I went on a break from llama.cpp maintenance for a few weeks because I felt I was too stressed out. Prior to the break I opened a PR for a general refactor of the MMQ configuration: #24127 . I'm currently working through my backlog but would like to have that PR merged before more changes are stacked on top of the MMQ kernel. |
|
@JohannesGaessler this just changes ggml_cuda_should_use_mmq so 2127 is not affected, any subsiquent gfx900 mmq tuneing should be done after that pr, but i dont see any reason not merge this. |
this is for vega64/56 and mi25 not mi50. These architectures are pretty unpopular in the ai space as they are pretty slow at this point all gfx900 has going for it is packed fp16 instructions. They lack the dprods and high global memory bandwith of mi50 that makes it popular. |
…org#24588) * HIP: keep MMQ for gfx900 MoE and Q8_0, use hipBLAS for dense K-quants Assisted-by: GitHub Copilot CLI * HIP: tighten conditional block to be explicitly for gfx900 * HIP: Further simplified gfx900 conditional block * removed unnecessary comment
…org#24588) * HIP: keep MMQ for gfx900 MoE and Q8_0, use hipBLAS for dense K-quants Assisted-by: GitHub Copilot CLI * HIP: tighten conditional block to be explicitly for gfx900 * HIP: Further simplified gfx900 conditional block * removed unnecessary comment
…org#24588) * HIP: keep MMQ for gfx900 MoE and Q8_0, use hipBLAS for dense K-quants Assisted-by: GitHub Copilot CLI * HIP: tighten conditional block to be explicitly for gfx900 * HIP: Further simplified gfx900 conditional block * removed unnecessary comment
…org#24588) * HIP: keep MMQ for gfx900 MoE and Q8_0, use hipBLAS for dense K-quants Assisted-by: GitHub Copilot CLI * HIP: tighten conditional block to be explicitly for gfx900 * HIP: Further simplified gfx900 conditional block * removed unnecessary comment
6 participants
Overview
gfx900(Vega 10) has no nativedp4a. The current code forces MMQ for all GCN. This routes densematmulstodequant+hipBLASon gfx900 while keeping MMQ for MoE, wherehipBLASis much slower.Overall Performance Gains:
Qwen3.5 4B: +36.1%Qwen3.6 27B: +18.9%Gemma4 12B: +65.1%Overall average: ~40%Additional information
Before (Qwen3.5 4B):
After (Qwen3.5 4B):
Before (Qwen3.6 27B):
After (Qwen 3.6 27B):
Before (Gemma4 12B):
After (Gemma-4 12B)
Requirements
I have read and agree with the contributing guidelines
AI usage disclosure:
dp4a. This led me to adding this -DGGML_CUDA_FORCE_CUBLAS=ONto my build and running tests to see how speeds compared, which is then when I noticed dense models in particular were consistently speeding up, but not MoE. Used that as a springboard for adding this conditional block, then did manual work to ensure it wasn't being too assuming (like how it assumed Q8_0 would regress, I doubted that and validated by hand).