perf(hip): enable -funsafe-math-optimizations for the ROCm backend#24668
Conversation
|
Hi @RapidMark, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
CUDA is compiled with fast math and AMD/HIP is not — this flag lets AMD use fast math too. We can't use -ffast-math: it implies -ffinite-math-only, which won't compile (ggml uses INFINITY for masking) and produces NaNs. -funsafe-math-optimizations gives the speedup without the NaN problems.
4fa2004 to
496aa8e
Compare
|
It's a one line change in the ggml/src/ggml-hip/CMakeLists.txt specifically fot HIP/ROCm... not AI written... just wrote the notes... thought it sounds better than me writting it... I re-wrote the notes and pushed... |
|
@RapidMark thanks for the solution. I thought I have same problem I am getting error when I don't gave --flash-attn off flag. I made the change in your PR but it doesn't solve in my case. |
|
@omerguzelelectronicguy This PR is only a build-time math flag (-funsafe-math-optimizations), it doesn't touch flash attention, so it won't change that crash either way. The assert you're hitting is in the FA launchers occupancy calc coming back as 0 (from what I can see). This looks like a launch-config bug on gfx1031, probably worth its own issue with that log attached. For now --flash-attn off is the right workaround. I do have a 6700XT, so I can try to see if I can find the issue you're having and will reply to you directly... but it's not in the scope of this PR. |
|
@omerguzelelectronicguy I dug into your crash... and the good news is you can get it running with --flash-attn off (you found this — it's the clean workaround). The bad news is... its an upstream bug, which I cannot look into right now because my 6700 XT is in a windows box and I don't have space in any linux box to test and run it (HIP is linux only, we use Vulkan on Windows). If you want FA, use the (linux) Vulkan build, instead of the HIP/ROCm as Vulkan has it's own FA that does work on RDNA2. |
|
@RapidMark Thank you for kindly helps and suggestions. |
|
Please never make this the default. -ffast-math enables a hodge-podge of different possible code manipulations that also lead to different hodge-podge behaviors / errors. If you're looking at faster special functions, those should be called specifically. See the intrinsics at https://rocm.docs.amd.com/projects/HIP/en/latest/reference/math_api.html . If you're looking for optimizations that ignore NaNs, please don't. They are no end of pain to debug. The sign of zero mostly matters in complex arithmetic so long as you can tolerate a projective rather than affine arithmetic in the reals. These are aspects that can be tuned and evaluated separately. And to me they should be handled separately. But then I'm on the IEEE 754 and P3109 committees, so I may be a tad biased. |
CUDA backend is already using it from what he says... precise math is often slower than what the hardware can do in the case of LLMs are fairly tolerant of imprecise math... if can be made to work there is no reason not to its not scientific compute I'd totally agree with you on that side of things. |
|
I agree with the above assessment. Either remove it from the CUDA path, or add it to the HIP path, it makes no sense to have one but not the other. |
|
As a fuddy-duddy, this compiler option lumps in a bunch of compiler- and architecture-dependent aspects. That doesn't matter much on the side that uses only one compiler but may matter on the sides that can use multiple compilers (at the moment). Breaking this into multiple options likely is a good choice moving forward. Signed zeros and infinities may not exist in some situations. Signalling NaNs may not exist in other situations (we can hope). And in some systems NaNs do not exist at all. Or Infs. It depends on the HW. Think of this as a form of quantization. Models that are trained along with a target's quantization methods potentially can perform better. Similarly, models trained with a target's available floating-point arithmetic potentially can perform better. I've seen some of those papers cross my desk. I have no data here and only experience debugging issues. I would suggest providing methods to enable/disable different floating-point optimizations without the -ffast-math option. Platforms and models then could supply their presets. I also have no time to make this happen. And the decisions are made by those doing the work. |
|
Oh, and background, I don't have any CUDA-supporting hardware. So I haven't paid attention to that side. |
|
Thanks @jasonriedy, but worth being precise about scope: this is -funsafe-math-optimizations, not -ffast-math, and the difference maps onto your concerns:
It also mirrors the CUDA backend's nvcc -use_fast_math (also inf-safe), so this is parity between the two paths, validated on RDNA4 with test-backend-ops 12508/12508 passing including EXPM1. Finally... CUDA has flags which make it inherently faster, but HIP/ROCm does not... this just ads parity... and if you want to debug... you turn off optimizations anyway... |
Overview
The HIP/ROCm backend is denied the fast math CUDA gets, so it runs without that speedup. We added -funsafe-math-optimizations to let AMD use faster math while staying away from NaNs.
Additional information
Validated on RDNA4 (gfx1201, ROCm 7.0.2): builds clean, test-backend-ops 12508/12508 (including EXPM1), ~+8–15% on FLASH_ATTN.
Requirements