Skip to content

Commit

Permalink
Fix a build issue: /MP was not enabled correctly (#19190)
Browse files Browse the repository at this point in the history
### Description

In PR #19073 I mistunderstood the value of "--parallel". Instead of
testing if args.parallel is None or not , I should test the returned
value of number_of_parallel_jobs function.

If build.py was invoked without --parallel, then args.parallel equals to
1. Because it is the default value. Then we should not add "/MP".
However, the current code adds it. Because if `args.paralllel` is
evaluated to `if 1` , which is True.
If build.py was invoked with --parallel with additional numbers, then
args.parallel equals to 0. Because it is unspecified. Then we should add
"/MP". However, the current code does not add it. Because `if
args.paralllel` is evaluated to `if 0` , which is False.

This also adds a new build flag: use_binskim_compliant_compile_flags, which is intended to be only used in ONNX Runtime team's build pipelines for compliance reasons. 

### Motivation and Context
  • Loading branch information
snnn authored Jan 29, 2024
1 parent 4ee2224 commit e91d91a
Show file tree
Hide file tree
Showing 33 changed files with 72 additions and 69 deletions.
2 changes: 1 addition & 1 deletion .pipelines/windowsai-steps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ jobs:
7z x cmake-3.26.3-windows-x86_64.zip
set PYTHONHOME=$(Build.BinariesDirectory)\${{ parameters.PythonPackageName }}.3.9.7\tools
set PYTHONPATH=$(Build.BinariesDirectory)\${{ parameters.PythonPackageName }}.3.9.7\tools
$(Build.BinariesDirectory)\${{ parameters.PythonPackageName }}.3.9.7\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 17 2022" --update --config RelWithDebInfo --enable_qspectre --enable_lto --use_telemetry --disable_rtti --enable_wcos $(BuildFlags) --cmake_extra_defines "CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO=/PROFILE" "CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO=/PROFILE" CMAKE_SYSTEM_VERSION=10.0.19041.0 --cmake_path $(Build.BinariesDirectory)\cmake-3.26.3-windows-x86_64\bin\cmake.exe --ctest_path $(Build.BinariesDirectory)\cmake-3.26.3-windows-x86_64\bin\ctest.exe
$(Build.BinariesDirectory)\${{ parameters.PythonPackageName }}.3.9.7\tools\python.exe "$(Build.SourcesDirectory)\tools\ci_build\build.py" --build_dir $(Build.BinariesDirectory) --parallel --use_binskim_compliant_compile_flags --build_shared_lib --enable_onnx_tests --ms_experimental --use_dml --use_winml --cmake_generator "Visual Studio 17 2022" --update --config RelWithDebInfo --enable_lto --use_telemetry --disable_rtti --enable_wcos $(BuildFlags) --cmake_extra_defines "CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO=/PROFILE" "CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO=/PROFILE" CMAKE_SYSTEM_VERSION=10.0.19041.0 --cmake_path $(Build.BinariesDirectory)\cmake-3.26.3-windows-x86_64\bin\cmake.exe --ctest_path $(Build.BinariesDirectory)\cmake-3.26.3-windows-x86_64\bin\ctest.exe
workingDirectory: '$(Build.BinariesDirectory)'
displayName: 'Generate cmake config'
Expand Down
43 changes: 23 additions & 20 deletions tools/ci_build/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,8 +442,8 @@ def convert_arg_line_to_args(self, arg_line):
parser.add_argument(
"--enable_address_sanitizer", action="store_true", help="Enable address sanitizer. Windows/Linux/MacOS only."
)
# The following feature requires installing some special Visual Studio components that do not get installed by default. Therefore the options is default OFF.
parser.add_argument("--enable_qspectre", action="store_true", help="Enable Qspectre. Windows only.")
# The following flag is mostly designed to be used in ONNX Runtime's Azure DevOps/Github build pipelines. Its main purpose is to make the built binaries pass BinSkim scan.
parser.add_argument("--use_binskim_compliant_compile_flags", action="store_true", help="Use preset compile flags.")
parser.add_argument(
"--disable_memleak_checker",
action="store_true",
Expand Down Expand Up @@ -1484,27 +1484,29 @@ def generate_build_tree(
f"-DVERSION_PRIVATE_PART={MM}{DD}",
f"-DVERSION_STRING={ort_major}.{ort_minor}.{build_number}.{source_version[0:7]}",
]
cflags = None
cxxflags = None
ldflags = None
cudaflags = []

for config in configs:
cflags = []
cxxflags = None
ldflags = None
cudaflags = []
if is_windows() and not args.ios and not args.android and not args.build_wasm:
njobs = number_of_parallel_jobs(args)
if njobs > 1:
if args.parallel == 0:
cflags += ["/MP"]
else:
cflags += ["/MP%d" % njobs]
# Setup default values for cflags/cxxflags/ldflags.
# The values set here are purely for security and compliance purposes. ONNX Runtime should work fine without these flags.
if (
"CFLAGS" not in os.environ
and "CXXFLAGS" not in os.environ
and (not args.use_cuda or "CUDAFLAGS" not in os.environ)
(args.use_binskim_compliant_compile_flags or args.enable_address_sanitizer)
and not args.ios
and not args.android
and not args.build_wasm
and not args.use_rocm
and not (is_linux() and platform.machine() != "aarch64" and platform.machine() != "x86_64")
):
if is_windows():
cflags = ["/guard:cf", "/DWIN32", "/D_WINDOWS"]
if args.parallel:
cflags += ["/MP"]
cflags += ["/guard:cf", "/DWIN32", "/D_WINDOWS"]
if not args.use_gdk:
# Target Windows 10
cflags += [
Expand All @@ -1516,21 +1518,20 @@ def generate_build_tree(
# The "/profile" flag implies "/DEBUG:FULL /DEBUGTYPE:cv,fixup /OPT:REF /OPT:NOICF /INCREMENTAL:NO /FIXED:NO". We set it for satisfying a Microsoft internal compliance requirement. External users
# do not need to have it.
ldflags = ["/profile", "/DYNAMICBASE"]
if args.enable_qspectre:
# Address Sanitizer libs do not have a Qspectre version. So they two cannot be both enabled.
if not args.enable_address_sanitizer:
cflags += ["/Qspectre"]
if config == "Release":
cflags += ["/O2", "/Ob2", "/DNDEBUG"]
elif config == "RelWithDebInfo":
cflags += ["/O2", "/Ob1", "/DNDEBUG"]
elif config == "Debug":
cflags += ["/Ob0", "/Od", "/RTC1"]
if args.enable_address_sanitizer:
cflags += ["/fsanitize=address"]
elif config == "MinSizeRel":
cflags += ["/O1", "/Ob1", "/DNDEBUG"]
if args.enable_address_sanitizer:
cflags += ["/fsanitize=address"]
cxxflags = cflags.copy()
if not args.disable_exceptions:
cxxflags += ["/EHsc"]
if args.use_cuda:
# On Windows, nvcc passes /EHsc to the host compiler by default.
cuda_compile_flags_str = ""
Expand Down Expand Up @@ -1590,6 +1591,8 @@ def generate_build_tree(
cxxflags = cflags.copy()
if args.use_cuda:
cudaflags = cflags.copy()
if cxxflags is None and cflags is not None and len(cflags) != 0:
cxxflags = cflags.copy()
config_build_dir = get_config_build_dir(build_dir, config)
os.makedirs(config_build_dir, exist_ok=True)
if args.use_tvm:
Expand All @@ -1604,7 +1607,7 @@ def generate_build_tree(
)
preinstalled_dir = Path(build_dir) / config
temp_cmake_args = cmake_args.copy()
if cflags is not None and cxxflags is not None:
if cflags is not None and cxxflags is not None and len(cflags) != 0 and len(cxxflags) != 0:
temp_cmake_args += [
"-DCMAKE_C_FLAGS=%s" % (" ".join(cflags)),
"-DCMAKE_CXX_FLAGS=%s" % (" ".join(cxxflags)),
Expand Down
6 changes: 3 additions & 3 deletions tools/ci_build/github/azure-pipelines/linux-ci-pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ stages:
--config Debug \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_csharp \
--enable_onnx_tests --enable_address_sanitizer \
--update --build;
Expand All @@ -102,7 +102,7 @@ stages:
--config Debug \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_csharp \
--enable_onnx_tests --enable_address_sanitizer \
--test;"
Expand Down Expand Up @@ -228,7 +228,7 @@ stages:
--config Release \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_wheel \
--build_csharp \
--enable_onnx_tests \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ jobs:
--config Release \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_wheel \
--skip_tests \
--cmake_extra_defines onnxruntime_ENABLE_ATEN=ON \
Expand Down Expand Up @@ -126,7 +126,7 @@ jobs:
--config Release \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_wheel \
--test \
--cmake_extra_defines onnxruntime_ENABLE_ATEN=ON"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:
--config Release \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--enable_lazy_tensor --enable_training --build_wheel --skip_test \
workingDirectory: $(Build.SourcesDirectory)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ jobs:
--config Debug \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--skip_tests \
--minimal_build \
--disable_exceptions \
Expand Down Expand Up @@ -222,7 +222,7 @@ jobs:
--build_dir /build/5 --cmake_generator Ninja \
--config Debug \
--skip_submodule_sync \
--build_shared_lib \
--build_shared_lib --use_binskim_compliant_compile_flags \
--parallel \
--minimal_build extended
workingDirectory: $(Build.SourcesDirectory)
Expand All @@ -246,7 +246,7 @@ jobs:
--skip_submodule_sync \
--build_shared_lib \
--build_wheel \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--skip_tests \
--disable_ml_ops \
--disable_types sparsetensor float8 optional \
Expand All @@ -272,7 +272,7 @@ jobs:
--config MinSizeRel \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--minimal_build \
--disable_exceptions \
--disable_ml_ops \
Expand Down Expand Up @@ -300,7 +300,7 @@ jobs:
--cmake_generator Ninja \
--config MinSizeRel \
--skip_submodule_sync \
--build_shared_lib \
--build_shared_lib --use_binskim_compliant_compile_flags \
--parallel \
--minimal_build extended \
--disable_exceptions \
Expand Down Expand Up @@ -330,7 +330,7 @@ jobs:
--cmake_generator Ninja \
--config MinSizeRel \
--skip_submodule_sync \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--android \
--android_sdk_path /android_home \
--android_ndk_path /ndk_home \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ jobs:
--config Release --update --build \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_wheel \
--enable_onnx_tests --use_cuda --cuda_version=${{parameters.CudaVersion}} --cuda_home=/usr/local/cuda-${{parameters.CudaVersion}} --cudnn_home=/usr/local/cuda-${{parameters.CudaVersion}} \
--enable_cuda_profiling --enable_cuda_nhwc_ops \
Expand Down Expand Up @@ -215,7 +215,7 @@ jobs:
cd /onnxruntime_src/java && /onnxruntime_src/java/gradlew cmakeCheck -DcmakeBuildDir=/build/Release -DUSE_CUDA=1; \
cd /tmp; \
/tmp/python3 /onnxruntime_src/tools/ci_build/build.py \
--build_dir /build --config Release --test --skip_submodule_sync --build_shared_lib --parallel --build_wheel --enable_onnx_tests \
--build_dir /build --config Release --test --skip_submodule_sync --build_shared_lib --parallel --use_binskim_compliant_compile_flags --build_wheel --enable_onnx_tests \
--use_cuda --cuda_version=${{parameters.CudaVersion}} --cuda_home=/usr/local/cuda --cudnn_home=/usr/local/cuda \
--enable_pybind --build_java --ctest_path '' "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ jobs:
--config Release \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_wheel \
--enable_onnx_tests \
--use_cuda --cuda_home=/usr/local/cuda-${{ parameters.CudaVersion }} --cudnn_home=/usr/local/cuda-${{ parameters.CudaVersion }} \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
python3 tools/ci_build/build.py \
--build_dir build \
--config Release \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--use_qnn \
--qnn_home $(QNN_SDK_ROOT) \
--cmake_generator=Ninja \
Expand All @@ -73,7 +73,7 @@ jobs:
- script: |
python3 tools/ci_build/build.py \
--build_dir build \
--config Release \
--config Release --use_binskim_compliant_compile_flags \
--test \
--qnn_home $(QNN_SDK_ROOT) \
--cmake_generator=Ninja \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
--build_dir build \
--skip_submodule_sync \
--cmake_generator=Ninja \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_shared_lib \
--config Debug \
--use_cache \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ jobs:
--use_xcode \
--config RelWithDebInfo \
--build_apple_framework \
--parallel
--parallel --use_binskim_compliant_compile_flags
displayName: (CPU, CoreML, XNNPACK EPs) Build onnxruntime for iOS x86_64 and run tests using simulator
env:
CC: clang
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
--enable_training_apis \
--cmake_extra_defines CMAKE_EXPORT_COMPILE_COMMANDS=ON \
--update --skip_submodule_sync \
--build --parallel --target onnx_proto
--build --parallel --use_binskim_compliant_compile_flags --target onnx_proto
displayName: Generate compile_commands.json and ONNX protobuf files
- script: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ stages:
displayName: 'Generate cmake config'
inputs:
scriptPath: '$(Build.SourcesDirectory)\tools\ci_build\build.py'
arguments: '$(BuildCommand) --path_to_protoc_exe $(Build.BinariesDirectory)\installed\bin\protoc.exe --build_csharp --update --config $(BuildConfig) ${{ variables.build_py_lto_flag }}'
arguments: '$(BuildCommand) --use_binskim_compliant_compile_flags --parallel --path_to_protoc_exe $(Build.BinariesDirectory)\installed\bin\protoc.exe --build_csharp --update --config $(BuildConfig) ${{ variables.build_py_lto_flag }}'
workingDirectory: '$(Build.BinariesDirectory)'

- ${{ if notIn(parameters['sln_platform'], 'Win32', 'x64') }}:
Expand Down Expand Up @@ -176,7 +176,7 @@ stages:
python.exe -m pip install -q --upgrade %WHEEL_FILENAME%
set PATH=%PATH%;$(Build.BinariesDirectory)\$(BuildConfig)\$(BuildConfig)
@echo %PATH%
python $(Build.SourcesDirectory)\tools\ci_build\build.py $(BuildCommand) --test --config $(BuildConfig) ${{ variables.build_py_lto_flag }}
python $(Build.SourcesDirectory)\tools\ci_build\build.py $(BuildCommand) --parallel --use_binskim_compliant_compile_flags --test --config $(BuildConfig) ${{ variables.build_py_lto_flag }}
workingDirectory: '$(Build.BinariesDirectory)\$(BuildConfig)\$(BuildConfig)'
displayName: 'Run tests'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ jobs:
--config Release \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_wheel \
--enable_onnx_tests \
--enable_training \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ stages:
--config Debug Release \
--skip_submodule_sync \
--build_shared_lib \
--parallel \
--parallel --use_binskim_compliant_compile_flags \
--build_wheel \
--enable_onnx_tests \
--enable_pybind --enable_training
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
buildArch: x64
setVcvars: true
ALLOW_RELEASED_ONNX_OPSET_ONLY: '1'
commonBuildArgs: '--compile_no_warning_as_error --build_dir $(Build.BinariesDirectory)\Windows --skip_submodule_sync --build_shared_lib --cmake_generator "Visual Studio 17 2022" --config ${{ parameters.build_config }} --use_qnn --qnn_home C:\data\qnnsdk\${{parameters.QnnSdk}} --parallel'
commonBuildArgs: '--compile_no_warning_as_error --build_dir $(Build.BinariesDirectory)\Windows --skip_submodule_sync --build_shared_lib --cmake_generator "Visual Studio 17 2022" --config ${{ parameters.build_config }} --use_qnn --qnn_home C:\data\qnnsdk\${{parameters.QnnSdk}} --parallel --use_binskim_compliant_compile_flags '

steps:
- template: templates/set-version-number-variables-step.yml
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ jobs:
docker run --rm --volume /data/onnx:/data/onnx:ro --volume $(Build.SourcesDirectory):/onnxruntime_src --volume $(Build.BinariesDirectory):/build \
--volume $HOME/.onnx:/home/onnxruntimedev/.onnx -e NIGHTLY_BUILD onnxruntimecpubuildcentos8${{parameters.OnnxruntimeArch}} /bin/bash -c "python3.9 \
/onnxruntime_src/tools/ci_build/build.py --enable_lto --build_java --build_nodejs --build_dir /build --config Release \
--skip_submodule_sync --parallel --build_shared_lib ${{ parameters.AdditionalBuildFlags }} && cd /build/Release && make install DESTDIR=/build/linux-${{parameters.OnnxruntimeArch}}"
--skip_submodule_sync --parallel --use_binskim_compliant_compile_flags --build_shared_lib ${{ parameters.AdditionalBuildFlags }} && cd /build/Release && make install DESTDIR=/build/linux-${{parameters.OnnxruntimeArch}}"
workingDirectory: $(Build.SourcesDirectory)
displayName: 'Build'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ jobs:
Today: $(TODAY)
CacheDir: $(ORT_CACHE_DIR)
AdditionalKey: " $(System.StageName) | ${{ parameters.BuildConfig }} "
BuildPyArguments: '--config ${{ parameters.BuildConfig }} --build_dir $(Build.BinariesDirectory) --skip_submodule_sync --build_csharp --update --parallel --cmake_generator "Visual Studio 17 2022" --build_shared_lib --enable_onnx_tests ${{ parameters.additionalBuildFlags }}'
BuildPyArguments: '--config ${{ parameters.BuildConfig }} --build_dir $(Build.BinariesDirectory) --skip_submodule_sync --build_csharp --update --parallel --use_binskim_compliant_compile_flags --cmake_generator "Visual Studio 17 2022" --build_shared_lib --enable_onnx_tests ${{ parameters.additionalBuildFlags }}'
MsbuildArguments: '-maxcpucount'
BuildArch: ${{ parameters.buildArch }}
Platform: ${{ parameters.msbuildPlatform }}
Expand Down
Loading

0 comments on commit e91d91a

Please sign in to comment.