From 77bcb0e7ac0598216c8a01466e32d85e2572cece Mon Sep 17 00:00:00 2001 From: Stephen Jia Date: Tue, 25 Aug 2026 17:15:11 -0400 Subject: [PATCH] Fix Windows MSVC Debug build of XNNPACK and clang-cl kernel gating Summary: Building any Debug preset with XNNPACK enabled under the MSVC toolset fails to compile backends/xnnpack/third-party/XNNPACK/src/subgraph.c. XNNPACK nests two variadic macros: XNN_RETURN_IF_ERROR expands to xnn_log_error, which in turn concatenates a location suffix onto its first parameter. MSVC's default legacy preprocessor forwards the outer __VA_ARGS__ into the inner macro as a single un-split argument, so the suffix is glued onto the last format argument rather than the format string, producing "syntax error: missing ')' before 'string'". Only the three call sites that pass a format string plus format arguments are affected. Release builds escape it because XNN_LOG_LEVEL is 0 outside of Debug, which makes xnn_log_error expand to nothing, and that is why the Release-only windows-msvc CI job never caught this. Enable /Zc:preprocessor for XNNPACK when the compiler is genuine MSVC. The same investigation turned up a second problem. The llm preset disables the quantized and LLM kernels on Windows whenever the MSVC variable is set, but CMake also sets MSVC for clang-cl, so those kernels were being dropped from clang-cl builds while the accompanying warning told the user to switch to -T ClangCL in order to get them. That contradicts the stated intent of #15719, so the check now keys off CMAKE_CXX_COMPILER_ID instead. Fixes #22084. Test Plan: On Windows with VS 2022 (MSVC 19.43), configured with the default MSVC toolset and built the xnnpack-subgraph target in Debug. Before the change the build fails with C2143/C2059 at subgraph.c lines 2663, 2918 and 3500, matching the issue report exactly; after the change it succeeds and produces xnnpack-subgraph.lib. Separately compiled all 96 XNNPACK logic-layer sources with XNN_LOG_LEVEL=5 both with and without /Zc:preprocessor to confirm the flag introduces no new diagnostics. Verified that cmake --preset llm-debug still disables the quantized and LLM kernels and warns under MSVC, and now enables them under -T ClangCL, where custom_ops, quantized_kernels and quantized_ops_lib all build clean. Reviewers: Co-Authored-By: Claude Opus 5 (1M context) --- backends/xnnpack/cmake/Dependencies.cmake | 9 +++++++++ tools/cmake/preset/llm.cmake | 6 ++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/backends/xnnpack/cmake/Dependencies.cmake b/backends/xnnpack/cmake/Dependencies.cmake index d02ac4a7926..fe81f63dac9 100644 --- a/backends/xnnpack/cmake/Dependencies.cmake +++ b/backends/xnnpack/cmake/Dependencies.cmake @@ -76,6 +76,15 @@ set(XNNPACK_BUILD_ALL_MICROKERNELS OFF CACHE BOOL "" ) + +# XNNPACK nests variadic macros (XNN_RETURN_IF_ERROR expands to xnn_log_error), +# which MSVC's default legacy preprocessor mis-expands. Only observable where +# XNN_LOG_LEVEL is non-zero, i.e. Debug configurations. Not applied to clang-cl, +# which sets MSVC but already has a conforming preprocessor. +if(CMAKE_C_COMPILER_ID STREQUAL "MSVC") + add_compile_options(/Zc:preprocessor) +endif() + add_subdirectory("${XNNPACK_SOURCE_DIR}") include_directories(SYSTEM ${XNNPACK_INCLUDE_DIR}) list(APPEND xnnpack_third_party XNNPACK) diff --git a/tools/cmake/preset/llm.cmake b/tools/cmake/preset/llm.cmake index d1b8064c342..67c9d09b794 100644 --- a/tools/cmake/preset/llm.cmake +++ b/tools/cmake/preset/llm.cmake @@ -20,9 +20,11 @@ set_overridable_option(EXECUTORCH_BUILD_KERNELS_OPTIMIZED ON) set_overridable_option(EXECUTORCH_BUILD_XNNPACK ON) # Turn on the quantized and LLM kernels unless on Windows with MSVC build since -# they don't currently compile. +# they don't currently compile. MSVC is also set for clang-cl, which does +# compile them, so key off the compiler ID instead. if(NOT ((CMAKE_SYSTEM_NAME STREQUAL "Windows" OR CMAKE_SYSTEM_NAME STREQUAL - "WIN32") AND MSVC) + "WIN32") + AND CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") ) set_overridable_option(EXECUTORCH_BUILD_KERNELS_QUANTIZED ON) set_overridable_option(EXECUTORCH_BUILD_KERNELS_LLM ON)