[llvm] Revert "LangRef: Clarify llvm.minnum and llvm.maxnum about sNaN and signed zero (#112852)" (PR #138451)

[Nikita Popov via llvm-commits]

nikic wrote:

> > As seen in recent PRs and issues, the current state where LangRef and the middle-end do not agree is causing unnecessary confusion, which is why I think it is important to revert it now, before the situation becomes worse.
> 
> I don't see a way this will ever not be confusing. The codegen never matched on most targets. If we at least have it documented how it should work, we can converge towards the end state.

It will always be somewhat confusing because the operations are confusing, but we don't have to add our own LLVM confusion on top, where LangRef and the middle-end don't match...

> > (An alternative I would put up for consideration is to remove maxnum entirely, auto-upgrading the intrinsic to maximumnum nsz to match the previous semantics. Not sure anyone actually wants llvm.maxnum with the new semantics?)
> 
> Yes, I think it is what most people want but not because it makes any sense. This is the instruction that's been implemented in billions of devices. The new fminimumnum/fmaximumnum require wasting cycles for quieting in codegen, and it's still useful to have raw access to the underlying operation. We have very limited capacity to optimize out unnecessary quieting given the fuzzy snan semantics. We can't just look at the floating point producers and assume they will quiet. Since nobody actually cares about signaling nans, and do care about the best possible code, most use cases are better served by minnum/maxnum in practice.
> 
> We have an AMDGPU only combine that tries to prune these out, but it doesn't work all that well due to the usual DAG reasons (and common usage of unanalyzable sources like function arguments and loads). We could do a little better if we strengthened the DAG/gMIR semantics to not permit dropping canonicalizes, but there's a cost to diverging from the IR rules

I do see your point. Another alternative, which is more consistent with LLVM's overall treatment of sNaN and also your "nobody actually cares about sNaN" perspective, is to specify that minnum/maxnum with an sNaN argument may non-deterministically convert it to a qNaN. This means we are allowed to lower minnum/maxnum to operations that either follow or don't follow the IEEE-754-2008 sNaN behavior. This would effectively specify the current backend behavior.

I haven't carefully thought through what the implication of that for the middle-end would be -- I think we'd still lose associativity, but not some of the other things.

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