From efcbdac58e542273c7cd42f5e5bcd01c0218f0e3 Mon Sep 17 00:00:00 2001 From: Changming Sun Date: Mon, 7 Nov 2022 09:06:28 -0800 Subject: [PATCH] Remove the cmake option: onnxruntime_DEV_MODE (#13573) 1. Remove the cmake option onnxruntime_DEV_MODE and replace it with "--compile-no-warning-as-error" 2. Suppress some GSL warnings because now we treat nvcc diag warnings as errors --- .pipelines/windowsai-steps.yml | 6 +-- cmake/CMakeLists.txt | 74 ++++++++++++------------------ cmake/external/gsl.cmake | 14 +++++- cmake/onnxruntime_objectivec.cmake | 4 -- cmake/onnxruntime_providers.cmake | 27 ++++++----- cmake/patches/gsl/1064.patch | 26 +++++++++++ tools/ci_build/build.py | 29 +++++++----- 7 files changed, 101 insertions(+), 79 deletions(-) create mode 100644 cmake/patches/gsl/1064.patch diff --git a/.pipelines/windowsai-steps.yml b/.pipelines/windowsai-steps.yml index bed9c9a2f4a15..aa9ec339c7b2a 100644 --- a/.pipelines/windowsai-steps.yml +++ b/.pipelines/windowsai-steps.yml @@ -76,11 +76,11 @@ jobs: # must call vsdevcmd first to add cmake to PATH - script: | - curl -O -L https://github.com/Kitware/CMake/releases/download/v3.22.2/cmake-3.22.2-windows-x86_64.zip - 7z x cmake-3.22.2-windows-x86_64.zip + curl -O -L https://github.com/Kitware/CMake/releases/download/v3.24.3/cmake-3.24.3-windows-x86_64.zip + 7z x cmake-3.24.3-windows-x86_64.zip set PYTHONHOME=$(Build.BinariesDirectory)\${{ parameters.PythonPackageName }}.3.7.9\tools set PYTHONPATH=$(Build.BinariesDirectory)\${{ parameters.PythonPackageName }}.3.7.9\tools - $(Build.BinariesDirectory)\${{ parameters.PythonPackageName }}.3.7.9\tools\python.exe "$(Build.SourcesDirectory)\tools\ci_build\build.py" --build_dir $(Build.BinariesDirectory) --build_shared_lib --enable_onnx_tests --ms_experimental --use_dml --use_winml --cmake_generator "Visual Studio 16 2019" --update --config RelWithDebInfo --enable_lto --use_telemetry --disable_rtti --enable_wcos $(BuildFlags) --cmake_extra_defines CMAKE_SYSTEM_VERSION=10.0.19041.0 --cmake_path $(Build.BinariesDirectory)\cmake-3.22.2-windows-x86_64\bin\cmake.exe --ctest_path $(Build.BinariesDirectory)\cmake-3.22.2-windows-x86_64\bin\ctest.exe + $(Build.BinariesDirectory)\${{ parameters.PythonPackageName }}.3.7.9\tools\python.exe "$(Build.SourcesDirectory)\tools\ci_build\build.py" --build_dir $(Build.BinariesDirectory) --build_shared_lib --enable_onnx_tests --ms_experimental --use_dml --use_winml --cmake_generator "Visual Studio 16 2019" --update --config RelWithDebInfo --enable_lto --use_telemetry --disable_rtti --enable_wcos $(BuildFlags) --cmake_extra_defines CMAKE_SYSTEM_VERSION=10.0.19041.0 --cmake_path $(Build.BinariesDirectory)\cmake-3.24.3-windows-x86_64\bin\cmake.exe --ctest_path $(Build.BinariesDirectory)\cmake-3.24.3-windows-x86_64\bin\ctest.exe workingDirectory: '$(Build.BinariesDirectory)' displayName: 'Generate cmake config' diff --git a/cmake/CMakeLists.txt b/cmake/CMakeLists.txt index 345d8af1e0938..693aac844eb1d 100644 --- a/cmake/CMakeLists.txt +++ b/cmake/CMakeLists.txt @@ -57,7 +57,6 @@ option(onnxruntime_USE_NNAPI_BUILTIN "Build with builtin NNAPI lib for Android N option(onnxruntime_USE_SNPE "Build with SNPE support" OFF) option(onnxruntime_USE_RKNPU "Build with RKNPU support" OFF) option(onnxruntime_USE_DNNL "Build with DNNL support" OFF) -option(onnxruntime_DEV_MODE "Enable developer warnings and treat most of them as error." OFF) option(onnxruntime_BUILD_UNIT_TESTS "Build ONNXRuntime unit tests" ON) option(onnxruntime_BUILD_CSHARP "Build C# library" OFF) option(onnxruntime_BUILD_OBJC "Build Objective-C library" OFF) @@ -482,12 +481,10 @@ if (onnxruntime_DISABLE_EXCEPTIONS) if (MSVC) string(REGEX REPLACE "/EHsc" "/EHs-c-" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") - if (onnxruntime_DEV_MODE) - # Eigen throw_std_bad_alloc calls 'new' instead of throwing which results in a nodiscard warning. - # It also has unreachable code as there's no good way to avoid EIGEN_EXCEPTIONS being set in macros.h - # TODO: see if we can limit the code this is disabled for. - string(APPEND CMAKE_CXX_FLAGS " /wd4834 /wd4702") - endif() + # Eigen throw_std_bad_alloc calls 'new' instead of throwing which results in a nodiscard warning. + # It also has unreachable code as there's no good way to avoid EIGEN_EXCEPTIONS being set in macros.h + # TODO: see if we can limit the code this is disabled for. + string(APPEND CMAKE_CXX_FLAGS " /wd4834 /wd4702") add_compile_definitions("_HAS_EXCEPTIONS=0") else() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables") @@ -607,13 +604,12 @@ if (MSVC) if (NOT onnxruntime_DISABLE_EXCEPTIONS) string(APPEND CMAKE_CXX_FLAGS " /EHsc") string(APPEND CMAKE_C_FLAGS " /EHsc") - if (onnxruntime_DEV_MODE) - string(APPEND CMAKE_CXX_FLAGS " /wd26812") - string(APPEND CMAKE_C_FLAGS " /wd26812") - # warning C4805: '|': unsafe mix of type 'uintptr_t' and type 'bool' in operation (from c10/core/TensorImpl.h) - if (onnxruntime_ENABLE_EAGER_MODE) - string(APPEND CMAKE_CXX_FLAGS " /wd4805") - endif() + + string(APPEND CMAKE_CXX_FLAGS " /wd26812") + string(APPEND CMAKE_C_FLAGS " /wd26812") + # warning C4805: '|': unsafe mix of type 'uintptr_t' and type 'bool' in operation (from c10/core/TensorImpl.h) + if (onnxruntime_ENABLE_EAGER_MODE) + string(APPEND CMAKE_CXX_FLAGS " /wd4805") endif() endif() string(APPEND CMAKE_CXX_FLAGS " /experimental:external /external:W0 /external:templates- /external:I ${CMAKE_CURRENT_SOURCE_DIR} /external:I ${CMAKE_CURRENT_BINARY_DIR}") @@ -867,7 +863,7 @@ if (UNIX AND onnxruntime_ENABLE_LTO AND NOT onnxruntime_PREFER_SYSTEM_LIB) target_link_options(protoc PRIVATE "-Wl,--no-as-needed") endif() -if (MSVC AND onnxruntime_DEV_MODE AND NOT onnxruntime_PREFER_SYSTEM_LIB) +if (MSVC AND NOT onnxruntime_PREFER_SYSTEM_LIB) target_compile_options(${PROTOBUF_LIB} PRIVATE "/wd5054") endif() @@ -1068,29 +1064,18 @@ if (WIN32) # parallel build # These compiler opitions cannot be forwarded to NVCC, so cannot use add_compiler_options string(APPEND CMAKE_CXX_FLAGS " /MP") - if (onnxruntime_DEV_MODE) - # class needs to have dll-interface to be used by clients - list(APPEND ORT_WARNING_FLAGS "/wd4251") - # issued by thrust nonstandard extension used: nameless struct/union - list(APPEND ORT_WARNING_FLAGS "/wd4201") - # warning C4800: Implicit conversion from 'X' to bool. Possible information loss - if (onnxruntime_USE_OPENVINO OR onnxruntime_ENABLE_EAGER_MODE) - list(APPEND ORT_WARNING_FLAGS "/wd4800") - endif() - # operator 'operator-name': deprecated between enumerations of different types - list(APPEND ORT_WARNING_FLAGS "/wd5054") - # Enable warning: data member 'member' will be initialized after data member 'member2' / base class 'base_class' - list(APPEND ORT_WARNING_FLAGS "/w15038") - # Treat warning as error if onnxruntime_DEV_MODE is ON - # For cross-compiled ARM64 binaries, there are too many warnings to fix, hence ignore warnings for now - if (onnxruntime_DEV_MODE AND NOT CMAKE_CROSSCOMPILING) - # treat warnings as errors - list(APPEND ORT_WARNING_FLAGS "/WX") - foreach(type EXE STATIC SHARED) - set(CMAKE_${type}_LINKER_FLAGS "${CMAKE_${type}_LINKER_FLAGS} /WX") - endforeach() - endif() + # class needs to have dll-interface to be used by clients + list(APPEND ORT_WARNING_FLAGS "/wd4251") + # issued by thrust nonstandard extension used: nameless struct/union + list(APPEND ORT_WARNING_FLAGS "/wd4201") + # warning C4800: Implicit conversion from 'X' to bool. Possible information loss + if (onnxruntime_USE_OPENVINO OR onnxruntime_ENABLE_EAGER_MODE) + list(APPEND ORT_WARNING_FLAGS "/wd4800") endif() + # operator 'operator-name': deprecated between enumerations of different types + list(APPEND ORT_WARNING_FLAGS "/wd5054") + # Enable warning: data member 'member' will be initialized after data member 'member2' / base class 'base_class' + list(APPEND ORT_WARNING_FLAGS "/w15038") # set linker flags to minimize the binary size. if (MSVC) @@ -1272,6 +1257,11 @@ endif() function(onnxruntime_set_compile_flags target_name) target_compile_definitions(${target_name} PUBLIC EIGEN_USE_THREADS) + set_target_properties(${target_name} PROPERTIES COMPILE_WARNING_AS_ERROR ON) + if (onnxruntime_USE_CUDA) + # Suppress a "conversion_function_not_usable" warning in gsl/span + target_compile_options(${target_name} PRIVATE "$<$:SHELL:--diag-suppress 554>") + endif() if (MSVC) foreach(CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORY ${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}) target_compile_options(${target_name} PRIVATE "$<$>:/external:I${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORY}>") @@ -1292,14 +1282,10 @@ function(onnxruntime_set_compile_flags target_name) # Enable warning target_compile_options(${target_name} PRIVATE "$<$:SHELL:--compiler-options -Wall>" "$<$>:-Wall>") target_compile_options(${target_name} PRIVATE "$<$>:-Wextra>") - if (onnxruntime_DEV_MODE) - if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") - #external/protobuf/src/google/protobuf/arena.h:445:18: error: unused parameter 'p' - target_compile_options(${target_name} PRIVATE "-Wno-unused-parameter") - endif() - target_compile_options(${target_name} PRIVATE "$<$:SHELL:--compiler-options -Werror>" "$<$>:-Werror>") + if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") + #external/protobuf/src/google/protobuf/arena.h:445:18: error: unused parameter 'p' + target_compile_options(${target_name} PRIVATE "-Wno-unused-parameter") endif() - target_compile_definitions(${target_name} PUBLIC -DNSYNC_ATOMIC_CPP11) target_include_directories(${target_name} PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/external/nsync/public") endif() diff --git a/cmake/external/gsl.cmake b/cmake/external/gsl.cmake index 464c2256bda60..b9ab273601c2b 100644 --- a/cmake/external/gsl.cmake +++ b/cmake/external/gsl.cmake @@ -3,11 +3,21 @@ include(FetchContent) -FetchContent_Declare( +if(onnxruntime_USE_CUDA) + FetchContent_Declare( GSL URL https://github.com/microsoft/GSL/archive/refs/tags/v4.0.0.zip URL_HASH SHA1=cf368104cd22a87b4dd0c80228919bb2df3e2a14 - ) + PATCH_COMMAND ${Patch_EXECUTABLE} --binary --ignore-whitespace -p1 < ${PROJECT_SOURCE_DIR}/patches/gsl/1064.patch + ) +else() + FetchContent_Declare( + GSL + URL https://github.com/microsoft/GSL/archive/refs/tags/v4.0.0.zip + URL_HASH SHA1=cf368104cd22a87b4dd0c80228919bb2df3e2a14 + ) +endif() + FetchContent_MakeAvailable(GSL) diff --git a/cmake/onnxruntime_objectivec.cmake b/cmake/onnxruntime_objectivec.cmake index 810fc3b5233fe..ae73e9c5f1228 100644 --- a/cmake/onnxruntime_objectivec.cmake +++ b/cmake/onnxruntime_objectivec.cmake @@ -32,10 +32,6 @@ endif() add_compile_options( "$<$:-Wall>" "$<$:-Wextra>") -if(onnxruntime_DEV_MODE) - add_compile_options( - "$<$:-Werror>") -endif() set(OBJC_ROOT "${REPO_ROOT}/objectivec") diff --git a/cmake/onnxruntime_providers.cmake b/cmake/onnxruntime_providers.cmake index 4c123912dda75..2a793b9342195 100644 --- a/cmake/onnxruntime_providers.cmake +++ b/cmake/onnxruntime_providers.cmake @@ -242,7 +242,7 @@ endif() if (MSVC) target_compile_options(onnxruntime_providers PRIVATE "/bigobj") - if(onnxruntime_DEV_MODE AND NOT CMAKE_SIZEOF_VOID_P EQUAL 8) + if(NOT CMAKE_SIZEOF_VOID_P EQUAL 8) target_compile_options(onnxruntime_providers PRIVATE "/wd4244") endif() endif() @@ -431,18 +431,17 @@ if (onnxruntime_USE_CUDA) if (CMAKE_CUDA_COMPILER_VERSION VERSION_GREATER_EQUAL 11.3) target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:--threads \"${onnxruntime_NVCC_THREADS}\">") endif() - if (onnxruntime_DEV_MODE) - if (UNIX) - target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:-Xcompiler -Wno-reorder>" - "$<$>:-Wno-reorder>") - target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:-Xcompiler -Wno-error=sign-compare>" - "$<$>:-Wno-error=sign-compare>") - else() - #mutex.cuh(91): warning C4834: discarding return value of function with 'nodiscard' attribute - target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:-Xcompiler /wd4834>") - target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:-Xcompiler /wd4127>") - endif() + if (UNIX) + target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:-Xcompiler -Wno-reorder>" + "$<$>:-Wno-reorder>") + target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:-Xcompiler -Wno-error=sign-compare>" + "$<$>:-Wno-error=sign-compare>") + else() + #mutex.cuh(91): warning C4834: discarding return value of function with 'nodiscard' attribute + target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:-Xcompiler /wd4834>") + target_compile_options(onnxruntime_providers_cuda PRIVATE "$<$:SHELL:-Xcompiler /wd4127>") endif() + onnxruntime_add_include_to_target(onnxruntime_providers_cuda onnxruntime_common onnxruntime_framework onnx onnx_proto ${PROTOBUF_LIB} flatbuffers) if (onnxruntime_ENABLE_TRAINING OR onnxruntime_ENABLE_TRAINING_OPS) onnxruntime_add_include_to_target(onnxruntime_providers_cuda onnxruntime_training) @@ -745,7 +744,7 @@ if (onnxruntime_USE_OPENVINO) ) if (WIN32) - set(CMAKE_MAP_IMPORTED_CONFIG_RELWITHDEBINFO Release) + set(CMAKE_MAP_IMPORTED_CONFIG_RELWITHDEBINFO Release) endif() # Header paths @@ -1089,7 +1088,7 @@ if (onnxruntime_USE_DML) else() add_dependencies(${target} RESTORE_PACKAGES) target_link_libraries(${target} PRIVATE "${DML_PACKAGE_DIR}/bin/${onnxruntime_target_platform}-win/DirectML.lib") - target_compile_definitions(${target} PRIVATE DML_TARGET_VERSION_USE_LATEST) + target_compile_definitions(${target} PRIVATE DML_TARGET_VERSION_USE_LATEST) endif() endfunction() diff --git a/cmake/patches/gsl/1064.patch b/cmake/patches/gsl/1064.patch new file mode 100644 index 0000000000000..90e694fbbb833 --- /dev/null +++ b/cmake/patches/gsl/1064.patch @@ -0,0 +1,26 @@ +diff --git a/include/gsl/assert b/include/gsl/assert +index a6012048..a5c216f8 100644 +--- a/include/gsl/assert ++++ b/include/gsl/assert +@@ -48,7 +48,7 @@ + #if defined(__clang__) + #define GSL_SUPPRESS(x) [[gsl::suppress("x")]] + #else +-#if defined(_MSC_VER) && !defined(__INTEL_COMPILER) ++#if defined(_MSC_VER) && !defined(__INTEL_COMPILER) && !defined(__NVCC__) + #define GSL_SUPPRESS(x) [[gsl::suppress(x)]] + #else + #define GSL_SUPPRESS(x) +diff --git a/include/gsl/byte b/include/gsl/byte +index 9231340b..f92a91c9 100644 +--- a/include/gsl/byte ++++ b/include/gsl/byte +@@ -24,7 +24,7 @@ + #if defined(__clang__) + #define GSL_SUPPRESS(x) [[gsl::suppress("x")]] + #else +-#if defined(_MSC_VER) && !defined(__INTEL_COMPILER) ++#if defined(_MSC_VER) && !defined(__INTEL_COMPILER) && !defined(__NVCC__) + #define GSL_SUPPRESS(x) [[gsl::suppress(x)]] + #else + #define GSL_SUPPRESS(x) diff --git a/tools/ci_build/build.py b/tools/ci_build/build.py index 0dae087ae6b89..077d9932dc6cf 100644 --- a/tools/ci_build/build.py +++ b/tools/ci_build/build.py @@ -170,7 +170,11 @@ def convert_arg_line_to_args(self, arg_line): ) parser.add_argument("--test", action="store_true", help="Run unit tests.") parser.add_argument("--skip_tests", action="store_true", help="Skip all tests.") - + parser.add_argument( + "--compile_no_warning_as_error", + action="store_true", + help="Preventing warnings from being treated as errors on compile.", + ) # Training options parser.add_argument("--enable_nvtx_profile", action="store_true", help="Enable NVTX profile in ORT.") parser.add_argument("--enable_memory_profile", action="store_true", help="Enable memory profile in ORT.") @@ -784,16 +788,18 @@ def setup_test_data(source_onnx_model_dir, dest_model_dir_name, build_dir, confi def use_dev_mode(args): + if args.compile_no_warning_as_error: + return False if args.use_acl: - return "OFF" + return False if args.use_armnn: - return "OFF" + return False if args.ios and is_macOS(): - return "OFF" + return False SYSTEM_COLLECTIONURI = os.getenv("SYSTEM_COLLECTIONURI") if SYSTEM_COLLECTIONURI and not SYSTEM_COLLECTIONURI == "https://dev.azure.com/onnxruntime/": - return "OFF" - return "ON" + return False + return True def add_default_definition(definition_list, key, default_value): @@ -832,9 +838,11 @@ def generate_build_tree( ): log.info("Generating CMake build tree") cmake_dir = os.path.join(source_dir, "cmake") - cmake_args = [ - cmake_path, - cmake_dir, + cmake_args = [cmake_path, cmake_dir] + if not use_dev_mode(args): + cmake_args += ["--compile-no-warning-as-error"] + + cmake_args += [ "-Donnxruntime_RUN_ONNX_TESTS=" + ("ON" if args.enable_onnx_tests else "OFF"), "-Donnxruntime_GENERATE_TEST_REPORTS=ON", # There are two ways of locating python C API header file. "find_package(PythonLibs 3.5 REQUIRED)" @@ -953,9 +961,6 @@ def generate_build_tree( if args.llvm_config: cmake_args.append("-Donnxruntime_TVM_USE_LLVM=" + args.llvm_config) - # It should be default ON in CI build pipelines, and OFF in packaging pipelines. - # And OFF for the people who are not actively developing onnx runtime. - add_default_definition(cmake_extra_defines, "onnxruntime_DEV_MODE", use_dev_mode(args)) if args.use_cuda: add_default_definition(cmake_extra_defines, "onnxruntime_USE_CUDA", "ON") if args.cuda_version: