lapack 3.12.1 compatibility with blis 1.1

[cdluminate]

lapack 3.12.1 has introduced a couple of new symbols such as sgemmtr_. But it seems missing from BLIS 1.1. As described in the pull request, when we use update-alternatives to switch to BLIS, the program will fail to find that symbol, see example here. Some relevant test failures from the Debian side also points to the same issue: https://qa.debian.org/excuses.php?experimental=1&package=lapack .

Any suggestion on this? Is copying some code from lapack and glue them into BLIS a reasonable temporary workaround?

[devinamatthews]

?gemmtr is just bli_gemmt so it should be easy to add an interface adapter. If you are willing to work on a PR then I suggest taking an existing level-3 BLAS interface (gemm, syrk, etc.) and adapting it.

[cdluminate]

Thanks for the pointer. I'll take a look

[cdluminate]

I noted that there is already frame/compat/extra/bla_gemmt.h, and a symbol sgemmt_ in the compat shared object. However, the upstream does not provide that sgemmt_. Should we instead rename the gemmt into gemmtr in the compat code?

$ readelf -sW /usr/lib/x86_64-linux-gnu/blis-openmp/libblas.so.3 | grep sgemm
    75: 0000000000805d50   831 FUNC    GLOBAL DEFAULT   12 cblas_sgemmt
   132: 0000000000807b40  2485 FUNC    GLOBAL DEFAULT   12 sgemm_batch_
   136: 00000000007fc470   742 FUNC    GLOBAL DEFAULT   12 cblas_sgemm
   234: 0000000000805950  1013 FUNC    GLOBAL DEFAULT   12 cblas_sgemm_batch
   249: 00000000007e2a00  2367 FUNC    GLOBAL DEFAULT   12 sgemm_
   380: 000000000080a290  1669 FUNC    GLOBAL DEFAULT   12 sgemmt_

$ readelf -sW tmp/usr/lib/x86_64-linux-gnu/blas/libblas.so.3.12.1 | grep sgemm
    24: 0000000000047d80  2941 FUNC    GLOBAL DEFAULT   12 sgemmtr_
    91: 0000000000010930   735 FUNC    GLOBAL DEFAULT   12 cblas_sgemm
   195: 0000000000047400  2417 FUNC    GLOBAL DEFAULT   12 sgemm_
   275: 0000000000010c10   879 FUNC    GLOBAL DEFAULT   12 cblas_sgemmtr

[cdluminate]

This seems to be the minimal change to rename the sgemmt_ symbol into sgemmtr_, but I guess it will leave some mess, such as inconsistent header name, etc.

diff --git a/frame/compat/cblas/src/cblas_f77.h b/frame/compat/cblas/src/cblas_f77.h
index acb354aa..31e9124d 100644
--- a/frame/compat/cblas/src/cblas_f77.h
+++ b/frame/compat/cblas/src/cblas_f77.h
@@ -205,10 +205,10 @@
 #define F77_caxpby  caxpby_
 #define F77_zaxpby  zaxpby_
 
-#define F77_sgemmt  sgemmt_
-#define F77_dgemmt  dgemmt_
-#define F77_cgemmt  cgemmt_
-#define F77_zgemmt  zgemmt_
+#define F77_sgemmt  sgemmtr_
+#define F77_dgemmt  dgemmtr_
+#define F77_cgemmt  cgemmtr_
+#define F77_zgemmt  zgemmtr_
 
 #define F77_sgemm_batch  sgemm_batch_
 #define F77_dgemm_batch  dgemm_batch_
diff --git a/frame/compat/extra/bla_gemmt.c b/frame/compat/extra/bla_gemmt.c
index 266663df..627a3784 100644
--- a/frame/compat/extra/bla_gemmt.c
+++ b/frame/compat/extra/bla_gemmt.c
@@ -148,7 +148,7 @@ void PASTEF77(ch,blasname) \
 	bli_init_auto(); \
 \
 	/* Perform BLAS parameter checking. */ \
