Skip to content

Commit 9cb582b

Browse files
committed
Give targets that link torch_cpu_library torch's compile settings back
Linking the torch target does more than link. TorchConfig also gives it TORCH_INCLUDE_DIRS, which CMake treats as system headers because the target is imported, TORCH_CXX_FLAGS, and on recent torch a minimum C++ standard (C++20). torch_cpu_library, which our targets now link instead so that a CPU-only torch can load them, only passes on torch/include. So those targets changed. torch/csrc/api/include became a normal include directory, so warnings inside torch's headers are now reported as if they were our code. And a TORCH_CXX_FLAGS set at configure time no longer reached them. For example, compiling kernels/portable/cpu/op_allclose.cpp with -Wall -Wextra -Werror went from no errors to 25 errors, all inside torch's headers. Default builds were not affected. find_package_torch now copies torch's interface include directories, compile options and compile features to torch_cpu_library right after it finds torch. Every target that links torch_cpu_library, directly or through a PUBLIC link such as custom_ops_aot_lib, compiles as if it linked torch, and a new link site cannot forget it. A new test configures a small project and checks that a target linking torch_cpu_library, directly or through a PUBLIC link, gets the same compile flags as a target linking torch. It fails without this change.
1 parent 54c0a1d commit 9cb582b

2 files changed

Lines changed: 103 additions & 0 deletions

File tree

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
# All rights reserved.
3+
#
4+
# This source code is licensed under the BSD-style license found in the
5+
# LICENSE file in the root directory of this source tree.
6+
7+
import importlib.util
8+
import json
9+
import shlex
10+
import shutil
11+
import subprocess
12+
import sys
13+
import tempfile
14+
import unittest
15+
from pathlib import Path
16+
17+
ROOT = Path(__file__).resolve().parents[3]
18+
19+
20+
def _compile_flags(entry: dict) -> set:
21+
"""The flags of one compile command, without the compiler, source and outputs."""
22+
flags = set()
23+
arguments = iter(shlex.split(entry["command"])[1:])
24+
for argument in arguments:
25+
if argument in ("-o", "-c", "-MT", "-MF"):
26+
next(arguments)
27+
elif argument == "-isystem":
28+
flags.add(f"-isystem {next(arguments)}")
29+
elif argument != "-MD":
30+
flags.add(argument)
31+
return flags
32+
33+
34+
@unittest.skipIf(sys.platform == "win32", "Visual Studio writes no compile commands")
35+
class TestTorchCpuLibraryCMake(unittest.TestCase):
36+
def test_torch_cpu_library_compiles_like_torch(self):
37+
cmake = shutil.which("cmake")
38+
if cmake is None or importlib.util.find_spec("torch") is None:
39+
self.skipTest("CMake and torch are required")
40+
41+
with tempfile.TemporaryDirectory() as directory:
42+
source = Path(directory)
43+
build = source / "build"
44+
(source / "probe.cpp").write_text("int probe() { return 0; }\n")
45+
(source / "CMakeLists.txt").write_text(
46+
"""cmake_minimum_required(VERSION 3.24)
47+
project(TorchCpuLibraryProbe LANGUAGES CXX)
48+
include("${EXECUTORCH_ROOT}/tools/cmake/Utils.cmake")
49+
find_package_torch()
50+
add_library(links_torch OBJECT probe.cpp)
51+
target_link_libraries(links_torch PRIVATE torch)
52+
add_library(links_torch_cpu_library OBJECT probe.cpp)
53+
target_link_libraries(links_torch_cpu_library PRIVATE torch_cpu_library)
54+
add_library(links_it_publicly STATIC probe.cpp)
55+
target_link_libraries(links_it_publicly PUBLIC torch_cpu_library)
56+
add_library(consumer OBJECT probe.cpp)
57+
target_link_libraries(consumer PRIVATE links_it_publicly)
58+
"""
59+
)
60+
result = subprocess.run(
61+
[
62+
cmake,
63+
"-S",
64+
str(source),
65+
"-B",
66+
str(build),
67+
f"-DEXECUTORCH_ROOT={ROOT}",
68+
f"-DPYTHON_EXECUTABLE={sys.executable}",
69+
"-DCMAKE_EXPORT_COMPILE_COMMANDS=ON",
70+
# Recent torch sets no TORCH_CXX_FLAGS, so set one to check it travels too.
71+
"-DTORCH_CXX_FLAGS=-DET_TORCH_CXX_FLAGS_PROBE",
72+
],
73+
capture_output=True,
74+
text=True,
75+
)
76+
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
77+
78+
flags = {}
79+
for entry in json.loads((build / "compile_commands.json").read_text()):
80+
target = Path(entry["output"]).parent.name.removesuffix(".dir")
81+
flags[target] = _compile_flags(entry)
82+
for target in ("links_torch_cpu_library", "links_it_publicly", "consumer"):
83+
self.assertEqual(flags[target], flags["links_torch"], target)
84+
85+
86+
if __name__ == "__main__":
87+
unittest.main()

‎tools/cmake/Utils.cmake‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,22 @@ macro(find_package_torch)
277277
if(NOT TARGET torch)
278278
add_torch_to_cmake_prefix_path()
279279
find_package(Torch CONFIG REQUIRED)
280+
# TorchConfig gives its include dirs, compile flags and C++ standard to
281+
# torch only. Our targets link torch_cpu_library instead, so copy them over
282+
# and those targets compile as if they linked torch.
283+
foreach(_torch_property
284+
INTERFACE_INCLUDE_DIRECTORIES INTERFACE_COMPILE_OPTIONS
285+
INTERFACE_COMPILE_FEATURES
286+
)
287+
get_target_property(_torch_value torch ${_torch_property})
288+
if(_torch_value)
289+
set_property(
290+
TARGET torch_cpu_library
291+
APPEND
292+
PROPERTY ${_torch_property} ${_torch_value}
293+
)
294+
endif()
295+
endforeach()
280296
endif()
281297
endmacro()
282298

0 commit comments

Comments
 (0)