Skip to content
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

POWER: Fix issues in zscal to address lapack failures #4712

Merged
merged 1 commit into from
Jun 1, 2024

Conversation

RajalakshmiSR
Copy link

This patch fixes following lapack failures with clang compiler on POWER.
zed.out: ZVX: 18 out of 5190 tests failed to pass the threshold
zgd.out: ZGV drivers: 25 out of 1092 tests failed to pass the threshold
zgd.out: ZGV drivers: 6 out of 1092 tests failed to pass the threshold

This patch fixes following lapack failures with clang compiler on POWER.
zed.out: ZVX:   18 out of  5190 tests failed to pass the threshold
zgd.out: ZGV drivers:     25 out of   1092 tests failed to pass the threshold
zgd.out: ZGV drivers:      6 out of   1092 tests failed to pass the threshold
Copy link

codspeed-hq bot commented May 22, 2024

CodSpeed Performance Report

Merging #4712 will not alter performance

Comparing RajalakshmiSR:zscalp10 (e112191) with develop (700ea74)

Summary

✅ 16 untouched benchmarks

@@ -38,6 +38,10 @@ USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#pragma GCC optimize "O1"

#if defined(__clang__)
Copy link
Contributor

@ChipKerchner ChipKerchner May 22, 2024

Choose a reason for hiding this comment

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

Is your intention to disable FMA for clang for both SINGLE and DOUBLE versions of this algorithm?

@martin-frbg
Copy link
Collaborator

At some point it will probably make more sense to replace the Reference-LAPACK testsuite, or at least relax its criteria. I only wish I knew how to define that point, and creating a replacement would be a huge task...

@ChipKerchner
Copy link
Contributor

How often are changes/fixes to LAPACK pulled in from?

https://github.com/Reference-LAPACK

@martin-frbg
Copy link
Collaborator

If it is not a documentation-only fix (and not a huge change either, like having both 32 and 64bit integer versions of all functions in the library), usually within days of the PR getting merged (or sometimes as soon as a plausible PR gets posted).
Do you think I missed something lately ?

@ChipKerchner
Copy link
Contributor

Just wondering if I had to manually merge LAPACK changes. Thanks for the clarification.

@martin-frbg
Copy link
Collaborator

I realize we already have the same fix/workaround on x86_64, so merging this after all. Can still discuss adding another ifdef to exclude single-precision COMPLEX later...

@martin-frbg martin-frbg added this to the 0.3.28 milestone Jun 1, 2024
@martin-frbg martin-frbg merged commit 83bc8d5 into OpenMathLib:develop Jun 1, 2024
70 of 76 checks passed
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.

3 participants