-	PASTEBLACHK(blasname) \
+	PASTEBLACHK(blisname) \
 	( \
 	  MKSTR(ch), \
 	  MKSTR(blasname), \
@@ -226,6 +226,6 @@ void PASTEF77(ch,blasname) \
 #endif
 
 #ifdef BLIS_ENABLE_BLAS
-INSERT_GENTFUNC_BLAS( gemmt, gemmt )
+INSERT_GENTFUNC_BLAS( gemmtr, gemmt )
 #endif
 
diff --git a/frame/compat/extra/bla_gemmt.h b/frame/compat/extra/bla_gemmt.h
index 3bef5a89..0a9de986 100644
--- a/frame/compat/extra/bla_gemmt.h
+++ b/frame/compat/extra/bla_gemmt.h
@@ -55,6 +55,6 @@ BLIS_EXPORT_BLAS void PASTEF77(ch,blasname) \
      );
 
 #ifdef BLIS_ENABLE_BLAS
-INSERT_GENTPROT_BLAS( gemmt )
+INSERT_GENTPROT_BLAS( gemmtr )
 #endif

[devinamatthews]

Some packages use sgemmt as an extension so that shouldn't be renamed. But, you could a) add a copy of the wrapper function with the different name, or b) create just a declaration with the new name and __attribute__((alias("..."))).

[cdluminate]

I tried the following change on master:

diff --git a/frame/compat/bli_blas.h b/frame/compat/bli_blas.h
index fca75e0f..e37a7e42 100644
--- a/frame/compat/bli_blas.h
+++ b/frame/compat/bli_blas.h
@@ -71,6 +71,7 @@
   // If BLIS_ENABLE_BLAS_DEFS is defined, then we should #include the BLAS
   // prototypes.
   #include "bli_blas_defs.h"
+BLIS_EXPORT_BLAS void sgemmtr_ __attribute__((alias("sgemmt_")));
 #else
   // Even if BLAS prototypes are not to be #included into blis.h, we still
   // need to #include the prototypes when compiling BLIS.

And gcc says sgemmtr_ is already defined:

make -j1                                                                                                                                                                15:24
Compiling obj/generic/config/generic/bli_cntx_init_generic.o ('generic' CFLAGS for config code)
In file included from config/generic/bli_cntx_init_generic.c:35:
./frame/compat//bli_blas.h:75:23: error: ‘sgemmtr_’ defined both normally and as ‘alias’ attribute
   75 | #else
      |                       ^       
compilation terminated due to -Wfatal-errors.
make: *** [Makefile:736: obj/generic/config/generic/bli_cntx_init_generic.o] Error 1

I'm confused by the preprocessor macros, and have not found where the sgemmtr_ was defined but not exported.

[devinamatthews]

The prototype in the .h file should be "normal" (exactly like sgemmt but with the name changed). Then, in the .c file put the version with attribute. An alias declaration can appear only once in the library and must be in the same translation unit as the referenced symbol.

[cdluminate]

The following seems simple and working (I'm still trying to figure out how to make alias attribute work though).

diff --git a/frame/compat/check/bla_gemmt_check.h b/frame/compat/check/bla_gemmt_check.h
index 0a754248..90c54385 100644
--- a/frame/compat/check/bla_gemmt_check.h
+++ b/frame/compat/check/bla_gemmt_check.h
@@ -89,4 +89,7 @@
    } \
 }
 
+#define bla_gemmtr_check( dt_str, op_str, uploc, transa, transb, m, k, lda, ldb, ldc ) \
+   bla_gemmt_check( dt_str, op_str, uploc, transa, transb, m, k, lda, ldb, ldc )
+
 #endif
diff --git a/frame/compat/extra/bla_gemmt.c b/frame/compat/extra/bla_gemmt.c
index 266663df..a38eb55b 100644
--- a/frame/compat/extra/bla_gemmt.c
+++ b/frame/compat/extra/bla_gemmt.c
@@ -227,5 +227,6 @@ void PASTEF77(ch,blasname) \
 
 #ifdef BLIS_ENABLE_BLAS
 INSERT_GENTFUNC_BLAS( gemmt, gemmt )
