On Fri, Jul 18, 2014 at 5:27 PM, Matt Turner <[email protected]> wrote: > On Fri, Jul 18, 2014 at 9:19 AM, Ilia Mirkin <[email protected]> wrote: >> The current code appears to work in simple tests, however this will >> guarantee that the returned exponent is 0 for a 0 value. >> >> Signed-off-by: Ilia Mirkin <[email protected]> >> --- >> >> I couldn't make a simple test-case that would cause the current logic to >> fail. However when I did the same thing with doubles, I ran into trouble. It >> seems safer to move the csel outside of the add in case the value actually >> has >> a non-0 exponent despite a 0 significand. >> >> src/glsl/builtin_functions.cpp | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp >> index e01742c..5755de9 100644 >> --- a/src/glsl/builtin_functions.cpp >> +++ b/src/glsl/builtin_functions.cpp >> @@ -4229,8 +4229,8 @@ builtin_builder::_frexp(const glsl_type *x_type, const >> glsl_type *exp_type) >> * to unsigned integers to ensure that 1 bits aren't shifted in. >> */ >> body.emit(assign(exponent, rshift(bitcast_f2i(abs(x)), exponent_shift))); >> - body.emit(assign(exponent, add(exponent, csel(is_not_zero, exponent_bias, >> - imm(0, vec_elem))))); >> + body.emit(assign(exponent, csel(is_not_zero, add(exponent, >> exponent_bias), >> + imm(0, vec_elem)))); > > So you're changing the logic from > > exponent = (f2i(abs(x) >> 23) + (x != 0.0f) ? -126 : 0; > > to > > exponent = (x != 0.0f) ? (f2i(abs(x) >> 23) - 126 : 0; > > These seem identical to me, and trivially so for 0.0f/-0.0f. I have a > feeling that this patch is papering over a bug in your code generation > for f2i(abs(x)).
Could be, I'll take a closer look at the generated code. Thanks for the hint. > In commit 9c48ae75 I fixed a bug in i965 where instead of generating > f2i(abs(x)) we'd apply the source modifier effectively after the > bitcast (reading the register with a different type), so we'd get > abs(f2i(x)). > > For 0.0f, applying the f2i and abs out of order doesn't affect the > result, but for -0.0f (0x80000000, -2147483648) instead of getting 0, > you'd get abs(-2147483648) (which is likely -2147483648 itself!). Couldn't you have a situation where 0.0 or -0.0 are actually not in normal form, and are, e.g. 0xff800000 or something (i.e. non-0 exponent)? I was concerned about that situation. If the floats are guaranteed to be in normalized form, then you're right that those two should be identical. -ilia _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
