Bug in conversion from float to long long
mateoconlechuga opened this issue · 2 comments
See relevant Cemetech issue here: https://www.cemetech.net/forum/viewtopic.php?p=304415#304415
Fix here: 6678f56
[9:43 PM]calc84maniac: there are two ways I can think of
[9:44 PM]calc84maniac: one is changing the <= back to < so it always shifts right by at least 1 in the optimized path
[9:44 PM]calc84maniac: the other is changing the const int24_t result to a const long long result
[9:45 PM]MateoConLechuga: I think I like that one
[9:45 PM]calc84maniac: the difference is in the optimized path it'll now be zero extending to 64-bit and then optionally negating, instead of optionally negating and then sign extending to 64-bit
[9:46 PM]calc84maniac: so the negation is going to be slower, but zero extending in the first place is cheaper
[9:46 PM]MateoConLechuga: it's float anyway, accuracy over speed 😛
[9:48 PM]calc84maniac: if <= is changed to <, then in the broken case it'll instead shift the 64-bit value left by 0 and then conditionally negate
[9:48 PM]calc84maniac: so the optimized path still gets to use a 24-bit negation
[9:49 PM]MateoConLechuga: idk making the type be the same as the return type feels "safer" to me
[9:49 PM]MateoConLechuga: even though it might not matter
Added regression test in bug0002, merged to master.