shader-slang / slang

Making it easier to work with shaders
MIT License
2.07k stars 178 forks source link

Operator % semantics, including floating-point #1059

Closed tangent-vector closed 8 months ago

tangent-vector commented 5 years ago

The % operator in C-family languages has a long and complicated history. The big decision point is whether it should map to a "modulus" operation or a "remainder" operation, with the difference having to do with what happens on negative operands.

Both C99 and C++11 appear to specify that % should be a remainder such that (a/b)*b + (a%b) == a (where that expression doesn't overflow/underflow or divide-by-zero), and that the a/b operation always rounds toward zero (rather than allowing round-toward-negative-infinity).

HLSL appears to follow the modern C/C++ behavior. DXC always maps % to the *Rem operations (not the *Mod operations), in both DXIL and SPIR-V.

As an extended feature on top of C/C++, HLSL supports % on floating-point inputs, where it performs an LLVM/SPIR-V FRem operation.

For our HLSL back-end this is all easy enough: we just need to map % through to the output and we are done. On the C/C++ back-end we can either rely on C99/C++11 semantics (which realistically are what most compilers/processors always did), or switch to std::div() for integers, and use the remainder() function from math.h for the floating-point case.

The GLSL target is the one where this gets interesting, because GLSL has no standard function that maps to a remainder operation, and thus could satisfy the semantics of HLSL input. For unsigned types this is a non-issue of course. For floating-point types there is a mod() function, but no equivalent for remainder.

tangent-vector commented 5 years ago

PR #1060 should address the missing floating-point % operator, and support it on D3D and CPU with the right semantics.

The issue for GLSL/SPIR-V is not addressed in that PR, and we need a plan for how to handle it. The three main options I can see are:

  1. Customize our version of glslang to have appropriate operations (or even change the default semantics of % on integers...). This requires us to maintain a set of patches to glslang, or to push those changes upstream (which might require us to write a GLSL extension spec for the new functions)

  2. Generate SPIR-V via dxc instead of glslang, as a way of guaranteeing we get the HLSL semantics for ops like % "for free." This would be a big churn, but could make some users happier than they are with the current GLSL cross-compilation path.

  3. Finally get around to cutting glslang out of our compiler and generating SPIR-V directly.

I obviously like (3) best, but it might not be the most practical if we need to support a proper remainder operation on Vulkan in a short timeframe.

csyonghe commented 10 months ago

Our SPIRV direct backend current emits SpvOpFMod for kIROp_FRem, and we should change that, although this means that it is going to break some existing test, but we should modify the expected results of those tests instead.

jkwak-work commented 8 months ago

The problem, if I understood correctly, is that when the modulo operator in HLSL is compiled to SPIRV, we are incorrectly using SpvOpFMod for floating-type. It should use SpvOpFRem. But there might be another problem regarding the sign. Is there a document that describes how SpvOpFRem is implemented?

chaGPT explained the different as following:

In HLSL, the result of the modulo operation preserves the sign of the dividend (the left operand). This means that if the dividend is negative, the result will also be negative. In GLSL, the sign of the result is determined by the sign of the divisor (the right operand). If the divisor is positive, the result will be positive; if the divisor is negative, the result will be negative.

// HLSL float resultHLSL = -5.0 % 3.0; // Result is -2.0

// GLSL float resultGLSL = mod(-5.0, 3.0); // Result is 1.0

csyonghe commented 8 months ago

You can refer to the SPIRV spec for a defintiion: https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html

Here is how SpvOpFRem is defined:

The floating-point remainder whose sign matches the sign of Operand 1.

Result Type must be a scalar or vector of floating-point type.

The types of Operand 1 and Operand 2 both must be the same as Result Type.

Results are computed per component. The resulting value is undefined if Operand 2 is 0. Otherwise, the result is the remainder r of Operand 1 divided by Operand 2 where if r ≠ 0, the sign of r is the same as the sign of Operand 1.

We want to map kIROp_FRem to SpvOpFRem. This means that the execution result will diverge from shaders compiled by glslang, but it will allow the same Slang shader code to behave the same across different APIs.

jkwak-work commented 8 months ago

Thank you for the link. That document is exactly what I was looking for. It clearly said about how the sign is handled.

jkwak-work commented 8 months ago

I made a change as discussed here and I am having a trouble with the testing. Note that I also changed tests/compute/frem.2.expected.txt and now it is exactly same as tests/compute/frem.expected.txt

When I run the test with a following command, all tests are passing. bin\windows-x64\release\slang-test.exe -emit-spirv-directly tests/compute/frem

But when I run it without "-emit-spirv-directly", the test is failing. bin\windows-x64\release\slang-test.exe tests/compute/frem

This might be expected, because my fix applies only to the direct route from HLSL to SPIRV. When it ran without "-emit-spirv-directly", HLSL is translated to GLSL incorrectly and the testing fails. I am not sure how to address this problem.

csyonghe commented 8 months ago

You need to add -emit-spirv-directly to the //TEST: line inside the test, so that it is always specified when targeting spirv.

jkwak-work commented 8 months ago

Aha. That's a good idea!

tangent-vector commented 8 months ago

Is there a document that describes how SpvOpFRem is implemented?

The SPIR-V spec documents the behavior of both OpFRem and OpFMod.