From 1f527a93b996093e06ef7a8e94fb47ee7e690ce0 Mon Sep 17 00:00:00 2001 From: "Field G. Van Zee" Date: Mon, 20 Sep 2021 17:56:36 -0500 Subject: [PATCH] Re-enable and fix fb93d24. Details: - Re-enabled the changes made in fb93d24. - Defined BLIS_ENABLE_SYSTEM in bli_arch.c, bli_cpuid.c, and bli_env.c, all of which needed the definition (in addition to config_detect.c) in order for the configure-time hardware detection binary to be compiled properly. Thanks to Minh Quan Ho for helping identify these additional files as needing to be updated. - Added additional comments to all four source files, most notably to prompt the reader to remember to update all of the files when updating any of the files. Also made the cpp code in each of the files as consistent/similar as possible. - Refer to issues #532 and PR #546 for more history. --- build/detect/config/config_detect.c | 39 ++++++++++++++++++++++++----- frame/base/bli_arch.c | 18 +++++++++++++ frame/base/bli_cpuid.c | 21 +++++++++++++++- frame/base/bli_env.c | 20 +++++++++++++++ frame/include/bli_system.h | 8 +++--- frame/include/blis.h | 12 +++++++-- 6 files changed, 105 insertions(+), 13 deletions(-) diff --git a/build/detect/config/config_detect.c b/build/detect/config/config_detect.c index 2b59f78bf..5e29defe1 100644 --- a/build/detect/config/config_detect.c +++ b/build/detect/config/config_detect.c @@ -33,12 +33,39 @@ */ -#define BLIS_INLINE static -#define BLIS_EXPORT_BLIS -#include "bli_system.h" -#include "bli_type_defs.h" -#include "bli_arch.h" -#include "bli_cpuid.h" +// NOTE: This file will likely only ever get compiled as part of the BLIS +// configure script, and therefore BLIS_CONFIGURETIME_CPUID is guaranteed to +// be #defined. However, we preserve the cpp conditional for consistency with +// the other three files mentioned above. +#ifdef BLIS_CONFIGURETIME_CPUID + + // NOTE: If you need to make any changes to this cpp branch, it's probably + // the case that you also need to modify bli_arch.c, bli_cpuid.c, and + // bli_env.c. Don't forget to update these other files as needed! + + // The BLIS_ENABLE_SYSTEM macro must be defined so that the correct cpp + // branch in bli_system.h is processed. (This macro is normally defined in + // bli_config.h.) + #define BLIS_ENABLE_SYSTEM + + // Use C-style static inline functions for any static inline functions that + // happen to be defined by the headers below. (This macro is normally defined + // in bli_config_macro_defs.h.) + #define BLIS_INLINE static + + // Since we're not building a shared library, we can forgo the use of the + // BLIS_EXPORT_BLIS annotations by #defining them to be nothing. (This macro + // is normally defined in bli_config_macro_defs.h.) + #define BLIS_EXPORT_BLIS + + #include "bli_system.h" + #include "bli_type_defs.h" + #include "bli_arch.h" + #include "bli_cpuid.h" + //#include "bli_env.h" +#else + #include "blis.h" +#endif int main( int argc, char** argv ) { diff --git a/frame/base/bli_arch.c b/frame/base/bli_arch.c index 7fe69919f..e1061985e 100644 --- a/frame/base/bli_arch.c +++ b/frame/base/bli_arch.c @@ -34,8 +34,26 @@ */ #ifdef BLIS_CONFIGURETIME_CPUID + + // NOTE: If you need to make any changes to this cpp branch, it's probably + // the case that you also need to modify bli_arch.c, bli_cpuid.c, and + // bli_env.c. Don't forget to update these other files as needed! + + // The BLIS_ENABLE_SYSTEM macro must be defined so that the correct cpp + // branch in bli_system.h is processed. (This macro is normally defined in + // bli_config.h.) + #define BLIS_ENABLE_SYSTEM + + // Use C-style static inline functions for any static inline functions that + // happen to be defined by the headers below. (This macro is normally defined + // in bli_config_macro_defs.h.) #define BLIS_INLINE static + + // Since we're not building a shared library, we can forgo the use of the + // BLIS_EXPORT_BLIS annotations by #defining them to be nothing. (This macro + // is normally defined in bli_config_macro_defs.h.) #define BLIS_EXPORT_BLIS + #include "bli_system.h" #include "bli_type_defs.h" #include "bli_arch.h" diff --git a/frame/base/bli_cpuid.c b/frame/base/bli_cpuid.c index bc04f5586..5360d3917 100644 --- a/frame/base/bli_cpuid.c +++ b/frame/base/bli_cpuid.c @@ -47,12 +47,31 @@ #endif #ifdef BLIS_CONFIGURETIME_CPUID + + // NOTE: If you need to make any changes to this cpp branch, it's probably + // the case that you also need to modify bli_arch.c, bli_cpuid.c, and + // bli_env.c. Don't forget to update these other files as needed! + + // The BLIS_ENABLE_SYSTEM macro must be defined so that the correct cpp + // branch in bli_system.h is processed. (This macro is normally defined in + // bli_config.h.) + #define BLIS_ENABLE_SYSTEM + + // Use C-style static inline functions for any static inline functions that + // happen to be defined by the headers below. (This macro is normally defined + // in bli_config_macro_defs.h.) #define BLIS_INLINE static + + // Since we're not building a shared library, we can forgo the use of the + // BLIS_EXPORT_BLIS annotations by #defining them to be nothing. (This macro + // is normally defined in bli_config_macro_defs.h.) #define BLIS_EXPORT_BLIS + #include "bli_system.h" #include "bli_type_defs.h" - #include "bli_cpuid.h" #include "bli_arch.h" + #include "bli_cpuid.h" + //#include "bli_env.h" #else #include "blis.h" #endif diff --git a/frame/base/bli_env.c b/frame/base/bli_env.c index 23b8e059e..92aba6970 100644 --- a/frame/base/bli_env.c +++ b/frame/base/bli_env.c @@ -34,10 +34,30 @@ */ #ifdef BLIS_CONFIGURETIME_CPUID + + // NOTE: If you need to make any changes to this cpp branch, it's probably + // the case that you also need to modify bli_arch.c, bli_cpuid.c, and + // bli_env.c. Don't forget to update these other files as needed! + + // The BLIS_ENABLE_SYSTEM macro must be defined so that the correct cpp + // branch in bli_system.h is processed. (This macro is normally defined in + // bli_config.h.) + #define BLIS_ENABLE_SYSTEM + + // Use C-style static inline functions for any static inline functions that + // happen to be defined by the headers below. (This macro is normally defined + // in bli_config_macro_defs.h.) #define BLIS_INLINE static + + // Since we're not building a shared library, we can forgo the use of the + // BLIS_EXPORT_BLIS annotations by #defining them to be nothing. (This macro + // is normally defined in bli_config_macro_defs.h.) #define BLIS_EXPORT_BLIS + #include "bli_system.h" #include "bli_type_defs.h" + //#include "bli_arch.h" + //#include "bli_cpuid.h" #include "bli_env.h" #else #include "blis.h" diff --git a/frame/include/bli_system.h b/frame/include/bli_system.h index 2541018ac..79333017b 100644 --- a/frame/include/bli_system.h +++ b/frame/include/bli_system.h @@ -70,7 +70,7 @@ #endif // Determine the target operating system. -//#if defined(BLIS_ENABLE_SYSTEM) +#if defined(BLIS_ENABLE_SYSTEM) #if defined(_WIN32) || defined(__CYGWIN__) #define BLIS_OS_WINDOWS 1 #elif defined(__gnu_hurd__) @@ -94,9 +94,9 @@ #else #error "Cannot determine operating system" #endif -//#else // #if defined(BLIS_DISABLE_SYSTEM) -// #define BLIS_OS_NONE -//#endif +#else // #if defined(BLIS_DISABLE_SYSTEM) + #define BLIS_OS_NONE +#endif // A few changes that may be necessary in Windows environments. #if BLIS_OS_WINDOWS diff --git a/frame/include/blis.h b/frame/include/blis.h index a42c7cce8..b374e8539 100644 --- a/frame/include/blis.h +++ b/frame/include/blis.h @@ -48,6 +48,15 @@ extern "C" { // NOTE: PLEASE DON'T CHANGE THE ORDER IN WHICH HEADERS ARE INCLUDED UNLESS // YOU ARE SURE THAT IT DOESN'T BREAK INTER-HEADER MACRO DEPENDENCIES. +// -- configure definitions -- + +// NOTE: bli_config.h header must be included before any BLIS header. +// It is bootstrapped by ./configure and does not depend on later +// headers. Moreover, these configuration variables are necessary to change +// some default behaviors (e.g. disable OS-detection in bli_system.h in case +// of --disable-system). +#include "bli_config.h" + // -- System and language-related headers -- // NOTE: bli_system.h header must be included before bli_config_macro_defs.h. @@ -55,9 +64,8 @@ extern "C" { #include "bli_lang_defs.h" -// -- configure definitions -- +// -- configure default definitions -- -#include "bli_config.h" #include "bli_config_macro_defs.h"