Skip to content

Remove inf and denorm from f32tof16 input#996

Merged
joaosaffran merged 6 commits intollvm:mainfrom
joaosaffran:bugfix/f32tof16_qc
Mar 25, 2026
Merged

Remove inf and denorm from f32tof16 input#996
joaosaffran merged 6 commits intollvm:mainfrom
joaosaffran:bugfix/f32tof16_qc

Conversation

@joaosaffran
Copy link
Copy Markdown
Contributor

@joaosaffran joaosaffran commented Mar 19, 2026

This patch removes inf and denorms from f32tof16 calls and output. It also changes the expected output representation to not use hex

Fix: #984

@joaosaffran joaosaffran added the test-all When applied to a PR this will opt-in to additional pre-merge test configurations.. label Mar 19, 2026
@joaosaffran joaosaffran changed the title [DO NOT REVIEW] Remove inf from f32tof16 input Remove inf and denorm from f32tof16 input Mar 24, 2026
@joaosaffran joaosaffran marked this pull request as ready for review March 24, 2026 20:23
Comment on lines +61 to +62
# f32tof16 represents f16 in the lower half of resulting uint
Data: [ 0, 12288, 13312, 1024, 13653, 15359, 15360, 15361, 31743, 16968, 15360, 32768, 22080, 54848, 47768, 20800 ]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think we should keep the hex values so it is easier to just take that and paste it to get a half representation.
While we are here though, maybe we could have a second output buffer of RWStructredBuffer OutHalf that we could check for clarity

Although I guess that means this test would require half, so perhaps not

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 I think we should keep the hex values

Copy link
Copy Markdown
Contributor

@kmpeng kmpeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@joaosaffran joaosaffran merged commit 3db6f24 into llvm:main Mar 25, 2026
9 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test-all When applied to a PR this will opt-in to additional pre-merge test configurations..

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[QC][Vulkan] /Feature/HLSLLib/f32tof16.test failling

5 participants