Fixing a memory issue in the cgemm zen4 packing kernel
In the loop section where the leftover m and k iterations were handled, the load operations (in the k-direction) were missing the mask instructions which has now been added.
Resolved memory-access issues in the SGEMM SUP kernels on AVX2 and AVX-512 by correcting instructions that could read invalid addresses in the C matrix.
Standardized all instruction macros for sgemm to lowercase in the Zen4 kernel to improve readability and code consistency.
AMD-Internal: CPUPL-8117
AMD-Internal: CPUPL-8189
Co-authored-by: Rohan Rayan rohrayan@amd.com
Implement zen6 cpuid and arch changes, and add zen6 as a
separate BLIS sub-configuration and code path within amdzen
configuration family. Currently all optimization choices are
copies of zen5 sub-configuration.
AMD-Internal: [CPUPL-7162]
APIs like GEMV use DOTXF (for parts of problem which are multiple of fuse_factor) and DOTXV (for parts not multiple of fuse_factor).
DOTXF and DOTXV use different numbers of temporary accumulation registers(rho).
This results in different round offs which can be significant when sizes are small and problem is about equally divided between DOTXF and DOTXV.
To fix this, the number of temporary accumulation (and therefore roundoffs) and have made identical across both kernels.
Known related GCC bugs to reference
GCC Bug #56812 — incorrect code with vzeroupper and register allocation
GCC Bug #95483 — vzeroupper clobbers live values
GCC Bug #101617 — wrong code generation with AVX intrinsics and transitions
AMD-Internal:CPUPL-8015
Fix NaN handling in AVX2 amax kernel by initializing global max to 0
In the AVX2 kernel, the global maximum (curr_max_val) was previously
initialized to -1, while the local maximum (temp) is initialized to 0.
The kernel determines the maximum using the condition:
max(abs(x[i]), temp) > curr_max_val
Because curr_max_val started at -1, this initial check always evaluates
to true for the first iteration ( 0 > -1), which is incorrect if the
first element is a NaN.
If subsequent elements are valid numbers, curr_max_val eventually recovers
and updates to the correct value. However, if the entire array consists of
NaNs, this logic fails to properly update the trackers, meaning we never
correctly return index 0 as required by the BLAS standard.
To fix this and align the behavior with the AVX-512 kernel, curr_max_val
is now initialized to 0. This ensures that the initial condition evaluates
correctly and all-NaN arrays return the proper index (0 if all values are NaN).
Window start and end index are also updated to 0 which is the minimum valid
value of index.
AMD-Internal: CPUPL-8047
Copy of similar change in upstream BLIS (843a5e8) to fix issues
https://github.com/flame/blis/issues/873 and
https://github.com/amd/blis/issues/50
Details:
- Previously, `<omp.h>` was included in `bli_thrcomm_openmp.h` so that the
framework could access the necessary OpenMP functions.
- As @melven reported (#873), this causes issues when `blis.h` is included
in C++ code since the `<omp.h>` include happens with `extern "C"`.
- Move the include from the header to the necessary .c files so that it
does not "pollute" `blis.h`.
Thanks to @DaAwesomeP and @bartoldeman for reporting this issue in
AOCL BLIS
AMD-Internal: [CPUPL-7303]
bli_arch_query_id() is used to select kernels in optimized BLAS APIs. Previous
implementation incurred the overhead of multiple function calls. This has
been reduced by:
- Changing the function to be defined in a header file so it can be inlined.
- Avoiding call to bli_arch_check_id_once that was a wrapper for a call to
bli_pthread_once. Instead bli_pthread_once is called directly.
- For builds with a single BLIS sub-configuration, correct arch_id is taken
directly from a header file in the corresponding config subdirectory,
avoiding the bli_pthread_once call and making the value explicit at
compile time, which may enable additional optimizations.
To enable these changes, the variables arch_id and model_id defined in
frame/base/bli_arch.c are no longer static, as they must be accessed in multiple
files (i.e. they are now global variables). Rename to g_arch_id and g_model_id
to distinguish from any locally defined arch_id or model_id variables.
Fixing some inefficiencies on the zen (AVX2) SUP RD kernel for SGEMM.
After performing the iteration for the 8 loop, the next loop that was being performed was the 1 loop for the k-direction.
This caused a lot of unnecessary iterations when the remainder of k < 8.
This has been fixed by introducing masked operations for k < 8
When remainder of k == 1, we handle this with the original non-masked code (with a branch) as the masked code introduces more penalty because of the masking operation.
There were also some unnecessary instructions in the zen4 kernels which have been removed.
AMD-Internal: https://amd.atlassian.net/browse/CPUPL-7775
Co-authored-by: rohrayan@amd.com
GCC over-optimizes intrinsics code by reordering and interleaving
instructions, making it difficult to verify correctness and causing
potential accuracy issues in certain cases. This change replaces
intrinsics-based implementations with inline assembly to ensure
one-to-one mapping between source and generated assembly.
Changes:
- bli_saxpyv_zen4_int: Converted AVX-512 intrinsics to inline assembly
* Processes blocks of 128, 64, 32, 16, and 8 elements
* Handles fringe cases with masked operations
* Preserves scalar path for non-unit strides
- bli_saxpyf_zen_int_5: Converted AVX2 intrinsics to inline assembly
* Processes blocks of 16 and 8 elements with 5-way fusion
* Handles fringe cases with masked operations
* Preserves scalar path for non-unit strides
Benefits:
- Predictable code generation with no compiler reordering
- Better numerical accuracy by preventing unexpected transformations
- Easier verification of generated assembly against specifications
- Explicit control over instruction sequence and register allocation
- The current build systems have the following behaviour
with regards to building "aocl_gemm" addon codebase(LPGEMM)
when giving "amdzen" as the target architecture(fat-binary)
- Make: Attempts to compile LPGEMM kernels using the same
compiler flags that the makefile fragments set for BLIS
kernels, based on the compiler version.
- CMake: With presets, it always enables the addon compilation
unless explicitly specified with the ENABLE_ADDON variable.
- This poses a bug with older compilers, owing to them not supporting
BF16 or INT8 intrinsic compilation.
- This patch adds the functionality to check for GCC and Clang compiler versions,
and disables LPGEMM compilation if GCC < 11.2 or Clang < 12.0.
- Make: Updated the configure script to check for the compiler version
if the addon is specified.
CMake: Updated the main CMakeLists.txt to check for the compiler version
if the addon is specified, and to also force-update the associated
cache variable update. Also updated kernels/CMakeLists.txt to
check if "aocl_gemm" remains in the ENABLE_ADDONS list after
all the checks in the previous layers.
AMD-Internal: [CPUPL-7850]
Signed-off by : Vignesh Balasubramanian <Vignesh.Balasubramanian@amd.com>
Fixing some inefficiencies on the zen4 SUP RD kernel for SGEMM
The loops for the 8 and 1 iteration of the K-loop were performing loads on ymm/xmm registers and computation on zmm registers
This caused multiple unnecessary iterations in the kernel for matrices with certain k-values.
Fixed by introducing masked loads and computations for these cases
AMD-Internal: https://amd.atlassian.net/browse/CPUPL-7762
Co-authored-by: Rohan Rayan <rohrayan@amd.com>
* Adding a model to determine which matrices enter the SGEMM tiny path
* This extends the sizes of matrices that enter the tiny path, which was constrained to the L1 cache size previously
* Now matrices that fit in L2 are also allowed into the tiny path, provided they are determined to be faster than the SUP path
* Adding thresholds based on the SUP path sizes
* Added for Zen4 and Zen5
---------
AMD-Internal: CPUPL-7555
Co-authored-by: Rohan Rayan <rohrayan@amd.com>
- Updated the conversion function(in case of receiving
column stored inputs) from BF16 to F32, in order to
use the correct strides while storing.
- Conversion of B is potentially multithreaded using
the threads meant for IC compute. With the wrong
strides in the kernel, this gives rise to incorrect
writes onto the miscellaneous buffer.
AMD-Internal: [CPUPL-7675]
Co-authored-by: Vishal-A <Vishal.Akula@amd.com>
Co-authored-by: Vignesh Balasubramanian <vignbala@amd.com>
- If blis is compiled as multithreaded library, BLIS_NUM_THREADS is set to 1, and sizes are large enough for multithreaded path to be optimal, we take multithreaded path even though we can spawn only one thread. This adds openmp overhead.
- A check has been added inside the multithreaded kernels to check to use single threaded code path of only 1 thread can be spawned.
AMD-Internal : [SWLCSG-3408]
(cherry picked from commit b60542c45c)
- If blis is compiled as multithreaded library, BLIS_NUM_THREADS is set to 1, and sizes are large enough for multithreaded path to be optimal, we take multithreaded path even though we can spawn only one thread. This adds openmp overhead.
- A check has been added inside the multithreaded kernels to check to use single threaded code path of only 1 thread can be spawned.
AMD-Internal : [SWLCSG-3408]
- When alpha == 0, we are expected to only scale y vector with beta and not read A or X at all.
- This scenario is not handled properly in all code paths which causes NAN and INF from A and X being wrongly propagated. For example, for non-zen architecture (default block in switch case) no such check is present, similarly some of the avx512 kernels are also missing these checks.
- When beta == 0, we are not expected to read Y at all, this also is not handled correctly in one of the avx512 kernel.
- To fix these, early return condition for alpha == 0 is added to bla layer itself so that each kernel does not have to implement the logic.
- DGEMV AVX512 transpose kernel has been fixed to load vector Y only when beta != 0.
AMD-Internal: [CPUPL-7585]
- When alpha == 0, we are expected to only scale y vector with beta and not read A or X at all.
- This scenario is not handled properly in all code paths which causes NAN and INF from A and X being wrongly propagated. For example, for non-zen cpus (default block in switch case) no such check is present, similarly some of the avx512 kernels are also missing these checks.
- When beta == 0, we are not expected to read Y at all, this also is not handled correctly in one of the avx512 kernel.
- To fix these, early return condition for alpha == 0 is added to bla layer itself so that each kernel does not have to implement the logic.
- DGEMV AVX512 transpose kernel has been fixed to load vector Y only when beta != 0.
AMD-Internal: [CPUPL-7585]
- Remove redundant AOCL_DTL_LOG_NUM_THREADS calls from early return paths
- Update thread count logging to use AOCL_get_requested_threads_count() for early exits
- Clean up duplicate DTL logging in gemv_unf_var1 and gemv_unf_var2 implementations
- Remove thread count logging from bli_dgemv_n_zen4_int kernel variants
- Simplify aocldtl_blis.c AOCL_DTL_log_gemv_sizes by removing redundant conditional
- Standardize DTL trace exit patterns across axpy, scal, and gemv operations
- Remove commented-out DTL logging code in zen4 gemv kernel
This patch ensures thread count is logged only once per operation and uses
the correct API (AOCL_get_requested_threads_count) for early exit scenarios
where the actual execution thread count may differ from requested threads.
- Remove redundant AOCL_DTL_LOG_NUM_THREADS calls from early return paths
- Update thread count logging to use AOCL_get_requested_threads_count() for early exits
- Clean up duplicate DTL logging in gemv_unf_var1 and gemv_unf_var2 implementations
- Remove thread count logging from bli_dgemv_n_zen4_int kernel variants
- Simplify aocldtl_blis.c AOCL_DTL_log_gemv_sizes by removing redundant conditional
- Standardize DTL trace exit patterns across axpy, scal, and gemv operations
- Remove commented-out DTL logging code in zen4 gemv kernel
This patch ensures thread count is logged only once per operation and uses
the correct API (AOCL_get_requested_threads_count) for early exit scenarios
where the actual execution thread count may differ from requested threads.
- In the current implementation of bf16 to f32 conversion for packed data
we handle both GEMM and GEMV conditions in the same function separated
with conditions.
- But, when n = (NC+1) the function would execute GEMV conversion logic
and write back the data inaccurately leading to accuracy issues.
- Hence, modified the convert function and reorder functions to have
separate conversion logic to make it cleaner and avoid confusions.
- Also, updated the API calls to adhere to the changes appropriately.
[AMD-Internal: CPUPL-7540]
- Add explicit parentheses around (n <= 1520) && (k <= 128) to clarify
operator precedence and resolve compiler warning. The intended logic
is (m <= 1380) OR (n <= 1520 AND k <= 128).
- This change eliminates the compiler warning about mixing || and &&
operators without explicit grouping.
- Add explicit parentheses around (n <= 1520) && (k <= 128) to clarify
operator precedence and resolve compiler warning. The intended logic
is (m <= 1380) OR (n <= 1520 AND k <= 128).
- This change eliminates the compiler warning about mixing || and &&
operators without explicit grouping.
Adding SGEMM tiny path for Zen architectures.
Needed to cover some performance gaps seen wrt MKL
Only allowing matrices that all fit into the L1 cache to the tiny path
Only tuned for single threaded operation at the moment
Todo: Tune cases where AVX2 performs better than AVX512 on Zen4
Todo: The current ranges are very conservative, there may be scope to increase the matrix sizes that go into the tiny path
AMD-Internal: CPUPL-7555
Co-authored-by: Rohan Rayan rohrayan@amd.com
- In the current implementation of bf16 to f32 conversion for packed data
we handle both GEMM and GEMV conditions in the same function separated
with conditions.
- But, when n = (NC+1) the function would execute GEMV conversion logic
and write back the data inaccurately leading to accuracy issues.
- Hence, modified the convert function and reorder functions to have
separate conversion logic to make it cleaner and avoid confusions.
- Also, updated the API calls to adhere to the changes appropriately.
[AMD-Internal: CPUPL-7540]
- Revert the logical operator from OR (||) to AND (&&) in the DCOMPLEX
(ZGEMM) SUP threshold condition for k <= 128. The previous change to
OR logic was causing performance regressions for certain input sizes
by incorrectly routing cases to the SUP path when the native path
would be more optimal.
- Revert the logical operator from OR (||) to AND (&&) in the DCOMPLEX
(ZGEMM) SUP threshold condition for k <= 128. The previous change to
OR logic was causing performance regressions for certain input sizes
by incorrectly routing cases to the SUP path when the native path
would be more optimal.
- Current kernel uses masked AVX512 instructions to handle fringe cases.
- These instructions are slow on genoa.
- To handle sizes less than 8, AVX2 and SSE code has been added.
- Existing masked AVX512 code is performing better when n > 8 therefore it is still kept for handling larger sizes where n % 8 != 0.
AMD-Internal: [CPUPL-7467]
(cherry picked from commit 1dca574a9d)
- Current kernel uses masked AVX512 instructions to handle fringe cases.
- These instructions are slow on genoa.
- To handle sizes less than 8, AVX2 and SSE code has been added.
- Existing masked AVX512 code is performing better when n > 8 therefore it is still kept for handling larger sizes where n % 8 != 0.
AMD-Internal: [CPUPL-7467]
Threshold tuning that determines whether SUP or native path should
be used for given input matrix size.
This tuning forces skinny matrices to take SUP path to ensure better
performance.
AMD-Internal: [CPUPL-7369]
Co-authored-by: harsh dave <harsdave@amd.com>
Threshold tuning that determines whether SUP or native path should
be used for given input matrix size.
This tuning forces skinny matrices to take SUP path to ensure better
performance.
AMD-Internal: [CPUPL-7369]
Co-authored-by: harsh dave <harsdave@amd.com>
* commit 'db3134ed6d239a7962a2b7470d8c46611b9d17ef':
Disabled no post-ops path in lpgemm f32 kernels for few gcc versions
DTL Log update
Add external PR integration process and flowchart to CONTRIBUTING.md
Enabled disable-sba-pools feature in AOCL-BLAS (#101)
Fix for F32 to BF16 Conversion and AVX512 ISA Support Checks
Fixed Integer Overflow Issue in TPSV
Add AI Code Review workflow (#211)
Add AI Code Review Self-enablement file (#209)
Re-tuned GEMV thresholds (#210)
Adding bli_print_msg before bli_abort() for bli_thrinfo_sup_create_for_cntl
Add missing license text
Modified AVPY kernel to ensure consistency of numerical results (#188)
Fix memory leak in DGEMV kernel (#187)
Tuned DGEMV no-transpose thresholds #193
Set Security flags default enable (#194)
Standardize Zen kernel names (2)
Compiler warnings fixes (2)
coverity issue fix for ztrsm (#176)
Fixes Coverity static analysis issue in the DTRSM (#181)
Add files via upload (#197)
Initialize mem_t structures safely and handle NULL communicator in threading
Tidying code
Compiler warnings fixes
Fixing the coverity issues with CID: 23269 and CID: 137049 (#180)
Fixed high priority coverity issues in LPGEMM. (#178)
GCC 15 SUP kernel workaround (2)
Disable small_gemm for zen4/5 and added single thread check for tiny path (#167)
Optimal rerouting of GEMV inputs to avoid packing
Updated Guards in s8s8s32of32_sym_quant Framework
Fixed out-of-bound access in F32 matrix add/mul ops (#168)
* DTL Log update
Updates logs with nt and AOCL Dynamic selected nt for axpy, scal and dgemv
Modified bench_gemv.c to able to process modified dtl logs.
* Updated DTL log for copy routine with actual nt and dynamic nt
* Refactor OpenMP pragmas and clean up code
Removed unnecessary nested OpenMP pragma and cleaned up function end comment.
* Fixed DTL log for sequential build
* Added thread logging in bla_gemv_check for invalid inputs
---------
Co-authored-by: Smyth, Edward <Edward.Smyth@amd.com>
- Fixed register assignment bug in lpgemv_m_kernel_f32_avx512 where zmm3
was incorrectly used instead of zmm4 in BF16_F32_BETA_OP_NLT16F_MASK macro.
- Replaced hardware-specific BF16 conversion intrinsics with manual
rounding, bit manipulation and F32 instruction set for compatibility on
hardware without native BF16 support.
- Added AVX512_BF16 ISA support checks for s8s8s32obf16 and u8s8s32obf16
GEMM operations to ensure processor compatibility before execution.
AMD-Internal: [CPUPL-7410]
Current DAXPY kernel uses C code to solve cases when n %8 != 0.
This results in the compiled code using MUL+ADD instruction using SSE, instead of FMA instruction.
This causes inconsistency of numerical results.
To fix this, AVX2 and C code is replaced with masked AVX512 instructions to compute fringe cases.
AMD-Internal : [CPUPL-7315]
Memory is not freed for GEMV when MT kernel with called for NT = 1;
Fixed this by adding an extra check to make sure memory is freed.
AMD-Internal: [CPUPL-7352]
Fix compiler warning messages in LPGEMM code:
- Removed extraneous parentheses in aocl_batch_gemm_s8s8s32os32.c
- Removed unused variables in lpgemv_{m,n}_kernel_s8_grp_amd512vnni.c
- Changed ERR_UBOUND in math_utils_avx2.h and math_utils_avx512.h
to match how it is specified in AOCL libm erff.c
AMD-Internal: [CPUPL-6579]
* Fixed coverity issue in ztrsm small code path
* Fixed coverity issue in ztrsm small code path
---------
Co-authored-by: harsh dave <harsdave@amd.com>
* Fixes Coverity static analysis issue in the DTRSM
- Initializes ps_a_use variable and calls bli_auxinfo_set_ps_a() to set
pack stride in auxinfo structure.
* Fixed unintialized variable issue in the DTRSM
- Initializes ps_a_use variable and calls bli_auxinfo_set_ps_a() to set
pack stride in auxinfo structure.
---------
Co-authored-by: harsdave <harsdave@amd.com>
- AMD specific BLAS1 and BLAS2 franework: changes to make variants
more consistent with each other
- Initialize kernel pointers to NULL where not immediately set
- Fix code indentation and other other whitespace changes in DTL
code and addon/aocl_gemm/frame/s8s8s32/lpgemm_s8s8s32_sym_quant.c
- Fix typos in DTL comments
- Add missing newline at end of test/CMakeLists.txt
- Standardize on using arch_id variable name
AMD-Internal: [CPUPL-6579]
