intel / intel-graphics-compiler

Other
594 stars 155 forks source link

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

Closed bashbaug closed 7 months ago

bashbaug commented 9 months ago

Found during a review of new fp16 CTS tests:

https://github.com/KhronosGroup/OpenCL-CTS/pull/1681#pullrequestreview-1737478851

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:

https://github.com/intel/intel-graphics-compiler/blob/d9e9d2961d14b88748fd27f8a1d65d76c5e4420b/IGC/BiFModule/Implementation/Math/fract.cl#L288-L295

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 commented 7 months ago

Fixed in https://github.com/intel/intel-graphics-compiler/commit/0db1db3c3bffa255e90912924871f60e0b6ab24c Thanks @bashbaug for the bug submission and thanks @MichalMroz12 for the fix.

bashbaug commented 3 months ago

It looks like the fix for this issue was backed out in https://github.com/intel/intel-graphics-compiler/commit/3cf68d7a152e776010cacbee4d2a37986c581455

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