[SPIR-V] FMin/FMax/FClamp NaN handling contradicts HLSL specification

[Triang3l]

Currently the SPIR-V emitter generates FMin, FMax and FClamp instructions for min, max, clamp and saturate. The specification of the GLSL.std.450 instruction set states that "which operand is the result is undefined if one of the operands is a NaN". This is in contrast with the DXIL, DXBC and HLSL specification of those instructions, which, if one of the operands is NaN, give the other operand as the result (and clamp/saturate gives the lower bound, as if max was performed first).

While HLSL is normally optimized as if there were no special floating-point values at all, with the precise qualifier or the "force IEEE strictness" compiler option, the compiler must honor the defined behavior for specials. Clamping operations may also be used with the implication of sanitizing the input, making sure the resulting value is strictly in the bounds and not NaN.

The correct implementation, which maps 1:1 to the DXIL/DXBC behavior, is NMax/NMin/NClamp from the GLSL.std.450 instruction set. Are those safe to use in DXC (such as, can we assume that NClamp to 0 and 1, on existing drivers or in the near future, will surely map to the hardware saturation modifier in all the same cases as when FClamp to 0 and 1 is used; and whether they're compatible with wave reduction)?

[sudonatalie]

From what I see in the HLSL spec, min and max would be correctly represented by the GLSL.std.450 instructions NMin and NMax, respectively, so I'll submit a fix for those. However, the clamp (and by extension, saturate) documentation states that "for values of NaN, the results are undefined." As far as I can see, this correctly matches the semantics of FClamp, so it will remain as-is.

[sudonatalie]

Thanks for reporting this @Triang3l !

[Triang3l]

Ooooh, didn't notice that property of clamp, thanks for telling me about it so I'll keep that in mind and recheck all the usages of it in my existing code @sudonatalie! :D And thank you for the fix!!!

[Triang3l]

However, the clamp (and by extension, saturate) documentation states that "for values of NaN, the results are undefined."

@sudonatalie While this is not specified in the HLSL documentation directly, however, the _sat modifier in Shader Model 4 assembly/bytecode is defined to return 0.0 for NaN according to the rules of the max DXBC instruction. Would it be reasonable to extend this logic to HLSL, or would that be just an excess optimization and maintenance constraint considering that the HLSL specification doesn't seem to directly link the saturate intrinsic to the _sat modifier (but at the same time, it doesn't connect saturate to clamp either — but neither does it to max and min, nor does it provide any info about NaN handling)?

[sudonatalie]

I'd suggest opening a new issue for questions about changing/clarifying the HLSL reference and someone else with more HLSL/DXBC context may be able to respond with more detail.