Fix Clang + libc++ compilation and portability issues#28049
Fix Clang + libc++ compilation and portability issues#28049mustjab wants to merge 3 commits intomicrosoft:mainfrom
Conversation
543449a to
5e8c7bf
Compare
There was a problem hiding this comment.
Pull request overview
This PR targets cross-toolchain portability, primarily fixing build issues when compiling ONNX Runtime with Clang/libc++ (e.g., Chromium toolchains) and clang-cl on Windows ARM64.
Changes:
- Adds/adjusts portability shims (export visibility, math constants header, Windows stacktrace gating, path handling,
overridefixups). - Moves several destructors/constructors out-of-line to satisfy libc++ requirements around
std::unique_ptrto incomplete types. - Updates ARM64/NEON + AArch64 asm macro guards to avoid MSVC-only intrinsics/ELF directives under clang-cl/COFF.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| onnxruntime/core/session/plugin_ep/ep_plugin_provider_interfaces.h | Declares out-of-line FusedNodeState destructor and adds move assignment to support libc++. |
| onnxruntime/core/session/plugin_ep/ep_plugin_provider_interfaces.cc | Defines FusedNodeState destructor out-of-line. |
| onnxruntime/core/session/inference_session.cc | Uses std::filesystem::path for ifstream construction for broader toolchain compatibility. |
| onnxruntime/core/providers/cpu/tensor/gelu.cc | Adds math_constants.h include (portability-related). |
| onnxruntime/core/providers/cpu/signal/window_functions.cc | Adds math_constants.h include and replaces M_PI usage. |
| onnxruntime/core/providers/cpu/signal/dft.cc | Adds math_constants.h include and replaces M_PI usage. |
| onnxruntime/core/providers/cpu/ml/ml_common.h | Adds math_constants.h include and replaces M_PI/M_SQRT2 usage. |
| onnxruntime/core/providers/cpu/math/cumsum.cc | Fixes incorrect ORT_ENFORCE usage for exclusive attribute validation. |
| onnxruntime/core/platform/windows/stacktrace.cc | Disables <stacktrace> path under libc++ to avoid build issues. |
| onnxruntime/core/platform/windows/env.h | Adds missing override to match base class interface. |
| onnxruntime/core/optimizer/stft_decomposition.cc | Adds math_constants.h include and replaces M_PI usage in weight generation. |
| onnxruntime/core/optimizer/qdq_transformer/selectors_actions/shared/utils.h | Moves ctor/dtor declarations out-of-line for libc++ incomplete-type rules. |
| onnxruntime/core/optimizer/qdq_transformer/selectors_actions/shared/utils.cc | Adds out-of-line ctor/dtor definitions for selector-related types. |
| onnxruntime/core/optimizer/optimizer_execution_frame.h | Moves OptimizerExecutionFrame::Info destructor out-of-line for libc++. |
| onnxruntime/core/optimizer/optimizer_execution_frame.cc | Defines OptimizerExecutionFrame::Info destructor out-of-line. |
| onnxruntime/core/optimizer/gelu_fusion.cc | Adds math_constants.h include and replaces M_SQRT2 usage. |
| onnxruntime/core/mlas/lib/qladd.h | Expands NEON fallback path to include clang under Windows. |
| onnxruntime/core/mlas/lib/mlasi.h | Guards MSVC-specific ARM64 NEON aliases when building with clang. |
| onnxruntime/core/mlas/lib/activate.cpp | Avoids MSVC-only NEON intrinsic path under clang on Windows. |
| onnxruntime/core/mlas/lib/aarch64/asmmacro.h | Adds _WIN32 case to avoid ELF-only .type directive on COFF. |
| onnxruntime/core/graph/graph.cc | Moves Node ctor/dtor (and another ctor) out-of-line for libc++ incomplete-type rules. |
| onnxruntime/core/graph/ep_api_types.h | Declares out-of-line EpNode::SubgraphState destructor for libc++. |
| onnxruntime/core/graph/ep_api_types.cc | Defines EpNode::SubgraphState destructor out-of-line. |
| onnxruntime/core/common/math_constants.h | Introduces portable M_* math constant definitions when missing. |
| onnxruntime/contrib_ops/cpu/bert/bias_gelu.cc | Adds math_constants.h include and replaces M_SQRT1_2 usage. |
| include/onnxruntime/core/session/onnxruntime_c_api.h | Broadens ORT_EXPORT visibility attribute usage to GCC/Clang (non-Windows). |
| include/onnxruntime/core/graph/graph.h | Declares Node ctor/dtor and one ctor out-of-line to match graph.cc. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5e8c7bf to
aaafb13
Compare
|
@microsoft-github-policy-service agree company="Microsoft"
@microsoft-github-policy-service agree company="Microsoft" |
Description
Fixes compilation of ONNX Runtime with Clang/libc++ toolchains (e.g. Chromium's build system) and clang-cl on ARM64 Windows.
Portability fixes
math_constants.hwith M_PI, M_SQRT2, M_SQRT1_2 for libc++ (which does not define these by default)__attribute__((visibility(default)))forORT_EXPORTon GCC/ClangTargetConditionals.hinclude outside namespace for Clang modulesNode/Modeldestructors for libc++unique_ptrwith incomplete typesgsl::narrowalias foronnxruntime::narrowinnarrow.h[[maybe_unused]]on catch variables for-Wunused-variablereverseattribute validation incumsum.cc(string literal as condition)math_constants.hin all signal/optimizer/contrib filesARM64 MLAS fixes (clang-cl on Windows)
neon_fmaxv,neon_fminv) with!defined(__clang__)inmlasi.h_exfallbacks inqladd.hvcleq_z_f32_exwith!defined(__clang__)inactivate.cpp_WIN32case inasmmacro.hto skip ELF.typedirective on COFFMotivation
These changes enable building ORT from source with Chromium's Clang/libc++ toolchain on all platforms (Windows x64/ARM64, Linux x64, Mac x64/ARM64).