Skip to content

[UnitTest] Add test for fmax reductions without fast-math.#266

Merged
fhahn merged 4 commits intollvm:mainfrom
fhahn:vector-fmax-without-fast-math
Jul 18, 2025
Merged

[UnitTest] Add test for fmax reductions without fast-math.#266
fhahn merged 4 commits intollvm:mainfrom
fhahn:vector-fmax-without-fast-math

Conversation

@fhahn
Copy link
Copy Markdown
Contributor

@fhahn fhahn commented Jul 17, 2025

Add unit tests for vectorizing FMax reductions without fast-math flags, covering a wide range of inputs, including various combinations of NaNs and signed-zeros.

Add unit tests for vectorizing FMax reductions without fast-math flags,
covering a wide range of inputs, including various combinations of NaNs
and signed-zeros.
check(ScalarFn, VectorFn, &Src1[0], N, "full-with-multiple-nan");
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would be good to test signaling nans, but it will be busted all over the place

Test some denormals?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added some tests with denormals both as start value and in the inputs, thanks

At least for AArch64, the tests also pass when replacing all quiet nans with signaling nans with llvm/llvm-project#148239, as it just checks if it matches the behavior of the scalar loop.

But in general it may be a bit risky to check signaling NaNs, as the behavior may not be 100% consistent across platforms?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It should be, but the state is in flux and has never been consistent

Comment on lines +11 to +14
if (std::isnan(A))
return std::isnan(B);

if (A == 0.0f)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think if isnan(B) and A == 0.0f this becomes dependent on the nan signbit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I updated the check above to check if either is nan, in that case both must be nan to be equal. I also check here if B is zero. I think that should take care of that.

@fhahn fhahn merged commit 42dab80 into llvm:main Jul 18, 2025
1 check passed
@fhahn fhahn deleted the vector-fmax-without-fast-math branch July 18, 2025 14:23
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.

2 participants