KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
3.07k stars 837 forks source link

Wrong SPIR-V precision decorations for bitcount and findMSB GLSL functions #350

Closed ghost closed 8 years ago

ghost commented 8 years ago

glslang seems to wrongly place OpDecorate ... RelaxedPrecision for unary build-in GLSL functions, that have precision defined by operands.

Consider following GLSL code

highp int value = ...;
mediump int count;
count = bitCount(value);

For above snippet, glslang generates following SPIR-V:

OpDecorate %47 RelaxedPrecision
OpDecorate %49 RelaxedPrecision    <--- Wrong!
%49 = OpBitCount %34 %48
OpStore %47 %49

GLSL has different precision semantics than SPIR-V, making the above code not equivalent and yielding different results:

GLSL:

The precision used to internally evaluate an
operation, and the precision qualification subsequently associated with any resulting intermediate values,
must be at least as high as the highest precision qualification of the operands consumed by the operation.

SPIR-V:

The RelaxedPrecision Decoration can be applied to:
...
* The Result <id> of an instruction that operates on numerical types, meaning the instruction is to operate at relaxed precision.

So GLSL precision of operation comes from operand, while in SPIR-V precision comes from Result id decoration. In example above this is highp in GLSL, and Relaxed in SPIR-V.

While RelaxedPrecision decodation of result variable (%47) is OK, glslang should have not decorated the Resuld id (%48).

This especially hurts bit operations like count() and findMSB(), where precision of operation and precision of result has different effect.

This bug comes from vulkanCTS, tests dEQP-VK.glsl.builtin.function.integer.bitcount.int_highp_compute, dEQP-VK.glsl.builtin.function.integer.findMSB.int_highp_compute and similar others.

johnkslang commented 8 years ago

From the ESSL specification, the built-in prototype is:

lowp genIType bitCount(genIType value);

That says the precision of the return value of the function is lowp, and that's what glslang is decorating as RelaxedPrecision. So, I'm not quite following the issue here.

bitCount() certainly has to count the number of bits in the operand, which is a 32-bit quantity, but the count can only go up to 32, which fits in RelaxedPrecision without loss.

johnkslang commented 8 years ago

Okay, the problem is SPIR-V RelaxedPrecision says:

For integer operations...

  • all inputs are truncated ...

And the ESSL specification says about lowp:

The precision of built-in functions ...

  • Some functions have predefined precisions. The precision is specified by the function signature

I think the most likely specification point to argue is in ESSL.

ESSL conflates precision of built-in operation with the precision qualifier used on the prototype. This is okay, as it's probably the best way to communicate that. However, since it's difficult to argue that an operation takes place at lower precision than its parameters innately require, things like bitCount() shouldn't have a precision qualifier on their prototype.

Since it is nonsensical to say a bitCount of a 32-bit int can operate on a reduced precision version of that 32-bit int, it makes sense to have glslang not treat the operation as a RelaxedPrecision operation.

The complete set of these that need to change is: