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

Specify precision in the format flag for float4 hexadecimal output #2064

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

delecui
Copy link
Contributor

@delecui delecui commented Aug 30, 2024

This will ensure consistent output on any conforming implementation.
Note that the specification does not define the output style when
precision is missing.

MSVC outputs full precision representation with trailing zeros for "%a"
specifier on Windows, which also conforms to OpenCL 3.0 specs.

Signed-off-by: Cui, Dele <dele.cui@intel.com>
@CLAassistant
Copy link

CLAassistant commented Aug 30, 2024

CLA assistant check
All committers have signed the CLA.

@delecui delecui marked this pull request as ready for review August 30, 2024 08:17
@delecui

This comment was marked as resolved.

This will ensure consistent output on any conforming implementation.
Note that the specification does not define the output style when
precision is missing.

Signed-off-by: Cui, Dele <dele.cui@intel.com>
@delecui delecui changed the title Update reference for float4 hexadecimal formatted output on Windows Specify precision in the format flag for float4 hexadecimal output Aug 30, 2024
@delecui
Copy link
Contributor Author

delecui commented Sep 2, 2024

Hi @svenvh, Could you help merge this PR? Thanks. I don't have permission to merge.

@svenvh
Copy link
Member

svenvh commented Sep 2, 2024

Hi @svenvh, Could you help merge this PR? Thanks. I don't have permission to merge.

We may want to leave this open for a few more days, so that other vendors have a chance to look at it.

@delecui
Copy link
Contributor Author

delecui commented Sep 2, 2024

Hi @svenvh, Could you help merge this PR? Thanks. I don't have permission to merge.

We may want to leave this open for a few more days, so that other vendors have a chance to look at it.

Understood. Thanks for the update. Will wait for further reviews.

@svenvh
Copy link
Member

svenvh commented Sep 6, 2024

Close/reopen to retrigger CI.

@svenvh svenvh closed this Sep 6, 2024
@svenvh svenvh reopened this Sep 6, 2024
@delecui
Copy link
Contributor Author

delecui commented Sep 19, 2024

Hi @svenvh, Can we land this PR now ? Thanks.

@svenvh
Copy link
Member

svenvh commented Sep 19, 2024

This has been open for almost 3 weeks now, and has 2 approvals, so merging.

@svenvh svenvh merged commit 9f88b0b into KhronosGroup:main Sep 19, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants