On Wed, Mar 11, 2015 at 3:34 PM, Carl Worth <[email protected]> wrote: > So I think I'd like to see at least three or four commits here: > > 1. Change _mesa_round_to_even to return a float
I don't think this is really meaningful as a separate change. The current _mesa_round_to_even function uses IROUND() which rounds to integer. I could do the conversion back to float in the function (that's occurring implicitly in ir_expression::constant_expression_value() already). We'd just be moving where the (bad) conversion is happening. Basically, the old thing is broken in multiple ways. Attempting to fix the individual bugs separately is more work than its worth when the new implementation is just rintf(x). > 2. Fix rounding bug in _mesa_round_to_even > 3. Add unit test for recent rounding-bug fix > 4. Optimize _mesa_round_to_even with SSE 4.1 ROUND if available Sure, these seem fine. I kind of agree that giving the new function the same name as something from a spec but giving it slightly different behavior might be bad. Keeping the same name also seems bad though -- changing a function that takes a float and returns an int to take and return a double (assuming the float and double implementations are named _mesa_round_to_evenf and _mesa_round_to_even). Ken suggested _mesa_roundevenf/_mesa_roundeven. I like this. It says it's the roundeven function, but doesn't claim that its completely identical. > With that: Reviewed-by: Carl Worth <[email protected]> I can't really apply a review for a bunch of commits you haven't seen yet. :) I'll send the three updated/split patches. _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
