saymrwulf/pytorch
1
0
Fork 0
mirror of https://github.com/saymrwulf/pytorch.git synced 2026-05-14 20:57:59 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Correct /MP usage in MSVC (#33120)

Browse source 
Summary:
## Several flags
`/MP[M]`: It is a flag for the compiler `cl`. It leads to object-level multiprocessing. By default, it spawns M processes where M is the number of cores on the PC.
`/maxcpucount:[M]`: It is a flag for the generator `msbuild`. It leads to project-level multiprocessing. By default, it spawns M processes where M is the number of cores on the PC.
`/p:CL_MPCount=[M]`: It is a flag for the generator `msbuild`. It leads the generator to pass `/MP[M]` to the compiler.
`/j[M]`: It is a flag for the generator `ninja`. It leads to object-level multiprocessing. By default, it spawns M processes where M is the number of cores on the PC.

## Reason for the change
1. Object-level multiprocessing is preferred over project-level multiprocessing.
2. ~For ninja, we don't need to set `/MP` otherwise M * M processes will be spawned.~ Actually, it is not correct because in ninja configs, there are only one source file in the command. Therefore, the `/MP` switch should be useless.
3. For msbuild, if it is called through Python configuration scripts, then `/p:CL_MPCount=[M]` will be added, otherwise, we add `/MP` to `CMAKE_CXX_FLAGS`.
4. ~It may be a possible fix for https://github.com/pytorch/pytorch/issues/28271, https://github.com/pytorch/pytorch/issues/27463 and https://github.com/pytorch/pytorch/issues/25393. Because `/MP` is also passed to `nvcc`.~ It is probably not true. Because `/MP` should not be effective given there is only one source file per command.

## Reference
1. https://docs.microsoft.com/en-us/cpp/build/reference/mp-build-with-multiple-processes?view=vs-2019
2. https://github.com/Microsoft/checkedc-clang/wiki/Parallel-builds-of-clang-on-Windows
3. https://blog.kitware.com/cmake-building-with-all-your-cores/
Pull Request resolved: https://github.com/pytorch/pytorch/pull/33120

Differential Revision: D19817227

Pulled By: ezyang

fbshipit-source-id: f8d01f835016971729c7a8d8a0d1cb8a8c2c6a5f
This commit is contained in:
peterjc123 2020-02-10 11:26:19 -08:00 committed by Facebook Github Bot
parent 9d94f56ce0
commit ebed008dd4
2 changed files with 10 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
CMakeLists.txt
View file

@ -271,7 +271,15 @@ if (MSVC)
# /bigobj increases number of sections in .obj file, which is needed to link # /bigobj increases number of sections in .obj file, which is needed to link
# against libraries in Python 2.7 under Windows # against libraries in Python 2.7 under Windows
set(${flag_var} "${${flag_var}} /MP /bigobj") # For Visual Studio generators, if /MP is not added, then we may need
# to add /MP to the flags.
# For other generators like ninja, we don't need to add /MP because it is
# already handled by the generator itself.
if(CMAKE_GENERATOR MATCHES "Visual Studio" AND NOT ${flag_var} MATCHES "/MP")
set(${flag_var} "${${flag_var}} /MP /bigobj")
else()
set(${flag_var} "${${flag_var}} /bigobj")
endif()
endforeach(flag_var) endforeach(flag_var)
foreach(flag_var foreach(flag_var

2
tools/setup_helpers/cmake.py
View file

@ -335,7 +335,7 @@ class CMake:
# minimum, which provides a '-j' option: build_args += ['-j', max_jobs] # minimum, which provides a '-j' option: build_args += ['-j', max_jobs]
# would be sufficient by then. # would be sufficient by then.
if IS_WINDOWS and not USE_NINJA: # We are likely using msbuild here if IS_WINDOWS and not USE_NINJA: # We are likely using msbuild here
build_args += ['--', '/maxcpucount:{}'.format(max_jobs)] build_args += ['--', '/p:CL_MPCount={}'.format(max_jobs)]
else: else:
build_args += ['--', '-j', max_jobs] build_args += ['--', '-j', max_jobs]
self.run(build_args, my_env) self.run(build_args, my_env)