Fixed compile errors with BLIS_DISABLE_BLAS_DEFS.
#730
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
Details: - This commit fixes a compile-time error related to the type definition (prototype) of dsdot_() when BLIS_DISABLE_BLAS_DEFS is defined by the application (or the configuration), which is actually a symptom of a larger design issue when disabling BLAS prototypes. The macro was intended to allow applications to bring their own BLAS prototypes and supress the inclusion of duplicate (or possibly conflicting) prototypes within blis.h. However, prototypes are still needed during compilation even if they are ultimately omitted from blis.h. The problem is that almost every source file in BLIS--including the BLAS compatibility layer--only includes one header (blis.h), and if we were to #include a new header in the BLAS source files (to isolate only the BLAS prototypes), we would also have to make the build system aware of the location of those headers. Thanks to Edward Smyth of AMD for reporting this issue. - The solution I settled upon was to remove all cpp guards from all BLAS headers (by changing them to #if 1, for each search-and-replace anchoring in the future if we ever need to re-insert guards) and modifying bli_blas.h so that the BLAS prototypes are #included if either (a) BLIS_ENABLE_BLAS_DEFS is defined, or (b) BLIS_ENABLE_BLAS_DEFS is *not* defined but BLIS_IS_BUILDING_LIBRARY *is* defined. (Thanks to Devin Matthews for steering me away from an inferior solution.) - This commit also spins off the actual BLAS prototypes/definitions to a separate file, bli_blas_defs.h. - CREDITS file update.
ct-clmsn
pushed a commit
to ct-clmsn/blis
that referenced
this pull request
Jul 29, 2023
* Fixed compile errors with BLIS_DISABLE_BLAS_DEFS. Details: - This commit fixes a compile-time error related to the type definition (prototype) of dsdot_() when BLIS_DISABLE_BLAS_DEFS is defined by the application (or the configuration), which is actually a symptom of a larger design issue when disabling BLAS prototypes. The macro was intended to allow applications to bring their own BLAS prototypes and suppress the inclusion of duplicate (or possibly conflicting) prototypes within blis.h. However, prototypes are still needed during compilation even if they are ultimately omitted from blis.h. The problem is that almost every source file in BLIS--including the BLAS compatibility layer--only includes one header (blis.h), and if we were to #include a new header in the BLAS source files (to isolate only the BLAS prototypes), we would also have to make the build system aware of the location of those headers. Thanks to Edward Smyth of AMD for reporting this issue. - The solution I settled upon was to remove all cpp guards from all BLAS headers (by changing them to #if 1, for easy search-and-replace anchoring in the future if we ever need to re-insert guards) and modifying bli_blas.h so that the BLAS prototypes are #included if either (a) BLIS_ENABLE_BLAS_DEFS is defined, or (b) BLIS_ENABLE_BLAS_DEFS is *not* defined but BLIS_IS_BUILDING_LIBRARY *is* defined. (Thanks to Devin Matthews for steering me away from an inferior solution.) - This commit also spins off the actual BLAS prototypes/definitions to a separate file, bli_blas_defs.h. - CREDITS file update.
fgvanzee
added a commit
that referenced
this pull request
May 29, 2024
Details: - This commit fixes a compile-time error related to the type definition (prototype) of dsdot_() when BLIS_DISABLE_BLAS_DEFS is defined by the application (or the configuration), which is actually a symptom of a larger design issue when disabling BLAS prototypes. The macro was intended to allow applications to bring their own BLAS prototypes and suppress the inclusion of duplicate (or possibly conflicting) prototypes within blis.h. However, prototypes are still needed during compilation even if they are ultimately omitted from blis.h. The problem is that almost every source file in BLIS--including the BLAS compatibility layer--only includes one header (blis.h), and if we were to #include a new header in the BLAS source files (to isolate only the BLAS prototypes), we would also have to make the build system aware of the location of those headers. Thanks to Edward Smyth of AMD for reporting this issue. - The solution I settled upon was to remove all cpp guards from all BLAS headers (by changing them to #if 1, for easy search-and-replace anchoring in the future if we ever need to re-insert guards) and modifying bli_blas.h so that the BLAS prototypes are #included if either (a) BLIS_ENABLE_BLAS_DEFS is defined, or (b) BLIS_ENABLE_BLAS_DEFS is *not* defined but BLIS_IS_BUILDING_LIBRARY *is* defined. (Thanks to Devin Matthews for steering me away from an inferior solution.) - This commit also spins off the actual BLAS prototypes/definitions to a separate file, bli_blas_defs.h. - CREDITS file update. - (cherry picked from commit 04090df)
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.
Details:
dsdot_()whenBLIS_DISABLE_BLAS_DEFSis defined by the application (or the configuration), which is actually a symptom of a larger design issue when disabling BLAS prototypes. The macro was intended to allow applications to bring their own BLAS prototypes and suppress the inclusion of duplicate (or possibly conflicting) prototypes withinblis.h. However, prototypes are still needed during compilation even if they are ultimately omitted fromblis.h. The problem is that almost every source file in BLIS--including the BLAS compatibility layer--only includes one header (blis.h), and if we were to#includea new header in the BLAS source files (to isolate only the BLAS prototypes), we would also have to make the build system aware of the location of those headers. Thanks to Edward Smyth of AMD for reporting this issue.#if 1, for each search-and-replace anchoring in the future if we ever need to re-insert guards) and modifyingbli_blas.hso that the BLAS prototypes are#included if either (a)BLIS_ENABLE_BLAS_DEFSis defined, or (b)BLIS_ENABLE_BLAS_DEFSis not defined butBLIS_IS_BUILDING_LIBRARYis defined. (Thanks to Devin Matthews for steering me away from an inferior solution.)bli_blas_defs.h.CREDITSfile update.