Skip to content

Commit 0f34462

Browse files
committed
PR agent fix: libxsmm install/export, ld guard, TA_LIBXSMM=ON test gate
- Install libxsmm.a+headers into TA's prefix; split TiledArray_LIBXSMM into BUILD/INSTALL interfaces so the exported config has no build-tree leak. - Guard 64->32-bit narrowing of lda/ldb/ldc in libxsmm_gemm_le64. - Make the libxsmm sub-make parallelism configurable (LIBXSMM_BUILD_NJOBS). - Add a TA_LIBXSMM=1 CTest gate + a direct scale_libxsmm_dgemm numerical test.
1 parent d68c80f commit 0f34462

4 files changed

Lines changed: 100 additions & 5 deletions

File tree

external/libxsmm.cmake

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ else ()
7070
set(_libxsmm_cxx ${CMAKE_CXX_COMPILER})
7171
endif ()
7272

73+
# parallelism for the libxsmm sub-make (overridable; default mirrors the
74+
# project's typical build budget rather than hardcoding into the command)
75+
set(LIBXSMM_BUILD_NJOBS 6 CACHE STRING "Parallel jobs for the libxsmm sub-make")
76+
7377
# libxsmm Make knobs:
7478
# STATIC=1 build libxsmm.a (we link the archive into tiledarray)
7579
# FORTRAN=0 skip the Fortran interface (no gfortran needed)
@@ -93,7 +97,7 @@ else ()
9397
BUILD_IN_SOURCE 1
9498
CONFIGURE_COMMAND ""
9599
#--Build step----------------- build + install in one make invocation
96-
BUILD_COMMAND make -j6 STATIC=1 FORTRAN=0 BLAS=0
100+
BUILD_COMMAND make -j${LIBXSMM_BUILD_NJOBS} STATIC=1 FORTRAN=0 BLAS=0
97101
CC=${_libxsmm_cc} CXX=${_libxsmm_cxx} AR=${CMAKE_AR}
98102
PREFIX=${_LIBXSMM_INSTALL_DIR} install
99103
BUILD_BYPRODUCTS ${LIBXSMM_BUILD_BYPRODUCTS}
@@ -112,16 +116,37 @@ else ()
112116

113117
endif (_LIBXSMM_INSTALL_DIR)
114118

119+
# Fold libxsmm's static lib + headers into TiledArray's OWN install prefix, so
120+
# the exported TiledArray config is self-contained and does not reference TA's
121+
# build tree (which a downstream find_package(TiledArray) consumer like MPQC
122+
# would otherwise link against -- and which breaks once the build tree is
123+
# wiped/relocated). Done for both the fetched and the prebuilt cases so the TA
124+
# install is identical either way.
125+
install(FILES "${_LIBXSMM_INSTALL_DIR}/lib/libxsmm.a"
126+
DESTINATION "${TILEDARRAY_INSTALL_LIBDIR}" COMPONENT tiledarray)
127+
install(DIRECTORY "${_LIBXSMM_INSTALL_DIR}/include/"
128+
DESTINATION "${TILEDARRAY_INSTALL_INCLUDEDIR}" COMPONENT tiledarray)
129+
115130
# Synthetic target carrying the include dir, the static archive, and the gating
116-
# define. PUBLIC propagation (via _TILEDARRAY_DEPENDENCIES) makes
117-
# TILEDARRAY_HAS_LIBXSMM + the include path visible to consumers (e.g. MPQC).
131+
# define. PUBLIC propagation (via _TILEDARRAY_DEPENDENCIES) makes the libxsmm.a
132+
# link requirement reach consumers (libtiledarray is a static archive, so its
133+
# undefined libxsmm symbols are resolved at the consumer's final link), plus
134+
# TILEDARRAY_HAS_LIBXSMM + the include path. The link/include paths are split
135+
# into BUILD_INTERFACE (TA's build tree) and INSTALL_INTERFACE (the installed
136+
# copy above), so the exported config never references the build tree. The
137+
# include INSTALL_INTERFACE is relative (CMake prepends the import prefix); the
138+
# link library INSTALL_INTERFACE must be an absolute path to the installed
139+
# archive -- CMake does NOT prepend the import prefix to INTERFACE_LINK_LIBRARIES
140+
# entries, so a relative path there would be resolved against the consumer's cwd
141+
# and fail to link. Absolute CMAKE_INSTALL_PREFIX is leak-free (the install tree
142+
# is the stable final location, unlike the build tree).
118143
add_library(TiledArray_LIBXSMM INTERFACE)
119144
set_target_properties(TiledArray_LIBXSMM
120145
PROPERTIES
121146
INTERFACE_INCLUDE_DIRECTORIES
122-
"$<BUILD_INTERFACE:${_LIBXSMM_INSTALL_DIR}/include>;$<INSTALL_INTERFACE:include>"
147+
"$<BUILD_INTERFACE:${_LIBXSMM_INSTALL_DIR}/include>;$<INSTALL_INTERFACE:${TILEDARRAY_INSTALL_INCLUDEDIR}>"
123148
INTERFACE_LINK_LIBRARIES
124-
"${_LIBXSMM_INSTALL_DIR}/lib/libxsmm.a;${CMAKE_DL_LIBS}"
149+
"$<BUILD_INTERFACE:${_LIBXSMM_INSTALL_DIR}/lib/libxsmm.a>;$<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/${TILEDARRAY_INSTALL_LIBDIR}/libxsmm.a>;${CMAKE_DL_LIBS}"
125150
INTERFACE_COMPILE_DEFINITIONS
126151
"TILEDARRAY_HAS_LIBXSMM"
127152
)

