CBLAS: unify the XERBLA name/INFO logic between the library and the test harness - #1340
Merged
Merged
Conversation
Replace the repeated
void
#ifdef HAS_ATTRIBUTE_WEAK_SUPPORT
__attribute__((weak))
#endif
preamble on the cblas_xerbla(), cblas_xerbla_64() and F77_xerbla_base()
declarations with a single CBLAS_WEAK_SYMBOL macro.
Define it in cblas.h ahead of the cblas_64.h include: cblas_64.h declares
cblas_xerbla_64() with the macro, and its own include of cblas.h is a
no-op while cblas.h is still inside its own include guard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c_xerbla.c needs the Fortran integer and character types to declare F77_xerbla(), but the testing sources deliberately do not include cblas_f77.h: that header maps every F77_* name to the real BLAS symbol while cblas_test.h maps them to the Fortran test wrappers, and 141 of those names collide. Copy the two fallbacks into cblas_test.h instead, alongside the BLAS_FORTRAN_STRLEN_END and FORTRAN_STRLEN definitions it already duplicates for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cblas_xerbla_internal.h collects the routine-name construction and the row-major INFO remapping that the library and the test harness each open-code today, so the two copies can no longer drift apart. Relative to those copies the helper also trims the blank padding Fortran supplies, drops the _64 suffix that BUILD_INDEX64_EXT_API rewrites into the XERBLA name literals, matches operation names exactly rather than by substring, and derives its buffer size from a named maximum so that long names are clamped instead of silently truncated. It has no user yet; the xerbla sources are switched over next. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Switch cblas_xerbla(), F77_xerbla_base() and the test harness over to
cblas_xerbla_internal.h. Two diagnostic bugs go away with the duplicated
code:
- the row-major remap keyed off strstr(rout, "gemm"), which also
matches gemmtr and so wrongly swapped its arguments 4 and 5;
- the six-character name buffer truncated cblas_sgemmtr and
cblas_sskewsyr2k in the library, while the harness used an
eleven-character buffer and did not, so the two disagreed about the
same routine.
The Fortran entry points now take FCHAR and read the argument number
through F77_INT consistently, honour the hidden string length instead of
assuming six characters, and carry doxygen comments.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Describes the style the xerbla sources are written in: three-space indent, Allman braces, 80 columns, and the return type on its own line for definitions. Every option clang-format knows about is listed, with the ones inherited from the LLVM base style commented out, so that the uncommented lines are exactly what this style changes. The file applies to all of CBLAS, but the older sources here do not follow it, so format only the lines you touch, e.g. with git clang-format. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
langou
approved these changes
Jul 30, 2026
This file contains hidden or 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
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.
Motivation
CBLAS carries three near-identical copies of the same two pieces of logic — building the
cblas_-prefixed routine name, and remapping the Fortran argument number to the CBLAS argument position for row-major calls:CBLAS/src/cblas_xerbla.c,CBLAS/src/xerbla.candCBLAS/testing/c_xerbla.c. They have drifted apart, and thestrstr()-based matching they use is wrong for some routine names.This PR extracts the logic into a single header,
CBLAS/include/cblas_xerbla_internal.h, and makes all three sites use it. No public API or ABI change.Bugs fixed
gemmtrgot thegemmremapping.strstr(rout, "gemm")also matchescblas_?gemmtr, so row-majorgemmtrerrors swapped arguments 4 and 5 and reportedthe wrong parameter number. Matching is now exact (with the test harness's existing
gemmtrspecial case folded in).Routine names longer than six characters were truncated.
src/xerbla.ccopied afixed six characters into a fixed-size buffer, so e.g.
cblas_dskewsymmwas reportedas
cblas_dskewsy. The name is now trimmed from the real Fortran string length into acorrectly sized buffer (with a bit of headroom for potential new routines).
Extended (64-bit) API builds reported the wrong name.
Generate64BitSuffixedSourcerewrites the XERBLA literals, so the Fortran layer passes
DGEMM_64. The suffix is nowstripped when rebuilding the name and re-applied when reporting, so a 64-bit build names
cblas_dgemm_64()and the test harness's name comparison matches.INFOwas read as the wrong integer type. Bothxerbla.cfiles dereferenced theFortran-supplied
void *infoasint/CBLAS_INT; it is now read asF77_INT.skewsymmwas only handled by substring accident (strstr(rout, "symm")); it is nowmatched explicitly.
Unbounded write into a fixed-size buffer in the test harness.
F77_xerbla()copiedthe name with
for (i = 0; i < srname_len; i++) rout[i+6] = ..., wheresrname_lenisthe hidden Fortran string length, so nothing bounds the write by the size of
rout.GCC 16 on macOS flags this:
cblas_xerbla_make_rout()takes the destination size and bounds every write by it, sothe warning goes away.
Other changes
CBLAS_WEAK_SYMBOLreplaces the repeated#ifdef HAS_ATTRIBUTE_WEAK_SUPPORT/__attribute__((weak))blocks incblas.h,cblas_64.handcblas_f77.h.cblas_test.hnow providesF77_INTandFCHAR, so the harness no longer rolls its ownF77_Integer/F77_Charvariants (which were invalid).(int)casts onCBLAS_INTvalues in the test printfs are replaced byCBLAS_IFMT,which was already the convention elsewhere; same for
PRId64incblas_example1_64.c.CBLAS/.clang-formatmatching the existing CBLAS style, and.gitignoreentries forshared libraries and debug info.
I tried to keep the .clang-format stye as close to the existing CBLAS formatting. This could enable us to actually format the entire CBLAS codebase without too many changes. Could be discussed in the future.