Skip to content

Commit 76d4404

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, and TORCH_CXX_FLAGS. torch_cpu_library, which our targets now link instead so that a CPU-only torch can load them, only passes on torch/include and has no TORCH_CXX_FLAGS. So two things changed for those targets. 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 adds both 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 76d4404

2 files changed

Lines changed: 100 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: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,19 @@ macro(find_package_torch)
277277
if(NOT TARGET torch)
278278
add_torch_to_cmake_prefix_path()
279279
find_package(Torch CONFIG REQUIRED)
280+
# TorchConfig adds TORCH_INCLUDE_DIRS and TORCH_CXX_FLAGS to torch only. Our
281+
# targets link torch_cpu_library instead, so add both to it as well and they
282+
# compile as if they linked torch.
283+
set_property(
284+
TARGET torch_cpu_library
285+
APPEND
286+
PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${TORCH_INCLUDE_DIRS}
287+
)
288+
set_property(
289+
TARGET torch_cpu_library
290+
APPEND
291+
PROPERTY INTERFACE_COMPILE_OPTIONS ${TORCH_CXX_FLAGS}
292+
)
280293
endif()
281294
endmacro()
282295

0 commit comments

Comments
 (0)