src/TiledArray/math/libxsmm_gemm.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifdef TILEDARRAY_HAS_LIBXSMM
1010
#include <cstdlib>
1111
#include <cstring>
12+
#include <limits>
1213
#include <mutex>
1314
#include <libxsmm.h>
1415
#endif
@@ -51,6 +52,11 @@ bool libxsmm_gemm_le64(bool trans_a, bool trans_b, std::int64_t m,
5152
// libxsmm SMM has no alpha and only beta in {0,1} (LIBXSMM_GEMM_NO_BYPASS).
5253
if (alpha != 1.0) return false;
5354
if (beta != 0.0 && beta != 1.0) return false;
55+
// libxsmm_blasint is 32-bit; refuse leading dims that would narrow silently.
56+
// (M,N,K are already <=64; lda/ldb/ldc are strides and unbounded in general.)
57+
constexpr std::int64_t bi_max =
58+
static_cast<std::int64_t>(std::numeric_limits<libxsmm_blasint>::max());
59+
if (lda > bi_max || ldb > bi_max || ldc > bi_max) return false;
5460

5561
static std::once_flag init_flag;
5662
std::call_once(init_flag, [] {

tests/CMakeLists.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,18 @@ else()
192192
)
193193
endif()
194194

195+
# When libxsmm is enabled, also run the strided-GEMM suites with the libxsmm
196+
# fast path forced ON (TA_LIBXSMM=1), so its numerics are gated against the
197+
# in-suite references (the default-OFF run above does not exercise libxsmm).
198+
if (TA_LIBXSMM)
199+
add_test(NAME tiledarray/unit/run-libxsmm
200+
COMMAND $<TARGET_FILE:${executable}> --log_level=unit_scope
201+
--run_test=arena_strided_dgemm_suite:arena_einsum_unit_suite)
202+
set_tests_properties(tiledarray/unit/run-libxsmm
203+
PROPERTIES FIXTURES_REQUIRED TA_UNIT_TESTS_EXEC
204+
ENVIRONMENT "${TA_UNIT_TESTS_ENVIRONMENT};TA_LIBXSMM=1")
205+
endif()
206+
195207
if (NOT TARGET test-cases-tiledarray)
196208
add_custom_target_subproject(tiledarray test-cases)
197209
endif()

tests/arena_strided_dgemm.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2109,4 +2109,56 @@ BOOST_AUTO_TEST_CASE(ce_ce_seg_killswitch_matches_left) {
21092109
}
21102110
}
21112111

2112+
// Directly validate the scale-path libxsmm entry point
2113+
// (TiledArray::detail::scale_libxsmm_dgemm) for BOTH scale regimes' transpose
2114+
// configs -- tot_x_t maps to (trans_a=true, trans_b=false) and t_x_tot maps to
2115+
// (trans_a=false, trans_b=false). The ce+e / ce+ce arena tests above already
2116+
// exercise libxsmm_gemm_le64 for these same transpose patterns, but the scale
2117+
// wrapper itself (tensor.cpp) is otherwise only reachable through Tensor::gemm;
2118+
// this gates its numerics standalone. When libxsmm is not active (built without
2119+
// TILEDARRAY_HAS_LIBXSMM, or TA_LIBXSMM=0) the wrapper returns false and writes
2120+
// nothing, so the numeric check is keyed on the returned flag.
2121+
BOOST_AUTO_TEST_CASE(scale_path_libxsmm_matches_reference) {
2122+
// Row-major C(m x n) = beta*C + op_a(A)(m x k) . op_b(B)(k x n), alpha=1,
2123+
// matching scale_libxsmm_dgemm / blas::gemm semantics.
2124+
auto ref_gemm = [](bool ta, bool tb, long m, long n, long k,
2125+
const std::vector<double>& A, long lda,
2126+
const std::vector<double>& B, long ldb, double beta,
2127+
std::vector<double>& C, long ldc) {
2128+
for (long i = 0; i < m; ++i)
2129+
for (long j = 0; j < n; ++j) {
2130+
double acc = 0.0;
2131+
for (long p = 0; p < k; ++p) {
2132+
const double av = ta ? A[p * lda + i] : A[i * lda + p];
2133+
const double bv = tb ? B[j * ldb + p] : B[p * ldb + j];
2134+
acc += av * bv;
2135+
}
2136+
C[i * ldc + j] = beta * C[i * ldc + j] + acc;
2137+
}
2138+
};
2139+
struct Cfg { bool ta, tb; long m, n, k; };
2140+
for (const Cfg cfg : {Cfg{true, false, 5, 7, 4}, // tot_x_t shape
2141+
Cfg{false, false, 6, 3, 8}}) { // t_x_tot shape
2142+
const long m = cfg.m, n = cfg.n, k = cfg.k;
2143+
const long lda = cfg.ta ? m : k; // A is (k x m) if trans_a else (m x k)
2144+
const long ldb = cfg.tb ? k : n; // B is (n x k) if trans_b else (k x n)
2145+
const long ldc = n;
2146+
std::vector<double> A(static_cast<std::size_t>(m) * k),
2147+
B(static_cast<std::size_t>(k) * n),
2148+
C(static_cast<std::size_t>(m) * n, 0.3);
2149+
for (std::size_t i = 0; i < A.size(); ++i) A[i] = 0.1 * double(i) + 0.5;
2150+
for (std::size_t i = 0; i < B.size(); ++i) B[i] = 0.2 * double(i) - 0.3;
2151+
std::vector<double> Cref = C;
2152+
ref_gemm(cfg.ta, cfg.tb, m, n, k, A, lda, B, ldb, /*beta=*/1.0, Cref, ldc);
2153+
const bool ran = TA::detail::scale_libxsmm_dgemm(
2154+
cfg.ta, cfg.tb, m, n, k, A.data(), lda, B.data(), ldb, /*beta=*/1.0,
2155+
C.data(), ldc);
2156+
if (ran) {
2157+
for (long i = 0; i < m * n; ++i)
2158+
BOOST_CHECK_CLOSE(C[static_cast<std::size_t>(i)],
2159+
Cref[static_cast<std::size_t>(i)], 1e-10);
2160+
}
2161+
}
2162+
}
2163+
21122164
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)