Repository navigation
Lack of rounding of lower precision constants #4241
Description
Constant values have a nonobvious behavior where they are not rounded until the SPIR-V OpConstant is emitted. The ConstantUnion stores them in double precision regardless of declared type. This means bits that should have been rounded away can contribute to calculations involving those constants. For example:
#version 450
#extension GL_EXT_shader_explicit_arithmetic_types_float16 : require
const float16_t c0 = 1.000000001hf; // would be rounded to 1.0 in fp16 or fp32
const float c1 = 1000000*(c0 - 1.0f);
void main() {
}
generates:
%half_0x1p_0 = OpConstant %half 0x1p+0
%float_0_00100000005 = OpConstant %float 0.00100000005
(see https://godbolt.org/z/x9vEEzeq1).
A developer would reasonably expect c0 to be rounded to exactly 1, and then for c1 to compute a value of zero.
I don't know if we should "fix" this (seems a bit risky?), or maybe add a warning somewhere. Or maybe try to document the behavior?
Activity
spirv-opt: Fix assertion with fp8
spirv-opt: Fix assertion with fp8
arcady-lunarg commentedon May 8, 2026
Yeah, this does seem to be a deficiency of the ConstantUnion. It has different union members for all the different int sizes but all floats are stored as doubles. I suppose the solution is to add union members for the various supported float types like float and float16. Would it be reasonable to convert the lower precision float types to at least 32-bit floats, do the operations using the host platform's available floating point implementation, and then round them off to the reduced precision when storing the result? Since C++ doesn't natively support float16 or float8 types, the rounding step would presumably have to do some amount of bit twiddling.
jeffbolznv commentedon May 8, 2026
What you describe makes sense. I'm not sure if you actually need to store them as the different types, or maybe just convert to the real precision and back to double when updating a constant value.
spirv-opt: Fix assertion with fp8
Wint3rNight commentedon Aug 6, 2026
I'd like to take this one if nobody's working on it.
Planning to go with the simpler option, convert to the declared precision and back when a constant value is updated, rather than adding separate union members for float and float16. That leaves TConstUnion's layout alone and as far as I can tell gets the same result.
Two things I'd like to settle before starting:
Is fixing the behaviour what you actually want here, or would you rather warn/document? The original report raised that as an open question and I'd rather not build the wrong thing.
It'll move some baselines for the fp16 and mediump tests. I'll go through those individually rather than blanket-regenerating, but flagging the churn up front.
I just opened #4373 for the BitsTo folding under the same TODO. I left the four float16 variants out of it specifically because they need half rounding, which is what this issue is about.
jeffbolznv commentedon Aug 6, 2026
IMO the current behavior is wrong and I would like to see it fixed. I agree it should be fine to convert to the intended precision but still store as double.
spirv-opt: Fix assertion with fp8 (#6677)