Skip to content

RFC: gallivm: do not introduce needless rounding-error in conversion

Erik Faye-Lund requested to merge kusma/mesa:gallivm-unorm-to-float into main

RFC discussion

This MR is marked RFC, because I believe it's questionable if this actually makes things more correct or just slightly slower. But given how few expected test-failures we have in our CI scripts for these tests, it seems most implementations give the current expected value.

So the big question is, what do we value more here? Behaving closer to other implementations, or performance? My gut reaction would be performance, but I'm a bit unsure, which is why I decided to send out this RFC MR.

Detailed commit message:

When converting a 8-bit unorm-value of 127 to float, most GPUs end up with the floating-point value of 0x3efefeff, which is what you get if you trivially convert this "by the book":

127 / 255.0f = 0x3efefeff

But, we do 1.0 / x and multiply instead. This still works as long as the immediate value is in double precision, which we calculate it in:

scale = 1.0 / 255 = 0.003922 = 0x3f70101010101010 127 * scale = 0x3efefeff

So far, so good. But we convert this value to a 32-bit float first, in lp_build_const_vec(), and then things starts going wrong:

127 * (float)scale = 0x3efeff00

And it's not like we can adjust the scale-factor a tad to account for unfortunate rounding, because:

127 * nextafterf(scale, 0) = 0x3efefefe

Argh. There's simply no 32-bit float that gives us 127 * x = 0x3efefeff.

While OpenGL doesn't explicitly require this conversion to give us that particular value, it seems like a good invariant to have if we can.

So let's change to doing a division, and letting LLVM optimize these to multiplies in the cases where that's possible, if at all.

While this fixes a few piglit failures, those are probably testing things outside the spec, and probably should be rewritten to not require exact values.

Merge request reports