Add pure FixedPoint sqrt implementation and tests #1770
Conversation
heinezen
left a comment
There was a problem hiding this comment.
Very nice, thanks a lot :)
I have a few comments about the code, but nothing too serious.
libopenage/util/fixed_point_test.cpp
Outdated
| } | ||
|
|
||
| // We lose some precision when raw_type == intermediate_type | ||
| for (uint64_t i = 1; i < (1ul << 63); i = (i << 1) ^ i) { |
There was a problem hiding this comment.
Same here, I think the std::numeric_limits definitions are more readable. Also, shifting by one is just multiplication by 2 and the compiler can probably optimize that out on its own :D
| for (uint64_t i = 1; i < (1ul << 63); i = (i << 1) ^ i) { | |
| for (uint64_t i = 1; i < std::numeric_limits<uint64_t>::max(); i = (i * 2) ^ i) { |
|
Thank you for all the feedback! I think I've incorporated everything. If this all looks good, I'd be happy to work on some of the other math functions too. (Hopefully I'll be able to break my C habits!) I ran into an interesting bug / edge case with An unsigned fixed point 0.0 and a double 0.0 fail the equality check. Here's an example case that fails on x86: I think it should be possible to resolve this by checking for |
heinezen
left a comment
There was a problem hiding this comment.
I have a very small suggestion but everything else looks nice.
Co-authored-by: Christoph Heine <6852422+heinezen@users.noreply.github.com>
|
Thanks again! |
|
Thank you for all the guidance! I might take a look at some of the others soon. |
This implements a pure fixed point square root function per #1543.
Notes:
I've just replaced the existing function with a pure fixed point implementation (can't overload when only return type differs). It should be trivial to make it return an actual FixedPoint instead, but that would requiring updating prior test cases and I wasn't sure if that was okay to do.
There's a small loss in precision depending on the value of
fractional_bitsand the position of the most significant bit: if the integer portion is very large, we won't have as much (absolute) precision. Ideally you would want theintermediate_typeto be twice the size ofraw_typeto avoid any losses.