Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect result for fp16 fract function for small negative inputs #307

Closed
bashbaug opened this issue Nov 27, 2023 · 2 comments
Closed

incorrect result for fp16 fract function for small negative inputs #307

bashbaug opened this issue Nov 27, 2023 · 2 comments

Comments

@bashbaug
Copy link

bashbaug commented Nov 27, 2023

Found during a review of new fp16 CTS tests:

KhronosGroup/OpenCL-CTS#1681 (review)

IGC is currently generating incorrect results for fract for small negative inputs. Specifically, it is returning 1.0, rather than a number close to but not quite 1.0.

For -0.000061 (8400): fract() returned 1.000000 (3C00) + -1.000000 (BC00)

It looks like this is because IGC is using to the fp32 constant 0x1.fffffep-1f rather than the fp16 constant 0x1.ffcp-1f:

INLINE half SPIRV_OVERLOADABLE SPIRV_OCL_BUILTIN(fract, _f16_p0f16, )( half x,
__private half* iptr )
{
*iptr = SPIRV_OCL_BUILTIN(select, _f16_f16_i16, )( SPIRV_OCL_BUILTIN(floor, _f16, )( x ), SPIRV_OCL_BUILTIN(nan, _i16, )( (short)0 ), (short)(SPIRV_BUILTIN(IsNan, _f16, )( x )) );
half temp = x - *iptr;
temp = SPIRV_OCL_BUILTIN(select, _f16_f16_i16, )( (half)SPIRV_OCL_BUILTIN(fmin, _f16_f16, )( temp, (half)(0x1.fffffep-1f)), (half)SPIRV_OCL_BUILTIN(copysign, _f16_f16, )((half)0.0f, x), (short)SPIRV_BUILTIN(IsInf, _f16, )(x));
return SPIRV_OCL_BUILTIN(select, _f16_f16_i16, )( temp, SPIRV_OCL_BUILTIN(nan, _i16, )((short)0), (short)(SPIRV_BUILTIN(IsNan, _f16, )(x)) );
}

If I use the fp16 constant instead:

half my_fract_fix(half x, private half* iptr) {
    *iptr = select(floor(x), nan((ushort)0), (short)isnan(x));
    half temp = x - *iptr;
    temp = select(fmin(temp, (half)(0x1.ffcp-1f)), (half)copysign((half)0.0f, x), (short)isinf(x));
    return select(temp, nan((ushort)0), (short)isnan(x));
}

Then I get the right results:

For -0.000061 (8400): fract() returned 0.999512 (3BFF) + -1.000000 (BC00)
@pszymich
Copy link
Contributor

Fixed in 0db1db3
Thanks @bashbaug for the bug submission and thanks @MichalMroz12 for the fix.

@bashbaug
Copy link
Author

It looks like the fix for this issue was backed out in 3cf68d7

So, it is still occurring. Can this issue be re-opened?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants