Skip to content

Fix nearbyint and rint in directed rounding modes - #1425

Open
monishpeddapally wants to merge 1 commit into
xtensor-stack:masterfrom
monishpeddapally:fix/current-rounding-mode
Open

monishpeddapally wants to merge 1 commit into
xtensor-stack:masterfrom
monishpeddapally:fix/current-rounding-mode

Conversation

@monishpeddapally

Copy link
Copy Markdown

Fixes #1424.

Batch nearbyint and rint currently return incorrect values under directed rounding. The common kernel rounds magnitudes before restoring the sign, which reverses the direction for negative inputs. SSE4.1, AVX and AVX-512 explicitly select round-to-nearest.

Keep signed inputs during the common kernel's rounding arithmetic, preserve signed zero, and use the current rounding direction in the x86 instructions. The regression checks mixed positive and negative lanes, halfway cases, integral values and signed zero for float and double. It restores the original rounding mode and skips modes unsupported by the platform.

Validation on macOS with Apple Clang 21:

  • Final regression against the original headers: 72 failed assertions. With the fix: all 520 assertions pass.
  • Full ARM NEON suite: 412 test cases and 6,526 assertions pass.
  • Rounding tests pass for SSE2, SSE4.1 and AVX2 under Rosetta, and for emulated 128-, 256- and 512-bit batches, compiled with warnings as errors.
  • A separate scalar comparison passes for the affected ARM and x86 kernels, including large integral values, infinities, NaN and signed zero.
  • AVX-512 compiles successfully; runtime validation needs AVX-512 hardware.
  • Changed-line clang-format and git diff --check pass. The local inline-specifier script could not run because clang-query is unavailable.

This change addresses returned values. Floating-point exception-flag behavior is outside its scope.

The common kernel rounds the absolute input before restoring its sign,
which reverses directed rounding for negative values. Keep the input
signed during addition and subtraction, and copy its sign to the result
to preserve signed zero.

Use the current rounding direction in SSE4.1, AVX and AVX-512 instructions
instead of selecting round-to-nearest. Add mixed-lane regressions for
float and double across the four standard rounding modes.

Fixes xtensor-stack#1424

Signed-off-by: Monish Peddapally <93381641+monishpeddapally@users.noreply.github.com>
@DiamonDinoia

Copy link
Copy Markdown
Contributor

we should test, nan, inf, and numbers largest than the largest representable integer since we are at it. Probably we should also fix the use of _MM_FROUND_CUR_DIRECTION I think we are suppressing exceptions there.

@DiamonDinoia DiamonDinoia left a comment

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.

Please, double check the code as I think there is some AI usage in it that might have changed things that should not be changed.

return s ^ select(v < t2n, d, v);
batch_type signed_t2n = t2n | s;
batch_type d0 = self + signed_t2n;
detail::reassociation_barrier(d0, "prevent collapsing (x + c) - c back to x");

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.

where is c? why did you change the variable names in the string?

This branch has not been deployed

No deployments
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.

Batch nearbyint and rint do not follow the current rounding mode

2 participants