Skip to content

[Mirror] MueLu: Enable floating point exceptions in Automatic_Test_ETI#70

Open
csiefer2 wants to merge 1 commit intodevelopfrom
pr-mirror-14733
Open

[Mirror] MueLu: Enable floating point exceptions in Automatic_Test_ETI#70
csiefer2 wants to merge 1 commit intodevelopfrom
pr-mirror-14733

Conversation

@csiefer2
Copy link
Copy Markdown
Owner

@csiefer2 csiefer2 commented Apr 7, 2026

Automated mirror of upstream PR trilinos#14733 @trilinos/muelu

Signed-off-by: malphil <malphil@sandia.gov>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 7, 2026

CDash for AT1 results [Only accessible from Sandia networks]
CDash for AT2 results [Currently only accessible from Sandia networks]

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to enable floating-point exceptions (Divide-by-Zero, Overflow, and Invalid) for Linux and macOS platforms within the MueLu unit tests. The enableFPE function is added to the MueLu_Test namespace and invoked during test initialization. Feedback suggests marking the function as inline to prevent linker errors, fixing a state-tracking bug in the macOS implementation, and adopting a stateless approach for toggling exception bits.

Comment on lines +51 to +71
void enableFPE(bool enableFPE) {
#if defined(__APPLE__) && defined(__GNUC__) && defined(__SSE__)
static int eMask = _MM_GET_EXCEPTION_MASK();
#endif

if (enableFPE) {
#if defined(__linux__) && defined(__GNUC__) && !defined(__INTEL_COMPILER)
feenableexcept(FE_DIVBYZERO | FE_OVERFLOW | FE_INVALID);
#elif defined(__APPLE__) && defined(__GNUC__) && defined(__SSE__)
eMask = _MM_GET_EXCEPTION_MASK(); // Save current eMask so we can disable.
_MM_SET_EXCEPTION_MASK(eMask & ~_MM_MASK_DIV_ZERO & ~_MM_MASK_OVERFLOW &
~_MM_MASK_INVALID);
#endif
} else {
#if defined(__linux__) && defined(__GNUC__) && !defined(__INTEL_COMPILER)
fedisableexcept(FE_DIVBYZERO | FE_OVERFLOW | FE_INVALID);
#elif defined(__APPLE__) && defined(__GNUC__) && defined(__SSE__)
_MM_SET_EXCEPTION_MASK(eMask);
#endif
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The enableFPE function has a few issues that should be addressed:

  1. Multiple Definition Risk: Since the function is defined in a header file, it should be marked inline to prevent linker errors if the header is included in multiple translation units.
  2. Logic Bug on Apple/SSE: The use of a static variable with an assignment (eMask = _MM_GET_EXCEPTION_MASK()) inside the if (enableFPE) block is problematic. If enableFPE(true) is called twice, the static variable will store the "enabled" state, causing a subsequent call to enableFPE(false) to fail to restore the original disabled state.
  3. Stateless Implementation: It is cleaner and more robust to implement this in a stateless manner by toggling only the specific exception bits, which also makes the Apple implementation consistent with the Linux/glibc behavior.
inline void enableFPE(bool enableFPE) {
#if defined(__linux__) && defined(__GNUC__) && !defined(__INTEL_COMPILER)
  if (enableFPE) {
    feenableexcept(FE_DIVBYZERO | FE_OVERFLOW | FE_INVALID);
  } else {
    fedisableexcept(FE_DIVBYZERO | FE_OVERFLOW | FE_INVALID);
  }
#elif defined(__APPLE__) && defined(__GNUC__) && defined(__SSE__)
  if (enableFPE) {
    _MM_SET_EXCEPTION_MASK(_MM_GET_EXCEPTION_MASK() & ~(_MM_MASK_DIV_ZERO | _MM_MASK_OVERFLOW | _MM_MASK_INVALID));
  } else {
    _MM_SET_EXCEPTION_MASK(_MM_GET_EXCEPTION_MASK() | (_MM_MASK_DIV_ZERO | _MM_MASK_OVERFLOW | _MM_MASK_INVALID));
  }
#endif
}

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