Eric Biggers <[email protected]> wrote: > On Thu, Nov 20, 2025 at 01:55:18PM +0000, David Howells wrote: > > Eric Biggers <[email protected]> wrote: > > > > > + /* Compute d = (c mod 2^32) * (q^-1 mod 2^32). */ > > > + s32 d = (s32)c * QINV_MOD_R; > > > > Hmmm... is "(s32)c" actually "(c mod 2^32)"? Should that be: > > > > u32 d = (u32)c * QINV_MOD_R; > > > > This is followed up by casting 'd' to "s64". I don't think that should > > sign-extend it, but... > > It selects the representative in the range [INT32_MIN, INT32_MAX], > rather than the representative in the range [0, UINT32_MAX]. The sign > extension is intentional.
I'm concerned about the basis on which it becomes positive or negative. It looks like the sign bit ends up being chosen arbitrarily. > > > + /* Reduce to [0, q), then tmp = w'_1 = UseHint(h, w'_Approx) */ > > > > Bracket mismatch. "[0, q]" > > It's intentional, since it denotes a mathematical range. Elsewhere I > used the words "the range" explicitly, so I'll add that above too. (Or > maybe reword it differently.) I meant you have an opening square bracket and a closing round bracket in "[0, q)". David
