stb icon indicating copy to clipboard operation
stb copied to clipboard

Replaced ldexp when decoding .hdr images with bit op equivalent

Open TheSandvichMaker opened this issue 3 years ago • 7 comments

This speeds up loading .hdr images somewhere between 2.6x-2.8x on my machine.

TheSandvichMaker avatar Jan 09 '21 22:01 TheSandvichMaker

This is not a safe way to do bit operations between float and int. Current practice is to use a union to convert between them, although there are indications that compiler writers have decided that that is also undefined behavior as well and there's some other thing we're supposed to be doing, I don't even remember what that is.

nothings avatar Jun 04 '21 18:06 nothings

Probing the Molly Rocket discord, the answer for C89 appears to be memcpy, union isn't considered safe until C99. Modern C/C++ compilers seem to happily do the right thing with memcpy:

C: https://godbolt.org/z/oYTb8oj9M C++: https://godbolt.org/z/Msdn5393G

TheSandvichMaker avatar Jun 04 '21 22:06 TheSandvichMaker

One concern is that on older compilers they may explicitly do a memory copy, which will behave correctly, but maybe will be even slower than ldexp?

nothings avatar Jun 05 '21 13:06 nothings

Yeah, that is a relevant point. Testing the oldest version available on godbolt of each of the "major three" still behaves correctly, but I can't speak for VC6 for example. Since type punning through unions appears to be happening in the stb codebases anyway, should I just switch to that?

EDIT: I am also amusingly reminded that type punning through unions is undefined behaviour in C++ too... Imagine how many codebases would break...

TheSandvichMaker avatar Jun 05 '21 16:06 TheSandvichMaker

I don't think VC6 performance is worth holding things up, so if it works on oldest versions on godbolt, I'm ok with that. (You don't use VC6 for perfomance. It's still optimizing for Pentium Pro.)

nothings avatar Jun 05 '21 16:06 nothings

union isn't considered safe until C99.

Hi, C11 adds following footnote (95) regarding type punning through unions. I don't see anything similar in C99, so it's probably unsafe in C99 as well unless I've missed something.

If the member used to read the contents of a union object is not the same as the member last used to store a value in the object, the appropriate part of the object representation of the value is reinterpreted as an object representation in the new type as described in 6.2.6 (a process sometimes called "type punning"). This might be a trap representation.

N-R-K avatar Jan 17 '22 11:01 N-R-K

StackOverflow discussion on type-punning through unions under various C standards. TLDR: that footnote was also added to C99 via a Technical Corrigendum, although the paranoid note that footnotes are not normative.

I’ve never heard of a C compiler where type-punning via unions didn’t work, it’s a widespread practice, and the footnote makes it even less likely that it will be broken by compiler writers in the future, so problems seem unlikely in C.

As @TheSandvichMaker noted, the same can’t be said for C++, where it’s definitively not officially OK to type-pun via unions (even though you’ll probably find it works today in practice).

I think memcpy is still safer overall as long as you can rely on inlining on compilers of interest.

musicinmybrain avatar Jan 17 '22 14:01 musicinmybrain