Skip to content

Commit 6627a2d

Browse files
committed
Decide ATen mode before the wrapper consumes the dependency list
The Buck build still failed compiling generated sources that include ATen: ATen/core/TensorBase.h:1031:5: error: unknown type name 'requires' The check for "is this an ATen target" reads the external dependency list, but by the time it ran, an earlier step in the same wrapper had already removed that list after turning its entries into real dependencies. So every target looked like a non-ATen one, no target was ever raised to C++20, and the flag added for it was never added at all. Read the list first, then let the rest of the wrapper consume it. Test Plan: Traced the order in the wrapper: the step that removes the list runs before the step that read it. Reproduced that by running both against the target from the failure: it reports ATen mode with the list present and not once it is gone, which is exactly the sequence the wrapper produced. Reran the whole path in the new order over the target from the failure, an ATen target that exports its dependency instead of naming it directly, and a plain library. The first two now come away at C++20 and the plain one is untouched, so the embedded builds keep the C++17 they rely on. Also read the build system's own source to confirm the flag can win where it is placed: a target's flags are appended after the toolchain's, and its comment says they are last on purpose. So the earlier concern that the flag was being placed somewhere that could not take effect was unfounded; it simply was not being added. Shortened an overlong comment that failed the formatter. I cannot run this Buck build here, so CI has to confirm the generated libraries compile.
1 parent 340fd55 commit 6627a2d

2 files changed

Lines changed: 12 additions & 12 deletions

File tree

‎backends/aoti/common_shims.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ aoti_torch_get_dim(Tensor* tensor, int64_t* ret_dim);
6262
AOTI_SHIM_EXPORT AOTITorchError
6363
aoti_torch_get_numel(Tensor* tensor, int64_t* ret_numel);
6464

65-
// Reports whether the handle refers to a real tensor. PyTorch has an undefined
66-
// tensor state that ExecuTorch has no equivalent for, so this is a null check.
65+
// PyTorch has an undefined-tensor state with no equivalent here: null check.
6766
AOTI_SHIM_EXPORT AOTITorchError
6867
aoti_torch_is_defined(Tensor* tensor, bool* ret_is_defined);
6968

‎shim_et/xplat/executorch/build/runtime_wrapper.bzl‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,9 @@ def _has_pytorch_dep(dep_list):
146146
return False
147147

148148
def _is_aten_target(kwargs):
149-
"""Whether a target compiles against ATen, and so needs C++20.
149+
"""Whether a target compiles against ATen.
150150
151-
Keyed on the external dep names the build maps onto libtorch, rather than on
152-
a substring of the label: every ExecuTorch label contains "torch".
151+
Keyed on exact dep names, not a substring: every label contains "torch".
153152
"""
154153
aten_external_deps = ["c10", "libtorch", "libtorch_python", "torch-core-cpp"]
155154
for key in ["external_deps", "exported_external_deps"]:
@@ -278,21 +277,21 @@ def _patch_kwargs_cxx(kwargs):
278277
env.remove_platform_specific_args(kwargs)
279278
return _patch_kwargs_common(kwargs)
280279

281-
def _patch_aten_mode_std(kwargs):
280+
def _patch_aten_mode_std(kwargs, aten_mode):
282281
"""Raises an ATen-mode target to C++20, which PyTorch's headers require.
283282
284-
Set here rather than in the OSS-only interface layer, so the internal build
285-
gets it too, and as a plain compiler flag so it overrides the C++17 the build
286-
config applies to everything.
283+
A plain compiler flag, which the prelude places after the toolchain's.
287284
"""
288-
if _is_aten_target(kwargs):
285+
if aten_mode:
289286
kwargs["compiler_flags"] = kwargs.get("compiler_flags", []) + ["-std=c++20"]
290287
return kwargs
291288

292289
def _cxx_library_common(*args, **kwargs):
290+
# Before _patch_kwargs_cxx, which consumes external_deps.
291+
aten_mode = _is_aten_target(kwargs)
293292
_patch_kwargs_cxx(kwargs)
294293
_patch_build_mode_flags(kwargs)
295-
_patch_aten_mode_std(kwargs)
294+
_patch_aten_mode_std(kwargs, aten_mode)
296295

297296
env.patch_platform_build_mode_flags(kwargs)
298297
env.patch_headers(kwargs)
@@ -317,9 +316,11 @@ def _cxx_library(*args, **kwargs):
317316
_cxx_library_common(*args, **kwargs)
318317

319318
def _cxx_binary_helper(*args, **kwargs):
319+
# Before _patch_kwargs_cxx, which consumes external_deps.
320+
aten_mode = _is_aten_target(kwargs)
320321
_patch_kwargs_cxx(kwargs)
321322
_patch_build_mode_flags(kwargs)
322-
_patch_aten_mode_std(kwargs)
323+
_patch_aten_mode_std(kwargs, aten_mode)
323324
env.patch_platform_build_mode_flags(kwargs)
324325
env.patch_cxx_compiler_flags(kwargs)
325326

0 commit comments

Comments
 (0)