From b335f3910fe1a36c3fb84452db5b0907c79e0650 Mon Sep 17 00:00:00 2001 From: Hong Xu Date: Mon, 29 Jul 2019 08:04:33 -0700 Subject: [PATCH] Remove redundant MSVC_Z7_OVERRIDE processing and combine "/EHa" flag 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: https://github.com/pytorch/pytorch/pull/23455 Differential Revision: D16542274 Pulled By: ezyang fbshipit-source-id: 4d3b8b07161478bbba8a21feb6ea24c9024e21ac --- CMakeLists.txt | 6 +++++- tools/setup_helpers/cmake.py | 5 +---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e8f4352223e..7657b0c7b17 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -166,7 +166,9 @@ 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.") @@ -174,6 +176,8 @@ set(ONNX_NAMESPACE "onnx_torch" CACHE STRING "A namespace for ONNX; needed to bu # 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 diff --git a/tools/setup_helpers/cmake.py b/tools/setup_helpers/cmake.py index a5610e5f36e..73fd579ebc3 100644 --- a/tools/setup_helpers/cmake.py +++ b/tools/setup_helpers/cmake.py @@ -189,10 +189,6 @@ class CMake: 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__)))) @@ -227,6 +223,7 @@ class CMake: 'EXPERIMENTAL_SINGLE_THREAD_POOL', 'MKL_THREADING', 'MKLDNN_THREADING', + 'MSVC_Z7_OVERRIDE', 'ONNX_ML', 'ONNX_NAMESPACE', 'ATEN_THREADING',