[CPU][ARM] Guard ARM SVE kernel entry points by allnes · Pull Request #36453 · openvinotoolkit/openvino

allnes · 2026-06-17T18:46:01Z

Cause

On AArch64 with GCC (cross-compile, gcc-10) + LTO, -march=armv8-a+sve target attributes from runtime-dispatched SVE clones leak into generic ARMv8-A baseline code.
GCC auto-vectorizes generic helpers (e.g. std::vector<...> resize in the SVE PagedAttention AttentionExecutor::init) with SVE instructions (mov z0.b, #0 / whilelo / st1d).
On cores without SVE (e.g. Cortex-A72) these are illegal instructions → SIGILL at runtime (reached on a TBB worker thread).
brgemm_kernel.cpp ISA selection unconditionally fell back to sve_128 without checking mayiuse(...), so it built oneDNN kernels that emit SVE on cores without SVE.

Solution

precision_support: add a single ARM ISA gate — enum class ArmISA { ASIMD, SVE, DOTPROD, I8MM } and bool hasArmISASupport(ArmISA) (ASIMD → always true baseline, SVE → with_cpu_sve(), DOTPROD/I8MM → with_cpu_arm_*). It consolidates the former hasIntDotProductSupport() / hasInt8MMSupport() helpers. Declared/defined for ARM targets only, so x86_64 / RISC-V are unaffected.
ACL executors: encapsulate the precondition common to every ACL executor in aclCommonExecutorSupported() (in acl_utils.hpp; today the ARMv8-A NEON/ASIMD baseline, extensible). Each ACL executor calls it in its supports() / isSupported() predicate (VERIFY(..., UNSUPPORTED_ACL_COMMON_PRECONDITION) or if/return + DEBUG_LOG), keeping its op-specific checks. ASIMD is always present on AArch64, so this never over-restricts.
kleidiai_mm / fullyconnected (KleidiAI path): query hasArmISASupport directly (ASIMD, plus DOTPROD/I8MM for ukernel selection).
brgemm_kernel.cpp: getSupportedSveIsa(min_isa = sve_128) returns the highest supported SVE ISA at or above min_isa, or isa_undef (then the caller throws) instead of the unconditional sve_128 fallback. The main brgemm kernel uses the default sve_128 floor; the copy_a / copy_b kernels pass sve_256 (oneDNN only provides sve_256/sve_512 for them — create_brgemm_matmul_copy_* asserts a superset of sve_256).
paged_attn.cpp: decline the ARM PagedAttention executor when the core has no SVE (hasArmISASupport(ArmISA::SVE)), in the baseline caller before make_pa_executor dispatches to the SVE clone, so the SVE-autovectorized AttentionExecutor::init is never reached. The node throws cleanly instead.
SVE codegen is unchanged: SVE-capable cores keep SVE paths; non-SVE cores decline them and fall back to ANY/NEON.

Tickets

CVS-179098

alvoron · 2026-06-18T14:48:56Z

        OPENVINO_THROW("make_pa_executor: unsupported precision: ", data_type);
    }
 #elif (defined(OPENVINO_ARCH_ARM64) && defined(HAVE_SVE))
