Skip to content

fix: harden reassociation barriers for fast-math#1276

Open
DiamonDinoia wants to merge 1 commit intoxtensor-stack:masterfrom
DiamonDinoia:fix/nearbyint-fastmath
Open

fix: harden reassociation barriers for fast-math#1276
DiamonDinoia wants to merge 1 commit intoxtensor-stack:masterfrom
DiamonDinoia:fix/nearbyint-fastmath

Conversation

@DiamonDinoia
Copy link
Copy Markdown
Contributor

@DiamonDinoia DiamonDinoia commented Mar 19, 2026 •
edited
Loading

There is a bug in nearbyint -fassociative-math breaks it as it does not define __FAST_MATH__.
Also the barrier used was causing a stack spill.

I centralized a barrier function that we can use everywhere in the code and used it in the places I know it helps.

Now we can just use reassociation_barrier to avoid compiler reordering of instructions.

Let me know if you like the internal API and if you need changes to it. I find that this is the solution that minimizes ifdef boilerplate and allows to dispatch to all archs. (With c++17 this will be simpler).

Cheers,
Marco

@DiamonDinoia DiamonDinoia force-pushed the fix/nearbyint-fastmath branch 5 times, most recently from f9a9992 to 2f9a431 Compare March 23, 2026 19:55
@DiamonDinoia DiamonDinoia changed the title fix: harden reassociation barriers for fast-math nearbyint fix: harden reassociation barriers for fast-math Mar 23, 2026
@DiamonDinoia
Copy link
Copy Markdown
Contributor Author

DiamonDinoia commented Mar 23, 2026

@serge-sans-paille this is also ready for review.

@DiamonDinoia DiamonDinoia mentioned this pull request Mar 23, 2026
Open
serge-sans-paille
serge-sans-paille requested changes Mar 27, 2026
View reviewed changes
@DiamonDinoia DiamonDinoia force-pushed the fix/nearbyint-fastmath branch 5 times, most recently from 3685ff0 to c4dec73 Compare March 27, 2026 19:57
@DiamonDinoia DiamonDinoia mentioned this pull request Mar 27, 2026
Open
@DiamonDinoia
Copy link
Copy Markdown
Contributor Author

DiamonDinoia commented Mar 27, 2026

I think I can simplify this a bit more. A is not needed anymore if I we go this route.

@DiamonDinoia DiamonDinoia force-pushed the fix/nearbyint-fastmath branch 2 times, most recently from 6d6bc61 to d571f2f Compare March 28, 2026 22:42
serge-sans-paille
serge-sans-paille reviewed Mar 29, 2026
View reviewed changes
include/xsimd/arch/common/xsimd_common_details.hpp Outdated
template <class T>
XSIMD_INLINE void reassociation_barrier(T& x, const char*) noexcept
{
#if XSIMD_WITH_INLINE_ASM
Copy link
Copy Markdown
Contributor

@serge-sans-paille serge-sans-paille Mar 29, 2026

Choose a reason for hiding this comment

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

Shouldn't we make this empty if we're not under fast-math?

@DiamonDinoia
Copy link
Copy Markdown
Contributor Author

DiamonDinoia commented Mar 29, 2026 via email

@serge-sans-paille
Copy link
Copy Markdown
Contributor

serge-sans-paille commented Mar 29, 2026

Well, I don't think we can say that

__asm__ volatile("" : : "r"(&x) : "memory");

is a no-op. Should we allow for user-defined XSIMD_REASSOCIATIVE which would be forced by __FAST_MATH__ but could also be set by the user?
I don't want fast-math to impact non-fast math code generation.

@DiamonDinoia
Copy link
Copy Markdown
Contributor Author

DiamonDinoia commented Mar 29, 2026 •
edited
Loading

Let me think about this for a second. MSVC does not allow inline asm. I should update the comment there. There might be better alternatives for scalars, WASM, and RISC-V that don't impact performance.

@DiamonDinoia DiamonDinoia force-pushed the fix/nearbyint-fastmath branch 3 times, most recently from b34c94a to d95d1d3 Compare March 30, 2026 16:53
@DiamonDinoia
fix: harden fast-math reassociation barriers
edba47b 
- Add zero-cost register constraints for all supported architectures:
  x86 "+x", ARM NEON/SVE "+w", PPC VSX "+wa", RISC-V scalar "+f",
  RISC-V RVV "+vr" (GCC 15+ / Clang 20+).
- Replace old "r"(&x):"memory" fallback with "+m" guarded by new
  XSIMD_REASSOCIATIVE_MATH macro so unknown targets only spill when
  the compiler can actually reassociate.
- Add XSIMD_REASSOCIATIVE_MATH config macro, auto-detected from
  __FAST_MATH__ / __ASSOCIATIVE_MATH__, user-overridable for Clang
  with standalone -fassociative-math.
- Add std::array overload so emulated batches get per-element
  barriers instead of spilling the whole array.
- Add missing barriers in exp/exp2/exp10 range reduction (float and
  double) after nearbyint() to prevent compensated subtraction
  reordering.
- Add missing barriers in log2 (float and double) after to_float(k)
  to protect Kahan compensated summation.
@DiamonDinoia DiamonDinoia force-pushed the fix/nearbyint-fastmath branch from d95d1d3 to edba47b Compare March 30, 2026 16:57
@DiamonDinoia
Copy link
Copy Markdown
Contributor Author

DiamonDinoia commented Mar 30, 2026

I tried to have the no op always executed and the spill asm is guarded with macros. I think is the best solution as users should not worry about FP reordering in practice, except corner cases. We could document those if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serge-sans-paille serge-sans-paille Awaiting requested review from serge-sans-paille

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@DiamonDinoia @serge-sans-paille