Skip to content

add tests for rcp #269

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

add tests for rcp #269

wants to merge 6 commits into from

Conversation

spall
Copy link
Collaborator

@spall spall commented Jun 20, 2025

add 16, 32, and 64 bit float tests for rcp
closes #127

…from ulp to epsilon to allow for larger differences
Copy link
Contributor

@bob80905 bob80905 left a comment

Choose a reason for hiding this comment

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

2 things.

  1. According to the issue, it looks like we should also test 32 and 64 bit int types, is that unnecessary?
  2. For division by 0, direct3dhlsl specs don't seem to specify that the expected result is inf. I could imagine some implementations might return a nan instead. Even though inf seems to work now, is this a case we want to include in these tests, or, like with previous tests, should we abstain from testing for nans / infs?

[numthreads(1,1,1)]
void main() {
Out[0] = rcp(In[0]);
float4 Tmp = {rcp(In[1].xyz), rcp(In[1].w)};
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we want these to be doubles? like double4 rather than float4?

@spall
Copy link
Collaborator Author

spall commented Jul 25, 2025

2 things.

1. According to the issue, it looks like we should also test 32 and 64 bit int types, is that unnecessary?

2. For division by 0, direct3dhlsl specs don't seem to specify that the expected result is inf. I could imagine some implementations might return a nan instead. Even though inf seems to work now, is this a case we want to include in these tests, or, like with previous tests, should we abstain from testing for nans / infs?

the body of the issue never got updated; it only supports floats. Yeah I guess we could remove the dvision by zero case;

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

Successfully merging this pull request may close these issues.

Add test for rcp
4 participants