Unify BUFFER_SIZE settings for x86_64 again to fix DYNAMIC_ARCH crashes #2729
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mismatch in DYNAMIC_ARCH builds
|
I think numpy/numpy#16913 is likely this issue. Crashes with Haswell, Zen and Sandybridge, but not on Prescott and Nehalem. |
3 tasks
h-vetinari
added a commit
to h-vetinari/openblas-feedstock
that referenced
this pull request
Jul 24, 2020
Also, turn gfortran-diff into proper patch & put patches into folder. Remove now-unnecessary patch to revert OpenMathLib/OpenBLAS/pull/2516.
isuruf
added a commit
to conda-forge/openblas-feedstock
that referenced
this pull request
Jul 24, 2020
This was referenced Jul 25, 2020
5 tasks
BrewTestBot
pushed a commit
to Homebrew/homebrew-core
that referenced
this pull request
Oct 15, 2020
* added env TARGET="HASWELL" to openblas build Openblas currently builds with DYNAMIC_ARCH=1, but if you don't specify the target CPU then the bottled version can only be run on any machine whose CPU is as least as new as the CPU which built openblas. By specifying TARGET="HASWELL" older machine should be able to use the bottled version of openblas without having to build from scratch. See Homebrew/discussions#54 for a discussion about how this error can cause libraries like numpy, which are built on openblas, to segfault. * Added OpenMathLib/OpenBLAS#2729 patch and fixed TARGET The currect version of openBLAS causes numpy to crash on HASWELL or later architectures when working with large matrices. The first change is to patch openBLAS to fix this error. According to the openBLAS developers this will be in 0.3.11 and so the patch can be removed then. I also adjusted the TARGET according to Jon Chang's suggestion so that it will build on the oldest possible architecture on both x86 and ARM. With DYNAMIC_ARCH=1 openBLAS should now work on all architectures. * Moved the patch comments to the correct spot In my last commit I accidentally had the patch comments at the wrong spot in the formula. This was because I moved things around to conform to the auditor. This is now fixed. Closes #62774. Signed-off-by: Sean Molenaar <1484494+SMillerDev@users.noreply.github.com> Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DYNAMIC_ARCH builds with a default TARGET of PRESCOTT (or actually anything older than Haswell) would suffer random
crashes in GEMM at larger matrix sizes when run on Haswell, Zen or SkylakeX due to the larger BUFFER_SIZE constant for these targets. Fixes #2728