[llvm] [NVPTX] Set default value of nvptx-allow-ftz-atomics to true (PR #206154)

[Artem Belevich via llvm-commits]

Artem-B wrote:

> To check my understanding: there's a bug in the user code sample. Technically speaking, programmers should insert `__syncwarp` / `__syncthreads` between the add and the load to ensure the load reads the updated value, but this is a common code pattern for programmers thinking in the SIMT model. If they don't write explicit branches, they expect converged execution, and this breaks when a branch-less operation expands to a loop or conditional.
> 
> On the other hand, we have the issue of floating point accuracy and correctness of not flushing to zero when the programmer didn't explicitly ask for it, so we're weighing FP precision against breaking technically incorrect user code.

UB code is how we found the issue, but since then, I've got a few more users that were affected by performance regression on a valid code. We do have the escape hatch option, so it's not a showstopper, but I'm willing to bet that there are more affected users why didn't notice the problem yet.

The more important issue is that we currently have no way to let users specify exactly what they want. I.e. if a function uses generic pointers (but the user knows the address space) LLVM will always use CAS. Or if the user mixes global and shared atomics in the same functions, only one of them will be fast, and the other will use CAS, regardless of FTZ setting.
 
> I'm not an FP expert, but there's a bell going off in my head from all the graphics programmers I've met over the years who have insisted to me that denormals are a performance trap, so at least for real-time applications, you always want FTZ.

I'd say that they are more keen on "you always want fast", and it's not the same as "you always want FTZ" (not on NVIDIA GPUs, at least), and we currently do not have sufficiently flexible controls to let them achieve that goal.

> Casual research shows that CUDA, Triton, and XLA all default to FTZ, so it seems like the blast radius here should be pretty small, and I'm surprised we're considering this workaround. It sounds like noftz is a build configuration you maintain for testing purposes to measure the impact of FTZ on the output.

Whichever FTZ mode they pick, some of the atomics will end up as CAS. Most of the time the impact will be negligible, but there are cases where regression is noticeable-to-catastrophic.

My proposal is to roll back default to the old behavior to avoid unnecessarily disrupting the users with *valid* code while we have no good solution for them, implement AMD-style per-instruction FTZ control, and *then* switch back to "correct by default". If/when somebody needs atomic to produce just-so variant, we can point to the docs and tell them how they can express that in IR or CUDA. 

In short, I agree with your points and and that LLVM should generate correct code by default, I'm trying to do it in the way that's less painful for the users. Considering that we lived with the old behavior for a long time with nobody complaining about it, and that the current correct code causes noticeable perf regressions, I think it makes sense to avoid disrupting users until we can give them a usable way to achieve what they need.

An alternative is to tell every affected user to use the escape hatch option, but then we'll have a problem of removing that option later on, as we'll grow an unknown number of users that depend on it. 

https://github.com/llvm/llvm-project/pull/206154