-
Notifications
You must be signed in to change notification settings - Fork 766
add patch for FlexiBLAS v3.3.1 to fix LAPACK errors triggered by -ftree-vectorize on zen4/zen5 microarchs
#23974
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
add patch for FlexiBLAS v3.3.1 to fix LAPACK errors triggered by -ftree-vectorize on zen4/zen5 microarchs
#23974
Conversation
|
I guess @bartoldeman input on this would be nice For more context, I tested to backport FlexiBLAS 3.4.4 (which builds and tests fine with tree-vectorize on 2024a) into 2023a and it also fails on tests with the same kind of errors. So the issue does not seem to be the FlexiBLAS/Lapack version, but that the GCC versions on 2023a,b is not producing proper vectorized builds of lapack code for zen4/zen5. |
-ftree-vectorize for netlib LAPACK
|
I'll have to look at this carefully. I'd prefer a patch but using So please try |
|
Reference-LAPACK/lapack#1033 disabled FMAs in one place using strategic parentheses. |
|
@bartoldeman I tried changing the patch to use |
|
@boegelbot please test @ jsc-zen3 |
|
@boegel: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 3326640773 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot |
|
Test report by @boegel |
|
I re-iterate my standpoint from way back, lapack's test code and matgen MUST be built without optimization of any kind and with -O0 since that code isn't written to handle compiler optimizations. |
|
A little bit more digging: allocates the relevant matrices which bypasses the miscompiled code here, so that's a source code based workaround. |
ok, i was just testing your other proposed patch, so we prefer this source code modif then, right ? |
Updated software
|
yes I'd prefer the source code modification as it is more targeted. |
bartoldeman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
I changed the fix in order to use the patch lapack code option. @bartoldeman for the I applied the same logic of making the work array dynamic on this other routine and it worked, so we have different patches for each toolchain. |
-ftree-vectorize for netlib LAPACK-ftree-vectorize on zen4/zen5 microarchs
|
finally this was fixed patching a gcc bug: |
This fixes a segmentation fault in FlexiBLAS LAPACK tests when compiling for Zen4/Zen5 See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114566 and #23974
(created using
eb --new-pr)With the modification of the bundle.py easyblock to include the tests of components introduced on eb v5.1.1 easyblocks PR#3748 we started hitting errors when testing the install of FlexiBLAS 3.3.1 on zen4/zen5.
It segfaults as:
Applying the
-fno-tree-vectorizeoption for the netlib LAPACK build as proposed in this PR fixes the issue.We've hit this kind of issues on the past for lapack code as in #19280
I don't know if we can go for disabling tree-vectorize fully as proposed with the patch, or for a more targeted fix, patching the lapack code as it was done on these #20745 #16406