Skip to content

Conversation

@amontoison
Copy link
Contributor

@amontoison amontoison commented Sep 16, 2022

We need this modification if we want to compile ReLAPACK and also not override the .o and symbols generated by LAPACK (INCLUDE_ALL=0 in relapack/config.h).

@martin-frbg
Copy link
Collaborator

Ahem, wouldn't that break the default build by enforcing relapack prefixes even when not desired ?

@amontoison
Copy link
Contributor Author

amontoison commented Oct 15, 2022

You're right @martin-frbg, we should only add a prefix if INCLUDE_ALL=0 in relapack/config.h.
If we have two .o with different names but the same exported symbols, the library will use the routines of the first .o file added in the library.

Do you have an idea how can check the value of INCLUDE_ALL from relapack/Makefile?
We could also just define RELAPACK_PREFIX in the same spirit of SUFFIX in the relapack/Makefile for more flexibility.

@martin-frbg
Copy link
Collaborator

It would probably make sense to have tighter integration with the OpenBLAS makefiles, i.e. moving the INCLUDE_ALL from config.h to a RELAPACK_INCLUDEALL in Makefile.rule (so that one does not have to hack around in the ReLAPACK internals) and then having the ReLAPACK makefile add -DINCLUDE_ALL=$RELAPACK_INCLUDEALL to the %.o: %.c build rule.
(Plus the equivalent for the cmake build system)

@amontoison
Copy link
Contributor Author

I saw that you opened #3796 and #3802, should I close this PR?

@martin-frbg
Copy link
Collaborator

Sorry, I did not intend to deny you recognition but the changes got a bit out of hand, what with the preexisting bugs in the cmake files and the need to integrate with the hackish GEMMT I already had lying around

@amontoison
Copy link
Contributor Author

It's perfectly fine @martin-frbg! I don't know well CMake.
I also didn't know if I should update the symbols or RELAPACK in OpenBLAS/exports folder.

@amontoison amontoison closed this Apr 6, 2023
@amontoison amontoison deleted the relapack branch April 6, 2023 22:57
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