+    if (!ov::intel_cpu::hasArmSVESupport()) {


Why do we need to call hasArmSVESupport here? The code is wrapped by HAVE_SVE define.

alvoron · 2026-06-18T14:49:18Z

@@ -1,5 +1,4 @@
 // Copyright (C) 2018-2026 Intel Corporation
-// Copyright (C) 2024 FUJITSU LIMITED


I think we need to bring it back

alvoron · 2026-06-18T14:49:36Z

 }

 bool MatMulKleidiAIExecutor::supports(const FCConfig& config) {
+    VERIFY(hasArmASIMDSupport(), UNSUPPORTED_ISA);


I think this check should be applied to all executors

allnes · 2026-06-18T17:29:47Z

@alvoron I fixed comments

alvoron · 2026-06-19T14:45:28Z


 bool hasHardwareSupport(const ov::element::Type& precision);
 ov::element::Type defaultFloatPrecision();
+bool hasArmASIMDSupport();


Does it make sense to compile on ARM only?

alvoron · 2026-06-19T14:56:49Z

+#endif
+}
+
+bool hasArmSVESupport() {


Add a comment why we need SVE >= 128
Probably we need to add commend here and rename the method

alvoron · 2026-06-23T14:10:12Z

As we discussed, the vulnerability of the proposed fix is to necessity to call new method aclCommonExecutorSupported in all ACL executors. So, the risk is to skip this check in future ACL executors.
We'll need to think how to mitigate this issue in future.

maxnick · 2026-06-23T14:59:07Z

+ * @return whether the current core meets the common preconditions to run ACL executors
+ */
+inline bool aclCommonExecutorSupported() {
+    return hasArmISASupport(ArmISA::ASIMD);


This is in fact only a ISA check. Do expect it to be extended?

maxnick · 2026-06-24T13:19:56Z

    }

+    if (!aclCommonExecutorSupported({srcDescs[0], dstDescs[0]})) {
+        DEBUG_LOG("ACL common preconditions not met");


Suggested change

DEBUG_LOG("ACL common preconditions not met");

DEBUG_LOG("ACL common preconditions are not met");

maxnick · 2026-06-24T13:20:56Z

                                   const std::vector<MemoryDescPtr>& srcDescs,
                                   const std::vector<MemoryDescPtr>& dstDescs) const override {
+        if (!aclCommonExecutorSupported({srcDescs[0], dstDescs[0]})) {
+            DEBUG_LOG("ACL common preconditions not met");


Suggested change

DEBUG_LOG("ACL common preconditions not met");

DEBUG_LOG("ACL common preconditions are not met");

Here and in other such messages

maxnick · 2026-06-24T13:23:13Z

+inline bool aclCommonExecutorSupported(const std::vector<MemoryDescPtr>& descs) {
+    return hasArmISASupport(ArmISA::ASIMD) && std::all_of(descs.begin(), descs.end(), [](const MemoryDescPtr& desc) {
+               return desc->empty() || precisionToAclDataType(desc->getPrecision()) != arm_compute::DataType::UNKNOWN;
+           });
+}


Suggested change

inline bool aclCommonExecutorSupported(const std::vector<MemoryDescPtr>& descs) {

return hasArmISASupport(ArmISA::ASIMD) && std::all_of(descs.begin(), descs.end(), [](const MemoryDescPtr& desc) {

return desc->empty() || precisionToAclDataType(desc->getPrecision()) != arm_compute::DataType::UNKNOWN;

});

}

inline bool aclSupported(const std::vector<MemoryDescPtr>& descs) {

return hasArmISASupport(ArmISA::ASIMD) && std::all_of(descs.begin(), descs.end(), [](const MemoryDescPtr& desc) {

return desc->empty() || precisionToAclDataType(desc->getPrecision()) != arm_compute::DataType::UNKNOWN;

});

}

maxnick · 2026-06-24T13:30:40Z

Do we have a better place for the hasArmISASupport declaration? It looks like this header is dedicated to a precision support checks, not ISA

Explain why getSupportedSveIsa() allows sve_128 (the main brgemm kernel has a sve_128 implementation) while getSupportedSve256Isa() requires >= sve_256 for the copy_a/copy_b kernels (oneDNN's create_brgemm_matmul_copy_* asserts the isa is a superset of sve_256), and that both return isa_undef on a core without the required SVE so callers decline instead of emitting illegal instructions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…opyright The three ACL executors that share the ACLCommonExecutor base (convolution, fully-connected, lowp fully-connected) no longer repeat the ASIMD ISA check in their per-type supports() predicate. The check is hoisted once into ACLCommonExecutor::update(), which already returns false (declining the executor so the framework falls back) on validation failure. ASIMD is the ARMv8-A baseline, so this never over-restricts. Executors that do not share this base (acl_eltwise, kleidiai) and the legacy ACL builders (which share a base with non-ACL builders) keep declaring the ISA explicitly. Also restore the original position of the FUJITSU copyright line in brgemm_kernel.cpp (line 2, as upstream had it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…helper Address review feedback on precision_support: - Declare the ArmISA enum and hasArmISASupport() only on ARM targets (OPENVINO_ARCH_ARM / OPENVINO_ARCH_ARM64). They are referenced solely by the ARM executors (ACL / KleidiAI), which compile only on ARM, so there is no reason to expose them elsewhere. - Make the ASIMD/SVE detection helpers file-local (static): they have no external callers and are used only to implement hasArmISASupport(). - Rename hasArmSVESupport() to hasArmBaselineSVESupport() and add a comment explaining why it checks mayiuse(sve_128): sve_128 is the lowest SVE tier in oneDNN's ISA model, so it answers "does this core have SVE at all". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Replace the nested OPENVINO_ARCH_ARM64 inside the ARM||ARM64 block with a flat #if ARM64 / #elif ARM chain (matching the file's existing arch-dispatch style): the ARM64 branch holds the aarch64::mayiuse queries, the 32-bit ARM branch is the permissive NEON-baseline gate. Shorten the helper comments to dry, descriptive notes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

hasArmISASupport() previously used dnnl aarch64::mayiuse, which exists only on AArch64, forcing an ARM64/ARM #if split. Switch the SVE/dotprod/i8mm queries to the cross-architecture with_cpu_sve()/with_cpu_arm_dotprod()/with_cpu_arm_i8mm() helpers from system_conf (already used by hasIntDotProductSupport/hasInt8MMSupport), and treat ASIMD as the always-present ARM baseline. The single function now compiles unchanged on AArch64 and 32-bit ARM under one OPENVINO_ARCH_ARM || OPENVINO_ARCH_ARM64 guard, with no per-arch branch. Drop the now-unused aarch64 cpu_isa_traits include. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…SASupport Remove the standalone hasIntDotProductSupport() and hasInt8MMSupport() helpers, which are exactly hasArmISASupport(ArmISA::DOTPROD) and hasArmISASupport(ArmISA::I8MM). hasArmISASupport already queries with_cpu_arm_dotprod()/with_cpu_arm_i8mm() directly, so this is behavior-preserving. Update the callers (fullyconnected, kleidiai_mm, skip_tests_config) to the single entry point, leaving one ISA query API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

clang-tidy misc-include-cleaner failed the x86_64/riscv64 build: after folding hasIntDotProductSupport/hasInt8MMSupport into hasArmISASupport and guarding the latter with #if ARM, system_conf.hpp became unused on non-ARM targets (the remaining with_cpu_* calls were all inside the ARM guard), so the include was flagged as not used directly. Make hasArmISASupport (and the ArmISA enum declaration) unconditional, like the original helpers were: its body uses only the cross-architecture with_cpu_* queries from system_conf (which return false off-ARM), so it compiles on every target and keeps system_conf.hpp used directly without any arch-conditional include. Drop the now-unneeded visibility.hpp include from the header. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The internal ARM CI (ie_python_api_arm, ubuntu22 arm64 cpack) still crashed with "Fatal Python error: Illegal instruction" in test_async_mixed_values[int16]: the SVE PagedAttention clone's AttentionExecutor::init resizes std::vector members, which GCC may auto-vectorize with SVE instructions that execute before any kernel-level backstop runs, so on a core without SVE the process hits an illegal instruction. Decline the ARM PagedAttention executor at make_pa_executor entry when the core has no SVE, so the executor is never constructed and no SVE instruction is reached. This is a runtime check via hasArmISASupport(ArmISA::SVE) (the same SVE detector the cross-compiled dispatcher uses) placed inside the existing ARM64 && HAVE_SVE block - no new build-time define. The precision_support.h include is added to that same block, so non-ARM and non-SVE builds are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

getSupportedSveIsa() and getSupportedSve256Isa() differed only by their minimum SVE level. Fold them into a single getSupportedSveIsa(min_isa) with a default of sve_128: the main brgemm kernel uses the default floor, while the copy_a/copy_b kernels (oneDNN provides only sve_512/sve_256 for them) pass sve_256. Returns the highest supported SVE ISA at or above min_isa, or isa_undef otherwise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…build Per review: ArmISA / hasArmISASupport should not exist on non-ARM targets. Guard both the declaration (header) and the definition (.cpp), and the system_conf.hpp include, under OPENVINO_ARCH_ARM || OPENVINO_ARCH_ARM64 together. Keeping the function unconditional while guarding only the include left with_cpu_* used on x86_64 with the header missing; guarding only the function left system_conf.hpp orphaned and tripped clang-tidy misc-include-cleaner. Guarding both keeps the consolidated single hasArmISASupport entry point, leaves no ARM symbols or unused includes on x86_64 / RISC-V, and all callers are ARM-only compiled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Trim the verbose comments on the ARM ISA gate to short technical notes, and document the SVE case of hasArmISASupport (with_cpu_sve() = any SVE vector length) that lost its explanation when the per-ISA helpers were folded in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…lder Per review, the SVE check did not belong inside executor_pa.cpp, which is already wrapped by HAVE_SVE (it is the SVE cross-compiled clone). Move it to the caller in paged_attn.cpp - a baseline translation unit, not cross-compiled - so the decision is made before make_pa_executor dispatches to the SVE clone, and there is no HAVE_SVE-guarded SVE-specific check. On a core without SVE the builder returns nullptr and the node throws cleanly instead of reaching the SVE-autovectorized AttentionExecutor::init. Drops the guard and the precision_support.h include from executor_pa.cpp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Move the ASIMD ISA gate for the three ACLCommonExecutor children (convolution, fully-connected, lowp fully-connected) out of ACLCommonExecutor::update() back into each executor's supports() predicate, via VERIFY(..., UNSUPPORTED_ISA) like the other ACL/KleidiAI executors. The gate belongs in the selection predicate, not the runtime update path, and this keeps the idiom uniform: every executor whose predicate is VERIFY-based now declares the ISA with VERIFY, while the if/return builders (eltwise and the .hpp builders) keep their own style. Drops the now-unused gate and precision_support.h include from acl_common_executor.cpp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Replace the per-executor hasArmISASupport(ArmISA::ASIMD) check with a single aclCommonExecutorSupported() helper in acl_utils.hpp that encapsulates the preconditions common to every ACL executor regardless of op/config type (today: the ARMv8-A NEON/ASIMD baseline). Each executor calls it with its own predicate idiom (VERIFY for the VERIFY-based supports()/isSupported(), if/return for the builders that have no VERIFY) and keeps its op-specific checks. New common ACL preconditions can now be added in one place. Drops the now-orphaned precision_support.h include from the ACL executors (acl_utils.hpp pulls it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

aclCommonExecutorSupported() is the extensible common-precondition gate (today ISA, possibly more later), but the VERIFY sites reported UNSUPPORTED_ISA, which would be misleading once a non-ISA precondition is added. Add and use a dedicated UNSUPPORTED_ACL_COMMON_PRECONDITION message, and give the if/return ACL builders a matching DEBUG_LOG so the decline reason is uniform across all ACL executors. KleidiAI keeps UNSUPPORTED_ISA (it calls hasArmISASupport directly, not the ACL common helper). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…rted hasArmISASupport() ended its switch with a fallthrough `return true`, so an ArmISA value not handled in the switch (e.g. a future ISA added to the enum but not wired up here) would be silently reported as supported — which could let an executor run instructions the core lacks. Replace the fallthrough with OPENVINO_THROW so such a gap fails loudly and visibly at the call site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The previous commit added an unconditional #include "openvino/core/except.hpp" for OPENVINO_THROW, but OPENVINO_THROW is only used inside the OPENVINO_ARCH_ARM/ARM64 block (hasArmISASupport). On x86_64/RISC-V the include was then unused and clang-tidy misc-include-cleaner failed the build. Move except.hpp into the same ARM #if block as system_conf.hpp so it is included only where used. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…tion Extend aclCommonExecutorSupported() so the precondition shared by every ACL executor covers not only the ISA (ASIMD) but also that all tensor precisions are representable by ACL (precisionToAclDataType != UNKNOWN); an unrepresentable type (e.g. u4/i4/nf4/string) must never reach an ACL kernel. The check accepts the descriptors in whatever form a predicate has them via small overloads (aclSupportedPrecisions for a vector, a MemoryDescArgs map, or an initializer list of individual descriptors), so each executor passes its own tensors without hand-rolling a vector. The map overload skips empty descriptors (e.g. an absent optional bias, whose precision is dynamic) so it never over-restricts. Op-specific precision/rank/layout checks stay in each executor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Replace the overload set + template (vector / MemoryDescArgs map / initializer_list variants) with one function: aclCommonExecutorSupported(const std::vector<MemoryDescPtr>&). Every executor calls it the same way with a braced list of its real I/O tensor descriptors, e.g. aclCommonExecutorSupported({srcDesc, weiDesc, dstDesc}). Empty descriptors are ignored, so an absent optional bias is not passed (no map iteration, no empty-bias false reject). Drops the now-unused memory_arguments.hpp / initializer_list includes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The gather-matmul executors added on master call the removed hasIntDotProductSupport()/hasInt8MMSupport() helpers; switch them to hasArmISASupport(ArmISA::DOTPROD/I8MM) like the rest of the plugin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ix messages Per review comments (maxnick): - Move ArmISA / hasArmISASupport out of precision_support (which is about precision support) into a dedicated utils/arm_isa_support.{h,cpp}. Update the ARM/ACL/KleidiAI consumers' includes; precision_support keeps only the cross-arch hasHardwareSupport / defaultFloatPrecision. Add the new file to the cpuUtils test library. - Rename aclCommonExecutorSupported() -> aclSupported() (shorter, matches the broadened ISA+precision check it now performs). - Fix the decline message grammar: "ACL common preconditions are not met". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…_isa_support.cpp - kleidiai_mm.cpp: move "utils/arm_isa_support.h" to its sorted position among the utils/* includes (clang-format). - arm_isa_support.cpp: include "arm_isa_support.h" inside the OPENVINO_ARCH_ARM* block. On non-ARM (x86_64 / RISC-V) the whole TU is empty, so an unconditional include was flagged unused by clang-tidy misc-include-cleaner. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…he #if arm_isa_support.cpp tested OPENVINO_ARCH_ARM/ARM64 before any header defining those macros was included, so the whole translation unit compiled empty and hasArmISASupport was never emitted. It only linked where the call got inlined (gcc), but failed with "undefined symbol ov::intel_cpu::hasArmISASupport" on clang/LTO (android/arm64). Include openvino/core/visibility.hpp (which defines OPENVINO_ARCH_*) before the guard so the function is actually compiled on ARM. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…skip_tests includes Match the CI clang-format (clang-format-18): remove the blank line between the "arm_isa_support.h" and the system includes inside the ARM block, and put the new "utils/arm_isa_support.h" include in its sorted position in skip_tests_config.cpp. Only the include block of skip_tests is touched (its pre-existing body misformat is upstream's and is left as-is). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

allnes requested review from a team as code owners June 17, 2026 18:46

github-actions Bot added the category: CPU OpenVINO CPU plugin label Jun 17, 2026

allnes force-pushed the an/arm-sve-illegal-instruction-guard branch 2 times, most recently from 2931403 to e57670e Compare June 17, 2026 18:55

allnes added the platform: arm OpenVINO on ARM / ARM64 label Jun 17, 2026

allnes assigned alvoron Jun 17, 2026

allnes requested a review from alvoron June 17, 2026 18:56

allnes force-pushed the an/arm-sve-illegal-instruction-guard branch from e57670e to a6f71ff Compare June 17, 2026 19:47

alvoron reviewed Jun 18, 2026

View reviewed changes

alvoron requested changes Jun 18, 2026

View reviewed changes

allnes force-pushed the an/arm-sve-illegal-instruction-guard branch 2 times, most recently from ef160b6 to 83f2c14 Compare June 18, 2026 15:08

allnes requested a review from alvoron June 18, 2026 17:29

alvoron reviewed Jun 19, 2026

View reviewed changes

alvoron requested changes Jun 19, 2026

View reviewed changes

allnes requested a review from alvoron June 22, 2026 19:01

alvoron approved these changes Jun 23, 2026

View reviewed changes

Comment thread src/plugins/intel_cpu/src/utils/precision_support.cpp Outdated

allnes assigned maxnick Jun 23, 2026

allnes requested a review from maxnick June 23, 2026 14:14

maxnick added this to the 2026.3 milestone Jun 23, 2026

maxnick requested changes Jun 23, 2026

View reviewed changes

allnes force-pushed the an/arm-sve-illegal-instruction-guard branch from 2560f95 to bc5178f Compare June 24, 2026 13:18

allnes requested a review from maxnick June 24, 2026 13:28

maxnick reviewed Jun 24, 2026

View reviewed changes

allnes and others added 22 commits June 24, 2026 16:38

allnes force-pushed the an/arm-sve-illegal-instruction-guard branch from 225d69c to 9cad7e7 Compare June 24, 2026 14:42

maxnick approved these changes Jun 24, 2026

View reviewed changes

allnes and others added 2 commits June 24, 2026 17:51

maxnick enabled auto-merge June 24, 2026 17:43

maxnick added this pull request to the merge queue Jun 24, 2026

Merged via the queue into openvinotoolkit:master with commit 59dfcdd Jun 24, 2026
204 of 205 checks passed

		@@ -1,5 +1,4 @@
		// Copyright (C) 2018-2026 Intel Corporation
		// Copyright (C) 2024 FUJITSU LIMITED

	DEBUG_LOG("ACL common preconditions not met");
	DEBUG_LOG("ACL common preconditions are not met");

Uh oh!

Conversation

allnes commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cause

Solution

Tickets

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allnes commented Jun 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvoron commented Jun 23, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

allnes commented Jun 17, 2026 •

edited

Loading