Skip to content

Support building ONNX runtime with clang on Windows for Windows ML - #29741

Open
adrastogi wants to merge 23 commits into
mainfrom
adrastogi/clang-windows
Open

adrastogi wants to merge 23 commits into
mainfrom
adrastogi/clang-windows

Conversation

@adrastogi

@adrastogi adrastogi commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Description

Enable ONNX Runtime to build with clang-cl on Windows x64 for the Windows ML flag set (--use_dml --enable_wcos --client_package_build --enable_msvc_static_runtime). Adds a --use_clang_cl build flag that selects the Visual Studio ClangCL platform toolset, and fixes the incompatibilities clang-cl surfaces that MSVC cl.exe tolerates.

Build configuration

  • build.py translates --use_clang_cl into -T ClangCL,host=.... It requires a Visual Studio generator and rejects WASM and Android builds, where the toolset would otherwise be skipped silently.
  • CMake is authoritative for the target: configuration fails unless clang-cl targets Windows x64.
  • clang-cl builds link with MSVC link.exe, through CMAKE_LINKER_TYPE=MSVC (CMake 3.29 or newer), instead of the toolset's default lld-link.
  • OneCore builds take the MSVC runtime directory from the Visual Studio instance and the selected MSVC tools version, not from a path relative to clang-cl.exe.
  • With --use_binskim_compliant_compile_flags, clang-cl skips only /Qspectre, which it does not implement. The Spectre-mitigated MSVC runtime is still selected: the /LIBPATH entry, and the OneCore runtime through onnxruntime_USE_MSVC_SPECTRE_RUNTIME.

Source and build fixes

  • MLAS (onnxruntime_mlas.cmake): the x86-64 C++ kernels are now defined once, in ISA groups with shared GNU-style flags, and both the Windows and non-MSVC paths use them. clang-cl feature-gates intrinsics, so it applies the same per-ISA flags as GCC/Clang, with a few documented exceptions. cl.exe flags, and non-MSVC source selection and flags, are unchanged.
  • spin_pause.cc: the _tpause intrinsic is isolated in a helper carrying target("waitpkg"), behind a single ORT_USE_TPAUSE macro. The runtime HasTPAUSE() guard is preserved.
  • float16.h: MLFloat16 gets operator==/!= taking the derived type, which resolves a C++20 reversed-candidate ambiguity.
  • DML EP: out-of-line AbstractOperatorDesc special members for two-phase name lookup, uint32_t → size_t template parameters to match std::array deduction, and an invalid ## token-paste removed.
  • GGUF CPU helper: clang-cl uses the scalar path, because __builtin_cpu_supports needs compiler-rt.
  • Other: a duplicate path_lib.cc translation unit is removed from onnxruntime_perf_test, and a test-only noexcept macro is adjusted.

There are no ABI changes. All clang-cl-specific paths are gated on the compiler, and MSVC build flags are unchanged.

Validation (Windows x64, VS 2022 17.14, clang-cl 19, MSVC 14.44):

  • python -m unittest tools.ci_build.test_build_args: 26 tests passed.
  • python tools/ci_build/test_onecore_spectre.py: 33 tests passed.
  • A clang-cl WinML-flag build succeeds.
    • onnxruntime_mlas_test: 32,033 tests passed.
    • onnxruntime_test_all: 2878 passed. The only failures are DML "no devices detected", because the build VM has no GPU.
    • The telemetry, no-exceptions and optional-provider ctests all pass.
  • A clang-cl build with --use_binskim_compliant_compile_flags --enable_wcos produces onnxruntime.dll. It links with link.exe and has CFG enabled, and its CRT/STL libraries come from lib\spectre\onecore\x64.
  • A CMake harness evaluating onnxruntime_mlas.cmake confirms that the MSVC and non-MSVC x86-64 sources and per-source flags are unchanged.

Motivation and Context

