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.

Sign up for GitHub

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

Jump to bottom

Update FindSystemBLAS cmake #509

Merged
merged 3 commits into from
Apr 9, 2024
Merged

Update FindSystemBLAS cmake #509

merged 3 commits into from
Apr 9, 2024

Conversation

s-Nick
Copy link
Collaborator

@s-Nick s-Nick commented Apr 3, 2024

This PR updates how BLAS library is found and set for testing.
With this patch cmake provides better messages to users if openBLAS is not found with some warnings and messages.

@s-Nick
Update FindSystemBLAS cmake
2eece9f 
Update how BLAS library is found and set. Provide better message to
users if openBLAS is not found. Clearly states that openBLAS is the
library used for testing.

Signed-off-by: nscipione <[email protected]>
@s-Nick s-Nick requested review from hjabird, Rbiessy and pgorlani April 3, 2024 16:05
@s-Nick s-Nick mentioned this pull request Apr 4, 2024
Closed
Rbiessy
Rbiessy reviewed Apr 4, 2024
View reviewed changes
Copy link
Collaborator

@Rbiessy Rbiessy left a comment

Choose a reason for hiding this comment

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

Some of my comments are not stricly needed because from what I understand portBLAS only uses the blas::blas and SystemBLAS_* are ignored outside of this file.
I've made some suggestions to make a bit more sense in this file.

cmake/Modules/FindSystemBLAS.cmake Outdated Show resolved Hide resolved
cmake/Modules/FindSystemBLAS.cmake Outdated Show resolved Hide resolved
cmake/Modules/FindSystemBLAS.cmake Outdated Show resolved Hide resolved
cmake/Modules/FindSystemBLAS.cmake Outdated Show resolved Hide resolved
@s-Nick
Update message in cmake when a BLAS Library is set & minor fixes.
268ff13 
The configuration used to print a generic type of library, change that
to message to print library location.
Plus minor adjustements.

Signed-off-by: nscipione <[email protected]>
hjabird
hjabird reviewed Apr 9, 2024
View reviewed changes
Copy link
Contributor

@hjabird hjabird left a comment

Choose a reason for hiding this comment

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

What happens if find_package(BLAS QUIET) fails? It looks like this will not produce an error here if a BLAS library can't be found.

@s-Nick
Copy link
Collaborator Author

s-Nick commented Apr 9, 2024

What happens if find_package(BLAS QUIET) fails? It looks like this will not produce an error here if a BLAS library can't be found.

It will fail to config because System_BLAS it is flag as REQUIRED here for tests and each benchmark looks for System_BLAS in the same way

@s-Nick s-Nick merged commit 5783414 into codeplaysoftware:master Apr 9, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rbiessy Rbiessy Rbiessy approved these changes

@hjabird hjabird hjabird approved these changes

@pgorlani pgorlani Awaiting requested review from pgorlani

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@s-Nick @hjabird @Rbiessy