-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Update to C++20 #27178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Update to C++20 #27178
Changes from all commits
5884637
98c31ae
9171d14
92b3334
831ae91
1e2ad83
2213158
dad1617
7f34d3b
ad1aab8
848c8b1
2c78745
a035d7f
d339390
06fec4f
9b1cdb1
7ec3538
79de47a
2ef0bdb
270f411
ec5d52e
4d22c08
282545b
ee25321
7cc02cf
9c21b3f
f13bee2
5543636
1a8f897
f2fcee0
5873aee
99f245f
042b504
bacffa2
b4aca51
e6297ff
6d7ff32
8dbb80f
6f801b2
f5c4222
294dbd3
bddc3ac
73019d4
d3b2ff9
90b2c73
0d613a3
291f0a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -530,93 +530,43 @@ jobs: | |
| --cmake_extra_defines onnxruntime_BUILD_UNIT_TESTS=OFF | ||
|
|
||
| # Job 7: Extended minimal build with NNAPI EP for Android(arm64-v8a) and skip tests. | ||
| # NOTE: Keeping this as direct docker run due to custom volume mounts needed for Android SDK/NDK | ||
| build_extended_minimal_android: | ||
| name: 7. Build Extended Minimal (Android NNAPI) | ||
| needs: build_full_ort # Depends on Job 1 for test data | ||
| runs-on: [ | ||
| "self-hosted", | ||
| "1ES.Pool=onnxruntime-github-Ubuntu2204-AMD-CPU", | ||
| "JobId=build_extended_minimal_android-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}" | ||
| ] | ||
| permissions: # Permissions needed for build-docker-image | ||
| contents: read | ||
| packages: write | ||
| id-token: write # If using OIDC for ACR login | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| submodules: false | ||
| - uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: 20 | ||
| - name: Download Test Data Artifact | ||
| uses: actions/download-artifact@v7 | ||
| with: | ||
| name: test_data | ||
| path: ${{ runner.temp }}/.test_data/ | ||
|
|
||
| - name: Get Docker Image using Action | ||
| uses: microsoft/onnxruntime-github-actions/build-docker-image@v0.0.9 | ||
| id: build_docker_image_step | ||
| with: | ||
| dockerfile: ${{ github.workspace }}/tools/ci_build/github/linux/docker/inference/x86_64/default/cpu/Dockerfile | ||
| image-name: ghcr.io/microsoft/onnxruntime/onnxruntimecpubuildcix64 | ||
| push: true | ||
| azure-container-registry-name: onnxruntimebuildcache | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Setup Android NDK | ||
| uses: ./.github/actions/setup-android-ndk | ||
| with: | ||
| ndk-version: 28.0.13004108 | ||
| # Use default android-sdk-root if not specified | ||
|
|
||
| - name: Run Build 7 (Using docker run) | ||
| - name: Run Build 7 | ||
| shell: bash | ||
| run: | | ||
| # Create the target dir for build output inside the runner's temp dir first | ||
| mkdir -p ${{ runner.temp }}/7 | ||
|
|
||
| # Ensure ANDROID_NDK_HOME is available and get its real path | ||
| if [ -z "$ANDROID_NDK_HOME" ]; then | ||
| echo "ANDROID_NDK_HOME is not set." | ||
| exit 1 | ||
| fi | ||
| NDK_HOME_REALPATH=$(realpath $ANDROID_NDK_HOME) | ||
|
|
||
| # Ensure ANDROID_HOME is available | ||
| if [ -z "$ANDROID_HOME" ]; then | ||
| echo "ANDROID_HOME is not set. Using default /usr/local/lib/android/sdk" | ||
| export ANDROID_HOME=/usr/local/lib/android/sdk | ||
| fi | ||
|
|
||
| docker run --rm \ | ||
| --volume ${{ env.BUILD_SOURCES_DIRECTORY }}:/onnxruntime_src \ | ||
| --volume ${{ runner.temp }}:/build \ | ||
| --volume $ANDROID_HOME:/android_home \ | ||
| --volume $NDK_HOME_REALPATH:/ndk_home \ | ||
| -e ALLOW_RELEASED_ONNX_OPSET_ONLY=1 \ | ||
| -e NIGHTLY_BUILD=1 -e RUNNER_TEMP=/build \ | ||
| ${{ steps.build_docker_image_step.outputs.full-image-name }} \ | ||
| bash -c "python3 -m pip install -r /onnxruntime_src/tools/ci_build/requirements/pybind/requirements.txt \ | ||
| && python3 /onnxruntime_src/tools/ci_build/build.py \ | ||
| --build_dir /build/7 \ | ||
| python3 ./tools/ci_build/build.py \ | ||
| --build_dir ./build.extended_minimal.nnapi \ | ||
| --cmake_generator Ninja \ | ||
| --config MinSizeRel \ | ||
| --skip_submodule_sync \ | ||
| --parallel --use_binskim_compliant_compile_flags \ | ||
| --android \ | ||
| --android_sdk_path /android_home \ | ||
| --android_ndk_path /ndk_home \ | ||
| --android_sdk_path "$ANDROID_HOME" \ | ||
| --android_ndk_path "$ANDROID_NDK_HOME" \ | ||
|
Comment on lines
560
to
+563
|
||
| --android_abi=arm64-v8a \ | ||
| --android_api=29 \ | ||
| --use_nnapi \ | ||
| --minimal_build extended \ | ||
| --build_shared_lib \ | ||
| --disable_ml_ops \ | ||
| --disable_exceptions \ | ||
| --skip_tests" | ||
| --skip_tests | ||
| working-directory: ${{ env.BUILD_SOURCES_DIRECTORY }} | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,6 +10,24 @@ cmake_policy(SET CMP0104 OLD) | |||||||||||||
| # Project | ||||||||||||||
| project(onnxruntime C CXX ASM) | ||||||||||||||
|
|
||||||||||||||
| # Set C/C++ standard versions | ||||||||||||||
| if (NOT CMAKE_C_STANDARD) | ||||||||||||||
| # Needed for Java | ||||||||||||||
| set(CMAKE_C_STANDARD 99) | ||||||||||||||
| endif() | ||||||||||||||
|
|
||||||||||||||
| if (NOT CMAKE_CXX_STANDARD) | ||||||||||||||
| set(CMAKE_CXX_STANDARD 20) | ||||||||||||||
| endif() | ||||||||||||||
|
|
||||||||||||||
| # We don't use C++20 modules yet. | ||||||||||||||
| # There are some known issues to address first: | ||||||||||||||
| # - Android builds from Linux Docker containers have trouble finding clang-scan-deps. | ||||||||||||||
| # - The MSVC /permissive option is needed for compiling some of the CUDA EP code which uses CUTLASS. | ||||||||||||||
| # This option is not compatible with C++20 modules. | ||||||||||||||
| # So we will skip module scanning for now. | ||||||||||||||
| set(CMAKE_CXX_SCAN_FOR_MODULES OFF) | ||||||||||||||
|
|
||||||||||||||
| # Disable fast-math for Intel oneAPI compiler | ||||||||||||||
| if("${CMAKE_CXX_COMPILER_ID}" MATCHES "IntelLLVM") | ||||||||||||||
| if("${CMAKE_CXX_COMPILER_ID}" MATCHES "MSVC-like") | ||||||||||||||
|
|
@@ -21,11 +39,6 @@ if("${CMAKE_CXX_COMPILER_ID}" MATCHES "IntelLLVM") | |||||||||||||
| endif() | ||||||||||||||
| endif() | ||||||||||||||
|
|
||||||||||||||
| # Needed for Java | ||||||||||||||
| if (NOT CMAKE_CXX_STANDARD) | ||||||||||||||
| set(CMAKE_C_STANDARD 99) | ||||||||||||||
| endif() | ||||||||||||||
|
|
||||||||||||||
| include(CheckCXXCompilerFlag) | ||||||||||||||
| include(CheckLanguage) | ||||||||||||||
| include(CMakeDependentOption) | ||||||||||||||
|
|
@@ -34,15 +47,6 @@ include(CheckFunctionExists) | |||||||||||||
| include(CheckSymbolExists) | ||||||||||||||
| include(GNUInstallDirs) # onnxruntime_providers_* require CMAKE_INSTALL_* variables | ||||||||||||||
|
|
||||||||||||||
| if (NOT CMAKE_CXX_STANDARD) | ||||||||||||||
| # TODO: update this once all system adapt c++20 | ||||||||||||||
| if (CMAKE_SYSTEM_NAME STREQUAL "Darwin") | ||||||||||||||
| set(CMAKE_CXX_STANDARD 20) | ||||||||||||||
| else() | ||||||||||||||
| set(CMAKE_CXX_STANDARD 17) | ||||||||||||||
| endif() | ||||||||||||||
| endif() | ||||||||||||||
|
|
||||||||||||||
| if (MSVC) | ||||||||||||||
| # Make sure Visual Studio sets __cplusplus macro correctly: https://learn.microsoft.com/en-us/cpp/build/reference/zc-cplusplus | ||||||||||||||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Zc:__cplusplus") | ||||||||||||||
|
|
@@ -1476,7 +1480,7 @@ configure_file(onnxruntime_config.h.in ${CMAKE_CURRENT_BINARY_DIR}/onnxruntime_c | |||||||||||||
| get_property(onnxruntime_GENERATOR_IS_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) | ||||||||||||||
|
|
||||||||||||||
| if (onnxruntime_USE_CUDA) | ||||||||||||||
| set(CMAKE_CUDA_STANDARD 17) | ||||||||||||||
| set(CMAKE_CUDA_STANDARD ${CMAKE_CXX_STANDARD}) | ||||||||||||||
|
||||||||||||||
| set(CMAKE_CUDA_STANDARD ${CMAKE_CXX_STANDARD}) | |
| if (CMAKE_CUDA_COMPILER_VERSION VERSION_LESS 12) | |
| set(CMAKE_CUDA_STANDARD 17) | |
| else() | |
| set(CMAKE_CUDA_STANDARD ${CMAKE_CXX_STANDARD}) | |
| endif() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ onnxruntime_fetchcontent_declare( | |
| URL ${DEP_URL_cutlass} | ||
| URL_HASH SHA1=${DEP_SHA1_cutlass} | ||
|
||
| EXCLUDE_FROM_ALL | ||
| PATCH_COMMAND ${Patch_EXECUTABLE} --ignore-whitespace -p1 < ${PROJECT_SOURCE_DIR}/patches/cutlass/cutlass_4.2.1.patch | ||
| PATCH_COMMAND ${Patch_EXECUTABLE} --ignore-whitespace -p1 < ${PROJECT_SOURCE_DIR}/patches/cutlass/cutlass_4.2.1.patch | ||
| ) | ||
|
|
||
| FetchContent_GetProperties(cutlass) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -379,8 +379,8 @@ class GPUProfilerBase : public EpProfiler { | |
| void MergeEvents(std::map<uint64_t, Events>& events_to_merge, Events& events) { | ||
| Events merged_events; | ||
|
|
||
| auto event_iter = std::make_move_iterator(events.begin()); | ||
| auto event_end = std::make_move_iterator(events.end()); | ||
| auto event_iter = events.begin(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: |
||
| auto event_end = events.end(); | ||
| for (auto& map_iter : events_to_merge) { | ||
| if (map_iter.second.empty()) { | ||
| continue; | ||
|
|
@@ -395,7 +395,7 @@ class GPUProfilerBase : public EpProfiler { | |
| (event_iter->ts == ts && | ||
| (event_iter + 1) != event_end && | ||
| (event_iter + 1)->ts == ts))) { | ||
| merged_events.emplace_back(*event_iter); | ||
| merged_events.emplace_back(*std::make_move_iterator(event_iter)); | ||
| ++event_iter; | ||
| } | ||
|
|
||
|
|
@@ -409,7 +409,7 @@ class GPUProfilerBase : public EpProfiler { | |
| copy_op_names = true; | ||
| op_name = event_iter->args["op_name"]; | ||
| parent_name = event_iter->name; | ||
| merged_events.emplace_back(*event_iter); | ||
| merged_events.emplace_back(*std::make_move_iterator(event_iter)); | ||
| ++event_iter; | ||
| } | ||
|
|
||
|
|
@@ -428,7 +428,9 @@ class GPUProfilerBase : public EpProfiler { | |
| } | ||
|
|
||
| // move any remaining events | ||
| merged_events.insert(merged_events.end(), event_iter, event_end); | ||
| merged_events.insert(merged_events.end(), | ||
| std::make_move_iterator(event_iter), | ||
| std::make_move_iterator(event_end)); | ||
| std::swap(events, merged_events); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's an error with CMake not finding
clang-scan-depsfrom the NDK for the particular configuration of:it works fine when running outside of Docker. this is what the other Android CI builds do, e.g., here:
onnxruntime/.github/workflows/android.yml
Line 194 in a5dc0f9
I just simplified this to make it consistent instead of debugging further for now.