Skip to content

Commit dfce3cb

Browse files
Laan33JonathanC-ARM
authored andcommitted
Validate HalfGemm packed-B edge cases
- Add validation for null HalfGemm data parameters and reject invalid backend-native packed-B parameter combinations before the zero-K fast path. - Make HalfGemm packed-B size calculation return a valid zero size for degenerate N/K shapes. - Restore the KleidiAI header include needed by debug logging in mlasi_kleidiai.h. - Update HalfGemm PackB tests to avoid exception-only gtest macros in no-exception builds, and add sentinel checks for early-return paths. Signed-off-by: Cathal Lawlor <cathal.lawlor@arm.com>
1 parent aafad59 commit dfce3cb

4 files changed

Lines changed: 46 additions & 12 deletions

File tree

onnxruntime/core/mlas/lib/halfgemm.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ TryGetHalfGemmBackendSelectorConfig(
9292
}
9393
#endif
9494

95-
9695
void
9796
MLASCALL
9897
MlasHalfGemmBatch(
@@ -108,7 +107,21 @@ MlasHalfGemmBatch(
108107
return;
109108
}
110109

110+
if (DataParams == nullptr) {
111+
MLAS_THROW_EX(std::runtime_error, "HalfGemm requires non-null DataParams.");
112+
}
113+
111114
if (K == 0) {
115+
for (size_t gemm_i = 0; gemm_i < BatchN; gemm_i++) {
116+
const auto& data = DataParams[gemm_i];
117+
if (data.BIsBackendNativePacked &&
118+
(data.ldb != 0 || data.Bias != nullptr || data.OutputProcessor != nullptr)) {
119+
MLAS_THROW_EX(
120+
std::runtime_error,
121+
"backend-native halfgemm packed B with K==0 requires ldb == 0, Bias == nullptr, and "
122+
"OutputProcessor == nullptr");
123+
}
124+
}
112125
MlasHalfGemmZeroKBatch(M, N, BatchN, DataParams);
113126
return;
114127
}

onnxruntime/core/mlas/lib/halfgemm.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ MlasHalfGemmTryGetPackedBSize(
6161
size_t* PackedBSize
6262
)
6363
{
64+
if (N == 0 || K == 0) {
65+
*PackedBSize = 0;
66+
return true;
67+
}
68+
6469
if (PackedK == 0) {
6570
return false;
6671
}

onnxruntime/core/mlas/lib/kleidiai/mlasi_kleidiai.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "../mlasi.h"
1010
#include <limits>
11+
#include <iostream>
1112
#include <vector>
1213

1314
// Fix to ensure compatibility with MSVC build

onnxruntime/test/mlas/unittest/test_halfgemm.cpp

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ struct HalfGemmPackBPaddingKernel {
3333
static constexpr size_t PackedK = 4;
3434
};
3535

36+
void ExpectBufferFilledWith(const std::vector<std::byte>& buffer, std::byte expected) {
37+
const auto expected_value = std::to_integer<unsigned int>(expected);
38+
for (size_t i = 0; i < buffer.size(); ++i) {
39+
EXPECT_EQ(std::to_integer<unsigned int>(buffer[i]), expected_value) << "index=" << i;
40+
}
41+
}
42+
3643
} // namespace
3744

3845
#if defined(USE_KLEIDIAI)
@@ -487,25 +494,33 @@ TEST(HalfGemmPackB, ReturnsZeroOnOverflow) {
487494
EXPECT_EQ(MlasHalfGemmPackBSize(max, 2, true), size_t{0});
488495
}
489496

490-
TEST(HalfGemmPackB, PackBReturnsOnOverflow) {
497+
TEST(HalfGemmPackB, PackBLeavesOutputUnchangedOnOverflow) {
491498
const size_t max = (std::numeric_limits<size_t>::max)();
492499
std::vector<MLAS_FP16> b(1);
493-
std::vector<std::byte> packed_b(1);
500+
constexpr std::byte sentinel{0x5A};
501+
std::vector<std::byte> packed_b(16, sentinel);
494502

495-
EXPECT_NO_THROW(MlasHalfGemmPackB(max, 2, b.data(), max, packed_b.data()));
503+
MlasHalfGemmPackB(max, 2, b.data(), max, packed_b.data());
504+
ExpectBufferFilledWith(packed_b, sentinel);
496505
}
497506

498-
TEST(HalfGemmPackB, PackBRejectsInvalidArguments) {
507+
TEST(HalfGemmPackB, PackBExitsEarlyForInvalidArguments) {
499508
constexpr size_t N = 5;
500509
constexpr size_t K = 3;
510+
constexpr std::byte sentinel{0x5A};
501511

502512
std::vector<MLAS_FP16> b(K * N);
503-
std::vector<std::byte> packed_b(MlasHalfGemmPackBSize(N, K, false));
513+
std::vector<std::byte> packed_b(MlasHalfGemmPackBSize(N, K, false), sentinel);
504514
ASSERT_FALSE(packed_b.empty());
505515

506-
EXPECT_NO_THROW(MlasHalfGemmPackB(N, K, b.data(), N - 1, packed_b.data()));
507-
EXPECT_NO_THROW(MlasHalfGemmPackB(N, K, nullptr, N, packed_b.data()));
508-
EXPECT_NO_THROW(MlasHalfGemmPackB(N, K, b.data(), N, nullptr));
516+
MlasHalfGemmPackB(N, K, b.data(), N - 1, packed_b.data());
517+
ExpectBufferFilledWith(packed_b, sentinel);
518+
519+
MlasHalfGemmPackB(N, K, nullptr, N, packed_b.data());
520+
ExpectBufferFilledWith(packed_b, sentinel);
521+
522+
MlasHalfGemmPackB(N, K, b.data(), N, nullptr);
523+
ExpectBufferFilledWith(packed_b, sentinel);
509524
}
510525

511526
TEST(HalfGemmPackB, CopyPackBZeroPadsAlignedKTail) {
@@ -746,7 +761,7 @@ TEST(HalfGemmKleidiAIPath, PackedBFloatSingleThreadVariedShapesAndBiasWithoutOut
746761
// KleidiAI-specific packed-B uses a separate direct-consumption contract from
747762
// generic halfgemm PackB. Unsupported combinations fail at the public API
748763
// boundary because generic MLAS cannot consume this backend-native layout.
749-
TEST(HalfGemmKleidiAIPath, KleidiAIPackedBWithBiasThrows) {
764+
TEST(HalfGemmKleidiAIPath, KleidiAIPackedBWithBiasIsRejected) {
750765
if (!MlasFp16AccelerationSupported()) {
751766
GTEST_SKIP();
752767
}
@@ -790,7 +805,7 @@ TEST(HalfGemmKleidiAIPath, KleidiAIPackedBWithBiasThrows) {
790805
#endif
791806
}
792807

793-
TEST(HalfGemmKleidiAIPath, KleidiAIPackedBWithOutputProcessorThrows) {
808+
TEST(HalfGemmKleidiAIPath, KleidiAIPackedBWithOutputProcessorIsRejected) {
794809
if (!MlasFp16AccelerationSupported()) {
795810
GTEST_SKIP();
796811
}
@@ -837,7 +852,7 @@ TEST(HalfGemmKleidiAIPath, KleidiAIPackedBWithOutputProcessorThrows) {
837852
#endif
838853
}
839854

840-
TEST(HalfGemmKleidiAIPath, ZeroKFallsBack) {
855+
TEST(HalfGemmKleidiAIPath, ZeroKIsNotHandledByKleidiAIOverride) {
841856
if (GetMlasPlatform().MlasHalfGemmBatchOverride == nullptr) {
842857
GTEST_SKIP() << "KleidiAI halfgemm override unavailable";
843858
}

0 commit comments

Comments
 (0)