Building ORT with clang on Windows unblocks security-testing workflows (ASan / libFuzzer / WebNN fuzzing) that depend on Clang-only tooling.

Copilot AI and others added 4 commits July 10, 2026 00:08
Add a --use_clang_cl build.py option that selects the Visual Studio
'ClangCL' toolset (-T ClangCL,host=x64), plus the source/cmake fixes
required for the core onnxruntime.dll to compile with clang-cl:

- build_args.py: new --use_clang_cl arg + validation (VS generator only,
  not for arm/arm64/arm64ec).
- build.py: prepend 'ClangCL,' to the x64 VS toolset; guard /Qspectre
  (unsupported by clang-cl) in the binskim-compliant flag set.
- spin_pause.cc: isolate the _tpause (waitpkg) intrinsic in a
  target-attributed helper so clang-cl will emit it (MSVC unaffected).
- onnxruntime_mlas.cmake: clang-cl feature-gates x86 intrinsics (unlike
  MSVC cl.exe), so give the per-ISA x64 kernels that carry no /arch: flag
  the equivalent -m<feature> flags (mirroring the non-MSVC branch), incl.
  -msse4.1 for qgemm_kernel_sse41.cpp and -mamx-tile -mamx-int8 for
  qgemm_kernel_amx.cpp.

Relates to #29465, #1025.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b55a1989-57d8-45eb-88fb-8619f855611c
Three portability fixes let the DirectML execution provider build under
clang-cl (conforming two-phase name lookup / stricter template rules),
while remaining valid for MSVC cl.exe:

- AbstractOperatorDesc.h: OperatorField and AbstractOperatorDesc are
  mutually recursive, so OperatorField is incomplete where the struct is
  defined. Move the special member functions and the GetTensors template
  out of line, defined after OperatorField is completed (via a
  bottom-of-header include of GeneratedSchemaTypes.h). cl.exe defers these
  instantiations; clang-cl performs them at definition time and otherwise
  fails with 'incomplete type OperatorField'.
- MLOperatorAuthorImpl.cpp: drop the spurious token paste in
  'initializer.##Z()' (CASE_PROTO). '.##Z' never forms a valid token;
  clang-cl errors with -Winvalid-token-paste. 'initializer.Z()' is the
  intended member call on both compilers.
- DmlDFT.h / DmlGridSample.h: the Dispatch() non-type template parameter
  is deduced from std::array's size_type, so it must be size_t, not
  uint32_t; clang requires an exact type match for the deduced argument.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b55a1989-57d8-45eb-88fb-8619f855611c
Enable onnxruntime_test_all, onnxruntime_provider_test, and
onnxruntime_perf_test to build under clang-cl (lld-link). All fixes are
standard-conformant and behavior-preserving on MSVC cl.exe.

- float16.h: MLFloat16 defines operator==/!= on the derived type instead of
  inheriting the CRTP base version via using. Under C++20 (which clang-cl
  applies), comparing two MLFloat16 through the inherited base operator
  synthesizes a reversed candidate that is ambiguous with the non-reversed one.
  Defining a homogeneous derived-type operator== (mirroring BFloat16) resolves
  it. The delegating call Base::operator==(rhs) is qualified, so no reversed
  candidate is synthesized. Inline, ABI-neutral.

- element_wise_ops_test.cc: MATH_NO_EXCEPT now also expands to empty for clang
  targeting MSVC (defined(__clang__) && defined(_MSC_VER)). clang-cl sees the
  global-namespace UCRT math functions (::sinf, ::cosf, ...) as non-noexcept, so
  the reference template parameter float (op)(float) noexcept failed to bind.
  cl.exe and clang+Linux are unaffected.

- onnxruntime_unittests.cmake: stop compiling core/platform/path_lib.cc directly
  into onnxruntime_perf_test. onnxruntime_common (linked in both the shared and
  non-shared configurations) already provides GetDirNameFromFilePath; the direct
  compile is a duplicate definition that link.exe tolerates but lld-link rejects.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b55a1989-57d8-45eb-88fb-8619f855611c
