Round low precision float constants when storing them by Wint3rNight · Pull Request #4374 · KhronosGroup/glslang

Wint3rNight · 2026-08-06T15:04:15Z

TConstUnion stores every float type in a double, so a constant keeps bits its declared type
can't hold, and those bits then take part in constant folding:

const float16_t c0 = 1.000000001hf;        // should be exactly 1.0 in fp16
const float     c1 = 1000000*(c0 - 1.0f);  // gives 0.001, should be 0

As suggested in the issue, this rounds to the declared precision and stores back as a double
rather than adding a union member per float type. The rounding lives in
RoundToDeclaredPrecision, called from TConstUnion::setDConst, so a constant is rounded once
when it's stored and every later fold sees the value the back end will emit.

It started as fp16 only and grew to cover every float type, because each one left out turned out
to be the same bug in a different format:

Area	What it does
Narrow formats	float16, bfloat16, e5m2, e4m3, e2m1, e3m2, e2m3, ue8m0, mxint8, each rounded through the same conversion its `Builder::make*Constant` performs
fp32	`EbtFloat` rounds too, so an intermediate that overflows or loses bits in fp32 does so while folding: `(1e20*1e20)/1e20` is `inf` now, not a finite `1e20`
Formats with no NaN or Inf encoding	ue8m0 and mxint8 pin those to whatever the emitter produces, instead of reading the largest exponent as Inf/NaN
Signalling NaN	`intBitsToFloat`/`uintBitsToFloat` keep the exact 32-bit pattern in a supplementary raw-bits field, since float→double widening quiets an sNaN
Comparison and selection	`min`/`max`/`clamp`/`mix`-with-bool/`faceforward` return an operand rather than computing one, so they copy the whole `TConstUnion`; `<=`/`>=` are spelled as disjunctions so they stay false for unordered operands
Signed zero	mxint8 is fixed point and has no signed zero, its emitter's cast to int drops the sign, so the fold canonicalizes `-0.0` to `+0.0` to match
Explicit types	`setDConst`'s `baseType = EbtDouble` default is gone and the invariant is asserted; four HLSL call sites relied on it, two of which built a double-typed union for an integer comparison

spv.double.comp is the clearest case. It declares pi four times with LF on two of them,
specifically to tell suffixed and unsuffixed apart — and all four were folding to the same value
and being deduplicated into a single SPIR-V constant, so the test had never been able to check
what it was written for. That's the id bound going 60 → 61. lf-suffixed literals keep full
double precision, which is the control that shows the rounding isn't over-applied.

Tests: Test/constFoldFloat16.frag and Test/spv.constFoldLowPrecision.comp are new, plus
additions to Test/constFold.frag. The low-precision file pins both directions, mxint8 has to
canonicalize -0.0 and e5m2 has to keep it so, an over-broad "canonicalize every zero" change
fails it.

Both ctest jobs are green with ENABLE_OPT=ON: glslang-gtests 2018/2018 and
glslang-testsuite clean. Each fix was reverted on its own to confirm a specific test goes red.

jeffbolznv · 2026-08-06T15:26:03Z


