Skip to content

Commit

Permalink
Remove redundant MSVC_Z7_OVERRIDE processing and combine "/EHa" flag …
Browse files Browse the repository at this point in the history
…setup (#23455)

Summary:
- MSVC_Z7_OVERRIDE has already handled in CMakeLists.txt. No need to process it for once more in the Python scripts.
- Option MSVC_Z7_OVERRIDE should be visible to the user only if MSVC is used.
- Move the setting of "/EHa" flag to CMakeLists.txt, where other MSVC-specific flags are processed. This also further prepares the removal of redundant cflags setup in Python build scripts.
Pull Request resolved: pytorch/pytorch#23455

Differential Revision: D16542274

Pulled By: ezyang

fbshipit-source-id: 4d3b8b07161478bbba8a21feb6ea24c9024e21ac
  • Loading branch information
xuhdev authored and facebook-github-bot committed Jul 29, 2019
1 parent 81e46d4 commit b335f39
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
6 changes: 5 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,18 @@ option(BUILDING_WITH_TORCH_LIBS "Tell cmake if Caffe2 is being built alongside t
# When generating debug symbols, CMake default to use the flag /Zi.
# However, it is not compatible with sccache. So we rewrite it off.
# But some users don't use sccache; this override is for them.
option(MSVC_Z7_OVERRIDE "Work around sccache bug by replacing /Zi and /ZI with /Z7 when using MSVC (if you are not using sccache, you can turn this OFF)" ON)
cmake_dependent_option(
MSVC_Z7_OVERRIDE "Work around sccache bug by replacing /Zi and /ZI with /Z7 when using MSVC (if you are not using sccache, you can turn this OFF)" ON
"MSVC" OFF)

set(ONNX_NAMESPACE "onnx_torch" CACHE STRING "A namespace for ONNX; needed to build with other frameworks that share ONNX.")

# For MSVC,
# 1. Replace /Zi and /ZI with /Z7
# 2. Switch off incremental linking in debug builds
if (MSVC)
string(APPEND CMAKE_C_FLAGS " /EHa")
string(APPEND CMAKE_CXX_FLAGS " /EHa")
if(MSVC_Z7_OVERRIDE)
foreach(flag_var
CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE
Expand Down
5 changes: 1 addition & 4 deletions tools/setup_helpers/cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,6 @@ def generate(self, version, cmake_python_library, build_python, build_test, my_e

cflags = os.getenv('CFLAGS', "") + " " + os.getenv('CPPFLAGS', "")
ldflags = os.getenv('LDFLAGS', "")
if IS_WINDOWS:
CMake.defines(args, MSVC_Z7_OVERRIDE=not check_negative_env_flag(
'MSVC_Z7_OVERRIDE'))
cflags += " /EHa"

base_dir = os.path.dirname(os.path.dirname(os.path.dirname(
os.path.abspath(__file__))))
Expand Down Expand Up @@ -227,6 +223,7 @@ def generate(self, version, cmake_python_library, build_python, build_test, my_e
'EXPERIMENTAL_SINGLE_THREAD_POOL',
'MKL_THREADING',
'MKLDNN_THREADING',
'MSVC_Z7_OVERRIDE',
'ONNX_ML',
'ONNX_NAMESPACE',
'ATEN_THREADING',
Expand Down

0 comments on commit b335f39

Please sign in to comment.