ONNX 1.22.0's onnx/common/scoped_resource.h defines
`using ScopedHandle = ScopedResource<INVALID_HANDLE_VALUE, close_handle>`.
INVALID_HANDLE_VALUE expands to ((HANDLE)(LONG_PTR)-1), a reinterpret_cast
that is not a constant expression and therefore cannot be used as a non-type
template argument. MSVC accepts this as an extension; clang-cl rejects it as a
hard error ("non-type template argument is not a constant expression"), which
blocks clang-cl builds of ONNX Runtime against the pinned ONNX.

Extend cmake/patches/onnx/onnx.patch to replace the templated ScopedHandle
alias with an equivalent non-template ScopedHandle class (same get()/release()
API) that only references INVALID_HANDLE_VALUE at runtime. ScopedFd is left
untouched since -1 is a valid constant expression.

Validated on an RTX 5060 Ti GPU box: clang-cl and MSVC builds both compile the
patched ONNX cleanly, and onnxruntime_test_all (1824 passed / 0 failed) and
onnxruntime_provider_test (5440 tests / 0 failed) run identically under both
compilers with the DirectML EP on real hardware.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 716cc750-9984-4096-b8bb-e0f9e548ab80
@adrastogi
adrastogi marked this pull request as ready for review July 23, 2026 00:04
@adrastogi
adrastogi requested a review from Copilot July 23, 2026 00:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds first-class support for building ONNX Runtime on Windows with clang-cl (MSVC ABI), primarily targeting the Windows ML distribution configuration, by introducing a --use_clang_cl build flag and addressing compiler-conformance issues exposed by clang-cl across MLAS, DML EP, tests, and the ONNX patch set.

Changes:

  • Add --use_clang_cl build flag (Visual Studio generator + x64) and plumb through CMake toolset selection; disable MSVC-only /Qspectre when using clang-cl.
  • Fix clang-cl conformance/build issues in core code and DML EP headers (two-phase lookup/incomplete-type, template deduction, macro token-paste).
  • Add per-file MLAS ISA compile flags for clang-cl and patch ONNX to avoid a non-constant NTTP (INVALID_HANDLE_VALUE) rejected by clang-cl.
Show a summary per file
File Description
tools/ci_build/build.py Adds ClangCL toolset selection and disables /Qspectre under clang-cl.
tools/ci_build/build_args.py Introduces --use_clang_cl flag and validates generator/arch constraints.
onnxruntime/test/providers/cpu/math/element_wise_ops_test.cc Adjusts noexcept macro handling for clang-cl/libc++ behavior.
onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlGridSample.h Updates template non-type parameter to size_t for std::array deduction compatibility.
onnxruntime/core/providers/dml/DmlExecutionProvider/src/Operators/DmlDFT.h Updates template non-type parameter to size_t for std::array deduction compatibility.
onnxruntime/core/providers/dml/DmlExecutionProvider/src/MLOperatorAuthorImpl.cpp Fixes invalid ## token-paste usage that clang rejects.
onnxruntime/core/providers/dml/DmlExecutionProvider/src/External/DirectMLHelpers/AbstractOperatorDesc.h Moves special members/tensor iteration out-of-line via include ordering to satisfy two-phase lookup rules.
onnxruntime/core/common/spin_pause.cc Isolates _tpause behind a waitpkg-targeted helper for clang-cl feature gating.
include/onnxruntime/core/common/float16.h Adds derived-type operator==/!= to avoid C++20 reversed-candidate ambiguity under clang-cl.
cmake/patches/onnx/onnx.patch Patches ONNX scoped handle RAII to avoid invalid NTTP with INVALID_HANDLE_VALUE on clang-cl.
cmake/onnxruntime_unittests.cmake Removes duplicate path_lib.cc compilation that lld-link rejects (ODR).
cmake/onnxruntime_mlas.cmake Adds per-ISA -m<feature> compile flags for clang-cl MLAS kernels.