+namespace {
+
+// IEEE 754 binary16 <-> binary32, rounding toward zero on the way down.


Would be nice to use hex_float.h if possible.

Ah nice, that works — no build changes needed and it gives the same values. Switched.

jeffbolznv · 2026-08-06T15:28:17Z


    TConstUnionArray unionArray(1);
-    unionArray[0].setDConst(d);
+    unionArray[0].setDConst(RoundToDeclaredPrecision(d, baseType));


I think we need to do this for all setDConst, so it would probably make sense to do it based on the TConstUnion's type (and let it be floating point types other than double)

Yeah that's better, catches folded results too. Looks like setDConst needs to stop hardcoding EbtDouble and the switches in ConstantUnion.h only handle double right now. Having a go at it.

Pushed. One thing, I left EbtFloat mapping to EbtDouble instead of giving it its own tag. A double holds a float exactly so there's nothing to round, and when I did tag it separately a bunch of stuff broke that was using type == EbtDouble to mean "is a float" sign() went down the int path, struct compares came out false. So only the narrower types get their own tag now. Can switch it if you'd rather.

using type == EbtDouble to mean "is a float"

IMO these sort of things are worth fixing. It would be nice to see what the complete changes needed would look like before preemptively making these kind of concessions.

Fair enough, did the full version.

The one I'd missed was promoteConstantUnion, it's how an int literal in an initializer becomes a float, and it wasn't passing a type, so {3, ...} came out EbtDouble while 3.0 was EbtFloat. With that fixed the EbtFloat special case isn't needed, so it's gone. Five places total were using type == EbtDouble to mean "is a float".

jeffbolznv · 2026-08-07T17:09:39Z

I have not had a chance to do a detailed review, but I did point codex at the change and asked it to review. Please check on these:

[P1] Low-width bit-cast built-ins are incorrectly folded as 32-bit operations. glslang/MachineIndependent/Constant.cpp:879 handles EOpFloatBitsToInt and related enums as float/int, but those enums are also used by bfloat16 and 8-bit float built-ins. For example, floate5m2BitsToUintEXT(floate5m2_t(1.0))
folds to 0; it should be 60 (0x3c). Either implement each width or decline to fold non-32-bit variants.
[P1] intBitsToFloat results are tagged as EbtDouble. At glslang/MachineIndependent/Constant.cpp:885, the calls omit returnType.getBasicType(). Consequently:

const float f = intBitsToFloat(1065353216);
const bool b = f == 1.0;

folds b to false. The unsigned case has the same problem.
[P1] clamp and both mix paths missed the new floating type propagation. glslang/MachineIndependent/Constant.cpp:1124 and glslang/MachineIndependent/Constant.cpp:1184 still use the default EbtDouble. Chaining any of these results into an equality test currently folds incorrectly; I confirmed three expected-true comparisons all became false.
[P2] Declared-precision rounding remains unimplemented for the other low-precision formats. glslang/MachineIndependent/Intermediate.cpp:2651 rounds only EbtFloat16, while bfloat16 and all supported 8-bit formats return unchanged. For example, promoting 1.1fe2m1 and subtracting 1.0 folds to 0.1, even though the emitted E2M1 value is 1.0. This is the same inconsistency the change is intended to eliminate.

castTo checked supportsInfinity() for the destination format but not for the source, so casting from one of the MX formats treated its largest exponent as Inf or NaN. Converting e2m1 6.0 to float produced a NaN and e2m1 4.0 produced an infinity, even though both are ordinary values in that format.

…ther low-precision formats Follow-up to review on KhronosGroup#4374. - floatBitsTo*/*BitsToFloat are shared with the bfloat16 and 8-bit float built-ins, which are not 32 bits wide. Folding those as float produced the wrong bits, so decline to fold them. - intBitsToFloat, uintBitsToFloat, clamp and mix stored their result without the declared type, so it kept the default EbtDouble and compared unequal to a literal of the same value. - RoundToDeclaredPrecision now covers bfloat16, e5m2, e4m3, e2m1, e3m2, e2m3, ue8m0 and mxint8, each matching the corresponding Builder::make*Constant.

Wint3rNight · 2026-08-07T19:44:50Z

Fixed all four.

Low-width bit-casts — declined to fold them, those enums are shared with the bfloat16/8-bit built-ins. Came in with #4373 which is already merged, so happy to split it out.

intBitsToFloat and the clamp/mix paths — same root cause, any setDConst without the type compares unequal now. 7 sites, incl. int64BitsToDouble/uint64BitsToDouble.

The other formats — did all of them. Ran into a separate bug doing it: castTo checks supportsInfinity() on the destination but not the source, so converting e2m1 6.0 to float gives a NaN and 4.0 gives an inf. Split that into its own commit since it's not really this PR's problem. No golden changes from it.

jeffbolznv · 2026-08-08T00:30:11Z

A few more comments from codex:

SPIRV/hex_float.h:845. FloatE4M3 has no infinity but does have NaN encodings, so gating is_nan on supportsInfinity() is incorrect. A focused shader folded float(floate4m3_t(NaN)) to 384.0. Use the format’s isNan() implementation or a separate NaN trait. The related test comment claiming the MX formats have no NaN is also inaccurate.
glslang/MachineIndependent/Constant.cpp:894. The intermediate float-to-double conversion quiets signaling NaNs. This expression:

floatBitsToUint(uintBitsToFloat(0x7fa12345u))

folds to 0x7fe12345, rather than preserving 0x7fa12345. NaN encodings should remain unfolded unless TConstUnion can retain their raw 32-bit representation.

glslang/MachineIndependent/Intermediate.cpp:2710. Masking with 0x7F800000 maps UE8M0 encoding 0x00 to floating zero and 0xFF to infinity. UE8M0 has no zero: 0x00 represents 2^-127, while 0xFF is NaN. My focused test folded those to 0.0 and infinity respectively. The conversion should explicitly decode the resulting UE8M0 exponent encoding.

Wint3rNight · 2026-08-08T02:37:11Z

All fixed, thanks.

castTo now uses the format's own isNan()/isInfinity(), and a NaN converted into e4m3 encodes as all ones. Fixed the test comment too.
Signaling NaN patterns no longer fold, since the double storage quiets them. Quiet NaNs still fold bit-exactly, so const initializers like floatBitsToUint(0.0/0.0) keep working — can make it a blanket decline instead if you'd prefer.
ue8m0 is now decoded from its actual encoding: 0x00 = 2^-127, 0xFF = NaN.
Checking the other formats for the same kind of thing turned up one more: mxint8 folded a NaN to NaN, but makeFloatMXINT8Constant casts NaN to int, UB that lands on 0x00. Both sides now pin NaN to zero so the fold and the emitted constant agree. Also fixed castTo truncating NaN payloads when widening, and added goldens for the encode directions and the ue8m0 negative/inf edges.

- castTo classified NaN and Inf with the IEEE bit pattern, gated on supportsInfinity(). e4m3 has a NaN but no infinity, so its NaN was read as an ordinary value, and a NaN converted into e4m3 was encoded as 0x7C, which is 384.0. Classify with the format's own isNan() and isInfinity(), and encode NaN into a format without an infinity as all ones, its only NaN encoding if it has one. - Bit casting a NaN folded through the double the constant is stored in, and the float-to-double conversion quiets a signaling NaN: floatBitsToUint(uintBitsToFloat(0x7fa12345u)) folded to 0x7fe12345. Decline to fold signaling NaN patterns so those bits come from the real instruction. A quiet NaN survives the round trip exactly, so it keeps folding and a NaN in a const initializer keeps compiling. - ue8m0 was folded by masking the float's exponent bits, which reads encoding 0x00 as 0.0 and 0xFF as infinity. The format has no zero and no infinity: 0x00 is 2^-127 and 0xFF is NaN. Decode the encoding the back end emits explicitly instead. - mxint8 has no NaN encoding at all, and makeFloatMXINT8Constant cast a NaN to int, which is undefined and lands on encoding 0x00. Pin a NaN to zero deterministically on both sides so the folded value and the emitted constant agree. - castTo shifted a widening NaN payload in the narrow source type, so the payload truncated to nothing and the fallback emitted payload 1, a signaling NaN. Shift in a type wide enough for either format.

jeffbolznv · 2026-08-09T00:01:59Z

codex is basically happy with it now. I still haven't had time to manually review, sorry.

The three previous correctness issues are fixed. I found one remaining, lower-priority completeness gap:

Constant.cpp:909. Consequently:

const float snan = uintBitsToFloat(0x7fa12345u);
still fails with “global const initializers must be constant,” while the same expression with a quiet NaN or finite value succeeds. This is intentional in the patch, but it
makes constant-expression support value-dependent and doesn’t fully deliver “fold bit-cast conversions as constant expressions.” Addressing it properly likely requires
retaining raw float bits in TConstUnion.

GLSL permits—but does not strictly require—additional pure built-ins such as this to be treated as constant expressions, so I would not call this a core-spec violation. GLSL
4.60 §4.3.3 (https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.4.60.html#constant-expressions)

Wint3rNight · 2026-08-09T02:01:21Z

Thanks Jeff, glad the correctness issues are resolved.

Agreed on the sNaN gap. The decline is there because the double storage would quiet the signaling bit, so the folded pattern wouldn't match what the instruction produces. Fixing it properly probably means retaining raw float bits in TConstUnion rather than routing through double.

Would you prefer that addressed in this PR, or is it cleaner as a separate change? Happy to go either way.

jeffbolznv · 2026-08-09T02:11:34Z

I think people would generally consider this value-dependent behavior to be unexpected and a bug. So if it's a regression from this change, then I think it should be fixed as part of this change.

intBitsToFloat/uintBitsToFloat now store the exact 32-bit pattern via a supplementary rawFloatBits field in TConstUnion, so signaling NaN payloads survive without passing through the float-to-double widening that quiets them. Removes the isFloatSignalingNanPattern guard.

Wint3rNight · 2026-08-09T07:08:27Z

Fixed, added a rawFloatBits field to TConstUnion so the exact 32-bit pattern from *BitsToFloat is preserved without going through double. isFloatSignalingNanPattern is gone.

jeffbolznv · 2026-08-09T12:52:00Z

codex says:

The signaling-NaN constant-expression issue is fixed, but the raw-bit implementation introduces one correctness regression:

ConstantUnion.h:265. When both operands have raw bits, operator== compares their encodings. That breaks IEEE/GLSL equality:

uintBitsToFloat(0x00000000u) == uintBitsToFloat(0x80000000u) // must be true
uintBitsToFloat(0x7fc12345u) == uintBitsToFloat(0x7fc12345u) // must be false
The refreshed compiler folds these to false and true, respectively; != is likewise inverted. The raw-bit special case should be removed from numeric equality—comparing dConst
already gives the correct zero and NaN behavior. Raw bits should only be used for bitcasts and SPIR-V emission.

Numeric comparison read the raw bits whenever both operands carried them, which inverted two IEEE results: +0.0 == -0.0 folded to false, and a NaN compared equal to itself. Comparison goes back through dConst, which gets both right, and the raw bits stay out of it. Negate and abs had the opposite problem. They folded through the double, which quiets a signaling NaN, but IEEE 754 makes them sign-bit operations that leave the payload alone. They now flip or clear the sign in the raw bits when the constant carries them.

Wint3rNight · 2026-08-11T08:44:32Z

Yep, that's a real bug, fixed. operator== doesn't touch the raw bits anymore, it compares dConst like it used to, so +0.0 == -0.0 is true again and a NaN isn't equal to itself. Raw bits are only read by the bitcast folds and the SPIR-V emitter now.

While I was in there I noticed abs() and unary minus had the opposite problem, going through the double meant they were quieting sNaN payloads, and IEEE treats those two as sign-bit ops. Fixed them to flip/clear the sign bit directly instead.

Tests for both in constFold.frag, and the negated sNaN in spv.constFoldLowPrecision.comp so the emitted OpConstant is covered too.

jeffbolznv · 2026-08-11T13:53:36Z

codex found two more issues with NaN handling:

<= and >= fold incorrectly for NaNs. They are implemented as !(a > b) and !(a < b) in Constant.cpp:356 and again at Constant.cpp:1210. For unordered operands, both become true; IEEE/GLSL requires false. A focused shader confirmed both s <= s and s >= s fold to true when s is a NaN from uintBitsToFloat().

These should use (a < b) || (a == b) and (a > b) || (a == b), including the vector paths.
Boolean mix() loses raw float bits. At Constant.cpp:1225, the selected operand is read through getDConst() and written with setDConst(). That is arithmetic-free selection, so it should copy the selected TConstUnion directly. Currently:

floatBitsToUint(mix(uintBitsToFloat(0x7fa12345u), 1.0, false))

folds to 0x7fe12345, quieting the signaling NaN, instead of preserving 0x7fa12345.

jeffbolznv · 2026-08-12T04:16:30Z

I think the main remaining things are to round to fp32 when baseType == EbtFloat is used, and to remove the default baseType = EbtDouble and make sure we always pass in the correct type.

Wint3rNight · 2026-08-12T09:08:30Z

Both done.

Removing the default turned up a real one: HLSL clip() was calling setDConst(0) on the integer path, building a double-typed union for an integer compare. Mirrored the scalar path below it.

spv.double.comp declares pi four times with LF on two of them, we were folding all four to the same value and deduplicating them, so it had never actually tested anything. That's the 60 → 61 id bound.

16 baselines regenerated, all fp32 rounding. Failure set unchanged.

RoundToDeclaredPrecision handled every narrow float format but returned EbtFloat unrounded, so a fold that overflowed or lost bits in fp32 kept double precision until emission: (1e20 * 1e20) / 1e20 folded to a finite 1e20 where the target produces inf. Remove the EbtDouble default from setDConst so callers must pass the type the value was declared as, and assert it. Four HLSL call sites relied on the default; two built a double-typed union for an integer comparison, which the scalar path alongside them already got right.

jeffbolznv · 2026-08-12T13:37:07Z

codex found two more minor issues:

EbtFloatMXINT8 still preserves -0.0 during folding. A temporary shader confirmed that converting -0.0 through MXINT8 and back produces folded bits 0x80000000, although the emitted MXINT8 encoding is 0. Canonicalize a rounded zero to +0.0 in glslang/MachineIndependent/Intermediate.cpp:2724.
HLSL clip(uintN) constructs its zero with setIConst(0), leaving an EbtInt union inside an EbtUint constant node. The regenerated baseline visibly says const int beside the uint operand. glslang/HLSL/hlslParseHelper.cpp:5025 should use setUConst(0) for EbtUint.

MXINT8 is fixed point with no signed zero -- makeFloatMXINT8Constant ends in a cast to int, which drops the sign, so -0.0 is emitted as encoding 0. The fold kept the sign bit, leaving folding and emission disagreeing. The other formats are unaffected: the sign-magnitude ones represent -0.0 legitimately and ue8m0 has no zero at all. HLSL clip()'s vector path builds its zero with arg0's TType, so the union has to match arg0's basic type; it was always EbtInt, which put an int zero inside a uint constant. Switch over the integer widths. The scalar path is fine as it stands -- it hands addBinaryMath a plain int and promotion resolves it.

Wint3rNight · 2026-08-12T18:37:44Z

Both fixed. Checked the other ten formats for the same −0.0 leak, the sign-magnitude ones (bfloat16, e5m2, e4m3, e2m1) legitimately keep it and ue8m0 has no zero at all, so mxint8 was the only one. Test pins both directions so a broader "canonicalize every zero" would fail it.

clip() now switches over every integer width rather than just uint. The scalar path turned out to be fine, it hands addBinaryMath a plain int and the usual promotion fixes it up; only the vector path forces the TType with no conversion step.

jeffbolznv · 2026-08-12T21:07:47Z

Thanks. codex didn't find any more issues. I skimmed through the code and it generally looks ok to me, though I haven't thought about it in great detail.

Wint3rNight · 2026-08-13T00:24:26Z

Thanks for pushing on those. I have updated the pr description, it was still describing the original fp16-only version.

arcady-lunarg · 2026-08-19T04:16:57Z

 #include "SymbolTable.h"
 #include "propagateNoContraction.h"

+#include "SPIRV/hex_float.h"


We have avoided including dependencies from the SPIRV/ directory in files under the glslang/ tree because, among other things, the SPIRV/ directory is not built at all if the ENABLE_SPIRV cmake option is disabled. I think the solution here might be to just move the hex_float files somewhere under the main glslang/ hierarchy.

Moved hex_float.h and bitutils.h to glslang/Include/

arcady-lunarg · 2026-08-24T20:34:04Z

 0:10          'w6' ( temp double)
 0:10          Constant:
-0:10            1.2345678901235e+15
+0:10            1.2345679481405e+15


While this change in result is absolutely correct per the spec, I am a little big concerned that this is going to cause regressions in user shaders that implicitly relied on an unsuffixed float literal being parsed as a double and maintaining that precision. The fact that glslang's own test suite relies on this behavior does not make me optimistic that we can get away with this without causing regressions for users.

I dropped the rounding for float/double and kept it just for the sub 32-bit types. If you'd rather have float rounding behind an opt-in flag, say so and I'll do that instead.

jeffbolznv reviewed Aug 6, 2026

View reviewed changes

Wint3rNight force-pushed the round-const-precision branch 2 times, most recently from e24c910 to 631223c Compare August 6, 2026 17:49

Wint3rNight added 2 commits August 7, 2026 02:04

Round low precision float constants when storing them

92789ca

Round low precision float constants when storing them

952d56f

Wint3rNight force-pushed the round-const-precision branch from 631223c to 952d56f Compare August 6, 2026 20:44

Wint3rNight added 2 commits August 8, 2026 01:10

Wint3rNight force-pushed the round-const-precision branch from 266180b to d04fe78 Compare August 8, 2026 02:48

Fix NaN handling in comparison and selection constant folds

499569a

Wint3rNight force-pushed the round-const-precision branch from c178989 to 499569a Compare August 12, 2026 03:25

Wint3rNight force-pushed the round-const-precision branch from 5eaa931 to 0f9a9fe Compare August 12, 2026 09:20

Wint3rNight force-pushed the round-const-precision branch from 0f9a9fe to 41f6946 Compare August 12, 2026 09:31

arcady-lunarg self-requested a review August 19, 2026 00:26

arcady-lunarg reviewed Aug 19, 2026

View reviewed changes

Move hex_float.h and bitutils.h into glslang/Include

6137927

arcady-lunarg reviewed Aug 24, 2026

View reviewed changes

Leave float and double constants unrounded

fa1c002

Wint3rNight requested a review from arcady-lunarg September 10, 2026 22:29


		namespace {

		// IEEE 754 binary16 <-> binary32, rounding toward zero on the way down.

Conversation

Wint3rNight commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffbolznv commented Aug 7, 2026

Uh oh!

Wint3rNight commented Aug 7, 2026

Uh oh!

jeffbolznv commented Aug 8, 2026

Uh oh!

Wint3rNight commented Aug 8, 2026

Uh oh!

jeffbolznv commented Aug 9, 2026

Uh oh!

Wint3rNight commented Aug 9, 2026

Uh oh!

jeffbolznv commented Aug 9, 2026

Uh oh!

Wint3rNight commented Aug 9, 2026

Uh oh!

jeffbolznv commented Aug 9, 2026

Uh oh!

Wint3rNight commented Aug 11, 2026

Uh oh!

jeffbolznv commented Aug 11, 2026

Uh oh!

jeffbolznv commented Aug 12, 2026

Uh oh!

Wint3rNight commented Aug 12, 2026

Uh oh!

jeffbolznv commented Aug 12, 2026

Uh oh!

Wint3rNight commented Aug 12, 2026

Uh oh!

jeffbolznv commented Aug 12, 2026

Uh oh!

Wint3rNight commented Aug 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Wint3rNight commented Aug 6, 2026 •

edited

Loading