Skip to content
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.

Sign up for GitHub

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

Manually add optimization flag for Android Release builds. #12390

Merged
merged 2 commits into from
Aug 1, 2022
Merged

Manually add optimization flag for Android Release builds. #12390

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8cdc15
Manually add optimization flag for Android Release builds.
edgchen1 Jul 29, 2022
fc1ba0c
Merge remote-tracking branch 'origin/master' into edgchen1/pass_optim…
edgchen1 Aug 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cmake/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,16 @@ if (onnxruntime_EXTENDED_MINIMAL_BUILD AND NOT onnxruntime_MINIMAL_BUILD)
set(onnxruntime_MINIMAL_BUILD ON)
endif()

# work around Android NDK bug which doesn't include -O flag for Release configuration
# https://github.com/android/ndk/issues/1740
# TODO: remove this when the NDK version(s) we support get fixed
if (CMAKE_SYSTEM_NAME STREQUAL "Android")
# NB: attempting to match the effects of this fix: https://android-review.googlesource.com/c/platform/ndk/+/2168845
string(APPEND CMAKE_C_FLAGS_RELEASE " -O3")
Copy link
Member

Choose a reason for hiding this comment

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

CMAKE has 4 build types. Your this change only impacts one of them: "release". So it may not be enough.

You can also build it in a way like:

https://github.com/microsoft/onnxruntime/blob/master/tools/ci_build/github/azure-pipelines/templates/c-api-linux-cpu.yml#L61

docker run --rm -e CFLAGS="${{parameters.OnnxruntimeCFlags}}" -e CXXFLAGS="${{parameters.OnnxruntimeCXXFlags}}"

In this way it will override the default values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Release" is the one we want to fix, so it is the only one that is updated here.

My understanding is that CXXFLAGS will initialize CMAKE_CXX_FLAGS and CMAKE_CXX_FLAGS_RELEASE values are passed after CMAKE_CXX_FLAGS.

string(APPEND CMAKE_CXX_FLAGS_RELEASE " -O3")
string(APPEND CMAKE_ASM_FLAGS_RELEASE " -O3")
endif()

# Enable space optimization for gcc/clang
# Cannot use "-ffunction-sections -fdata-sections" if we enable bitcode (iOS)
if (NOT MSVC AND NOT onnxruntime_ENABLE_BITCODE)
Expand Down