+INSERT_GENTFUNC_BLAS( gemmtr , gemmt )
 #endif
 
diff --git a/frame/compat/extra/bla_gemmt.h b/frame/compat/extra/bla_gemmt.h
index 3bef5a89..9dda6ff3 100644
--- a/frame/compat/extra/bla_gemmt.h
+++ b/frame/compat/extra/bla_gemmt.h
@@ -56,5 +56,6 @@ BLIS_EXPORT_BLAS void PASTEF77(ch,blasname) \
 
 #ifdef BLIS_ENABLE_BLAS
 INSERT_GENTPROT_BLAS( gemmt )
+INSERT_GENTPROT_BLAS( gemmtr )
 #endif
 

I can see both sgemmt_ and sgemmtr_ from readelf, but they points at different location, meaning the code is duplicated as expected. I'll further try to see how to make that attribute alias work.

[cdluminate]

The last remaining bit is to correctly make preprocessor concat r to blasname, instead of hardcoding like the follows:

diff --git a/frame/compat/check/bla_gemmt_check.h b/frame/compat/check/bla_gemmt_check.h
index 0a754248..90c54385 100644
--- a/frame/compat/check/bla_gemmt_check.h
+++ b/frame/compat/check/bla_gemmt_check.h
@@ -89,4 +89,7 @@
        } \
 }
 
+#define bla_gemmtr_check( dt_str, op_str, uploc, transa, transb, m, k, lda, ldb, ldc ) \
+       bla_gemmt_check( dt_str, op_str, uploc, transa, transb, m, k, lda, ldb, ldc )
+
 #endif
diff --git a/frame/compat/extra/bla_gemmt.c b/frame/compat/extra/bla_gemmt.c
index 266663df..6e105126 100644
--- a/frame/compat/extra/bla_gemmt.c
+++ b/frame/compat/extra/bla_gemmt.c
@@ -39,6 +39,8 @@
 //
 // Define BLAS-to-BLIS interfaces.
 //
+#define STRINGIFY( name ) #name
+#define EXPAND_AND_STRINGIFY( name ) STRINGIFY( name )
 
 #ifdef BLIS_BLAS3_CALLS_TAPI
 
@@ -118,7 +120,9 @@ void PASTEF77(ch,blasname) \
 \
        /* Finalize BLIS. */ \
        bli_finalize_auto(); \
-}
+}; \
+void PASTEF77 (ch, gemmtr)() \
+       __attribute__ ((alias(EXPAND_AND_STRINGIFY(PASTEF77(ch,blasname)))));
 
 #else
 
@@ -221,7 +225,9 @@ void PASTEF77(ch,blasname) \
 \
        /* Finalize BLIS. */ \
        bli_finalize_auto(); \
-}
+}; \
+void PASTEF77 (ch, gemmtr)() \
+       __attribute__ ((alias(EXPAND_AND_STRINGIFY(PASTEF77(ch,blasname)))));
 
 #endif
 
diff --git a/frame/compat/extra/bla_gemmt.h b/frame/compat/extra/bla_gemmt.h
index 3bef5a89..9dda6ff3 100644
--- a/frame/compat/extra/bla_gemmt.h
+++ b/frame/compat/extra/bla_gemmt.h
@@ -56,5 +56,6 @@ BLIS_EXPORT_BLAS void PASTEF77(ch,blasname) \
 
 #ifdef BLIS_ENABLE_BLAS
 INSERT_GENTPROT_BLAS( gemmt )
+INSERT_GENTPROT_BLAS( gemmtr )
 #endif
 

Now both sgemmt_ and sgemmtr_ point at the save address.

[devinamatthews]

I think bla_gemmtr_check can be removed entirely, right? If you would please put this in a PR it should be a pretty quick merge.

[devinamatthews]

Should the same be done for CBLAS?

[cdluminate]

The PR is filed at #849 . I'll look into cblas later when I'm available.