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

A commit causes cl-denorms-are-zero is added to char_rtn_float test where gForceFTZ is false. #2066

Open
haonanya opened this issue Sep 3, 2024 · 2 comments

Comments

@haonanya
Copy link

haonanya commented Sep 3, 2024

The change https://github.com/KhronosGroup/OpenCL-CTS/blob/main/test_conformance/conversions/basic_test_conversions.cpp#L1543 is from b6941b6.
Without the commit cl-denorms-are-zero is not added for char_rtn_float test. With the commit for char_rtn_float test gForceFTZ is false and gForceHalfFTZ is true and then cl-denorms-are-zero is added.

opencl test is like:
convert_char_rtn(-0x1.fffffcp-127)

adding the cl-denorms-are-zero option causes output is 0, while host expects -1. opencl spec doesn't explicitly mention flush to zero for input or output or both. There are no cl-denorms-are-zero for cc1 options since llvm/llvm-project@a4451d88ee456304c. Currently intel opencl-clang treats cl-denorms-are-zero as https://github.com/intel/opencl-clang/blob/main/options_compile.cpp#L106 which means flush to zero for input and output https://github.com/llvm/llvm-project/blob/main/llvm/docs/LangRef.rst#L2424.
opencl spec mention that This is intended to be a performance hint and the OpenCL compiler can choose not to flush denorms to zero if the device supports single precision (or double precision) denormalized numbers. But this is not very same as denormal-fp-math which say If the input mode is "preserve-sign", or "positive-zero", a floating-point operation must treat any input denormal value as zero.
@bashbaug , @svenvh , do you have any comments for this? Thanks very much.

@svenvh
Copy link
Member

svenvh commented Sep 3, 2024

Without the commit cl-denorms-are-zero is not added for char_rtn_float test. With the commit for char_rtn_float test gForceFTZ is false and gForceHalfFTZ is true and then cl-denorms-are-zero is added.

That seems like a bug in the test. Addition of the fp16 tests should not have caused the testing behaviour for fp32 to change. @haonanya I have opened #2067, could you please confirm if that fixes the issue for char_rtn_float on your side?

opencl spec mention that This is intended to be a performance hint and the OpenCL compiler can choose not to flush denorms to zero if the device supports single precision (or double precision) denormalized numbers. But this is not very same as denormal-fp-math which say If the input mode is "preserve-sign", or "positive-zero", a floating-point operation must treat any input denormal value as zero.

Setting -cl-denorms-are-zero predates the fp16 additions, but I agree something looks odd there too. The specification states that "This option is ignored for single precision numbers if the device does not support single precision denormalized numbers i.e. CL_FP_DENORM bit is not set in CL_DEVICE_SINGLE_FP_CONFIG". But the conversions test seems to do exactly that: add the option when CL_FP_DENORM is not set.

@haonanya
Copy link
Author

haonanya commented Sep 4, 2024

@svenvh , thanks, the test passes with your fix.

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