Review details

  • Files reviewed: 12/12 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment thread tools/ci_build/build_args.py Resolved
Comment thread onnxruntime/core/common/spin_pause.cc Resolved
adrastogi and others added 4 commits November 4, 2026 15:40
Resolve conflicts:
- cmake/patches/onnx/onnx.patch: take main. ONNX v1.23.2 replaces the INVALID_HANDLE_VALUE
  non-type template argument with a Traits-based ScopedResource, so the clang-cl
  scoped_resource.h hunk is no longer needed.
- onnxruntime/core/common/spin_pause.cc: keep the waitpkg-targeted TPAUSE helper for clang-cl,
  scope tpause_spin_delay_cycles to the TPAUSE branch (#33532), and make the helper static and
  noinline under clang so it cannot be inlined back into SpinPause().

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Main split the AVX-VNNI SQNBitGemm kernels into sqnbitgemm_kernel_avx2vnni.cpp and
added sqnbitgemm_kernel_avx512vnni_2bit.cpp. Give both the clang-cl ISA flags they need,
and stop passing -mavxvnni to the pure-AVX2 kernels so the auto-vectorizer cannot emit
VNNI instructions there (matching the non-MSVC branch).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The telemetry shutdown/transport tests and the no-exceptions optional provider probe link
patched copies of library sources that override the libraries' definitions. MSVC link.exe
reads all objects before searching libraries, but lld-link (clang-cl) resolves inputs in
command-line order and the Visual Studio generator passes libraries before objects, so an
earlier referencing object pulled in the original archive member and produced duplicate
symbols. List the overriding sources/objects before the objects that reference them.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The shared 2-bit packer is compiled with AVX-512 enabled, allowing clang-cl to emit unsupported instructions that can crash MatMulNBits prepacking on AVX2-only CPUs.

1 open issue
2 resolved since last review

Review effort: Balanced

Comment thread cmake/onnxruntime_mlas.cmake Resolved
- Compile sqnbitgemm_kernel_avx512_2bit.cpp without ISA flags. Its scalar packers are shared
  with the AVX2 dispatch tables, so AVX-512 flags could let clang auto-vectorize them into
  instructions that crash on AVX2-only CPUs (matches the non-MSVC branch).
- Pass -ffp-contract=off to intrinsics/avx2/q2_dq_avx2.cpp. clang-cl contracts a*b+c into FMA
  under /arch:AVX2, breaking bit-identity with the scalar 2-bit dequant kernel
  (Q2BlockwiseFpZpDequant.ShortExecute); MSVC does not contract under /fp:precise.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The x64-only validation misses implicitly selected Win32 and ARM64 targets, allowing unsupported configurations to receive the ClangCL toolset instead of failing early.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Reject unsupported host architectures for the x64-only compiler option

tools/​ci_build/​build_args.py:1126

The x64-only check misses targets selected implicitly by the host environment. With no architecture switches, build.py selects Win32 under 32-bit Python and ARM64 on native ARM64 Windows; both pass this validation and receive the ClangCL toolset despite being unsupported by this option. Please also validate the host/interpreter architecture so these configurations receive the intended early error.

1 resolved since last review

Review effort: Balanced

adrastogi and others added 2 commits November 5, 2026 07:40
Same lld-link input-order issue as the optional provider probe: list the no-exception
provider_bridge_ort.cc/helper.cc copies before the objects that reference them so clang-cl
links onnxruntime_provider_bridge_no_exceptions_test without duplicate symbols.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Without an explicit target switch, build.py derives the target from the host: Win32 under
32-bit Python and ARM64 on native ARM64 Windows. Reject those for --use_clang_cl too, and add
unit tests for the --use_clang_cl argument validation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@adrastogi

Copy link
Copy Markdown
Contributor Author

[Agentic Response]: Addressed the previously missed Copilot finding on \ ools/ci_build/build_args.py\ (x64-only validation missing implicitly selected targets) in f181107. --use_clang_cl\ now also errors when the host is not x64 or Python is 32-bit, which covers the implicit Win32 and native ARM64 targets \�uild.py\ would otherwise pick. Added unit tests in \ ools/ci_build/test_build_args.py\ for the clang-cl validation; the two new host cases fail without the fix.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Newer-host validation does not resolve compiler-dependent non-VNNI AVX-512 dispatch safety or Windows link-order risks, which need generated-code inspection and Windows fixture testing.

No open issues 🎉

Review effort: Balanced

…under clang-cl

sqnbitgemm_kernel_avx512.cpp backs MlasSQNBitGemmDispatchAvx512, which platform.cpp selects on
AVX-512 CPUs without VNNI. It uses no VNNI intrinsics, so compile it with the base AVX-512 flags;
otherwise clang could auto-vectorize VNNI instructions into code reachable on those CPUs.
q4gemm_avx512.cpp keeps -mavx512vnni because its VNNI-only Q8Q4 kernels need the intrinsics;
the generated code has VNNI instructions only in those VNNI-dispatched instantiations.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@adrastogi

Copy link
Copy Markdown
Contributor Author

[Agentic Response]: Following up on the latest Copilot summary ("needs a closer look") on f181107:

AVX-512 non-VNNI dispatch safety — fixed in 1101a88. platform.cpp selects MlasSQNBitGemmDispatchAvx512 (sqnbitgemm_kernel_avx512.cpp) and MlasFpQ4GemmDispatchAvx512 (q4gemm_avx512.cpp) on AVX-512 CPUs without VNNI, yet clang-cl compiled both with -mavx512vnni.

  • sqnbitgemm_kernel_avx512.cpp uses no VNNI intrinsics, so it now gets the base AVX-512 flags only. Its rebuilt object has 0 vpdpbusd/vpdpwssd instructions.
  • q4gemm_avx512.cpp keeps -mavx512vnni (same as the non-MSVC branch) because the VNNI-only MlasQ8Q4GemmDispatchAvx512vnni kernels call _mm256_dpbusd_epi32. Inspecting the clang-cl object shows VNNI instructions only in the four MlasQ8Q4GemmOperation<..., MLAS_FP_Q4_GEMM_KERNEL_AVX512VNNI> instantiations, which are reached only after the CPUID VNNI check. A comment in the cmake records this.
  • Related MLAS tests under clang-cl (QNBit/SQ4/SQ8/Q4Gemm/Q8Q4/2-bit/FpQ4): 19341 passed, 0 failed.

Windows link order — the reordered override sources were verified on Windows with both toolchains. clang-cl/lld-link: full build with no errors, 20/20 provider-bridge, optional-provider-probe and telemetry ctests pass. MSVC: the same targets build and their tests pass. Linux CI ctest on this PR also passes; the current Linux x64 and Windows CUDA failures (TestBeamSearchT5, CUDA test job) also occur on main.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues block the clang-cl build support changes.

No open issues 🎉

Review effort: Balanced

adrastogi and others added 3 commits November 5, 2026 15:25
Resolve tools/ci_build/test_build_args.py: keep the --use_clang_cl validation tests and drop the ACL deprecation tests that main removed along with the ACL EP (#33851).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@edgchen1 Edward Chen (edgchen1) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eight separate inline comments: two correctness findings and six non-blocking suggestions/questions.

Validation: 23 build-argument tests passed; the changed spin-pause translation unit compiled with MSVC for x64 and ARM64. No full clang-cl build or DirectML runtime tests were run.

🤖 Written by GitHub Copilot on behalf of Edward Chen (@edgchen1).

Comment thread tools/ci_build/build.py Resolved
Comment thread tools/ci_build/build.py Resolved
Comment thread tools/ci_build/build_args.py Resolved
Comment thread tools/ci_build/build.py Resolved
Comment thread onnxruntime/core/common/spin_pause.cc Resolved
Comment thread cmake/onnxruntime_unittests.cmake Resolved
Comment thread cmake/onnxruntime_telemetry_tests.cmake Resolved
Comment thread cmake/onnxruntime_mlas.cmake Resolved
// ambiguity on conforming compilers such as clang-cl ("use of overloaded operator '==' ... is
// ambiguous"); a derived-typed operator== resolves cleanly on both clang-cl and MSVC.
bool operator==(const MLFloat16& rhs) const noexcept { return Base::operator==(rhs); }
bool operator!=(const MLFloat16& rhs) const noexcept { return Base::operator!=(rhs); }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why is this true for integrating clang on windows also? Also why not for <?

}

#ifdef _LIBCPP_VERSION
#if defined(_LIBCPP_VERSION) || (defined(__clang__) && defined(_MSC_VER))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is because clang on windows implicitly uses the llvm libc++ implementation? Should this be a single variable like ORT_USING_LIBCPP and replaced wholistically?

adrastogi and others added 6 commits November 12, 2026 10:14
- Resolve the OneCore MSVC runtime directory from the VS instance and selected
  MSVC tools version when compiling with clang-cl, instead of relative to
  clang-cl.exe (which lives under VC/Tools/Llvm). Previously this silently
  linked the desktop CRT; after #33139 it failed configuration.
- Order the telemetry SDK lifetime test's instrumented objects ahead of the
  test source for lld-link.
- Exclude clang-cl from the GGUF AVX2 path: __builtin_cpu_supports needs
  compiler-rt's __cpu_model, which clang-cl does not link.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The Visual Studio ClangCL toolset defaults to lld-link. Select the MSVC linker
via CMAKE_LINKER_TYPE (CMake 3.29+), which binary analysis tooling supports, and
drop the lld-link-specific input-order workarounds in the test targets.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Define the x86-64 C++ kernel sources once, grouped by the instruction set they
are compiled for, along with the GNU-style target-feature flags for each
group. The Windows and non-MSVC x86-64 paths both select sources from these
groups; clang-cl applies the same flags as GCC/Clang, keeping only genuine
compiler exceptions local. cl.exe flags and non-MSVC source selection and
compile flags are unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Keep Spectre-mitigated MSVC runtime selection for clang-cl: skip only
  /Qspectre, keep the Spectre library search path and warning, and request
  the Spectre OneCore runtime via onnxruntime_USE_MSVC_SPECTRE_RUNTIME.
- Validate clang-cl targets in CMake (Windows x64 only), so direct CMake
  invocations are covered; build.py keeps only the toolset translation, the
  Visual Studio generator requirement and native-Windows-only checks.
- Build the Visual Studio toolset string in its final order.
- Use a single ORT_USE_TPAUSE macro in spin_pause.cc.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The validation accepts clang-cl requests for ARM-family targets, including --buildasx combinations, but the build omits the ClangCL toolset, silently selecting cl.exe and bypassing CMake’s unsupported-target check.

1 open issue

Review effort: Balanced

Comment thread tools/ci_build/build_args.py Resolved
- Pass -T ClangCL on the ARM/ARM64/ARM64EC Visual Studio path too, so
  --use_clang_cl with an ARM target fails configuration instead of silently
  building with cl.exe.
- Raise the CMake timeout in test_onecore_spectre.py to 120s; a cold CI runner
  exceeded 30s on the first configure.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The build configuration leaves the Spectre runtime enabled in CMake’s cache after BinSkim is disabled or AddressSanitizer is enabled, unnecessarily requiring Spectre OneCore libraries unless the option is explicitly set on every configure.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread tools/ci_build/build.py
Comment on lines +1428 to +1429
if use_msvc_spectre_runtime:
temp_cmake_args += ["-Donnxruntime_USE_MSVC_SPECTRE_RUNTIME=ON"]

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants