Skip to content

Commit aaac91b

Browse files
larryliu0820facebook-github-bot
authored andcommitted
Add a namespace for ATen mode (#9894)
Summary: Pull Request resolved: #9894 ## Context As titled. This is an effort trying to solve a big pain point for ATen mode users: duplicate symbols and duplicate operators. A typical duplicate symbol issue looks like: ``` ld.lld: error: duplicate symbol: executorch::runtime::Method::get_num_external_constants() >>> defined at __stripped__/method.cpp.pic.stripped.o:(executorch::runtime::Method::get_num_external_constants()) in archive buck-out/v2/gen/fbcode/712c6d0a4cb497c7/executorch/runtime/executor/__program_no_prim_ops_aten__/libprogram_no_prim_ops_aten.stripped.pic.a >>> defined at __stripped__/method.cpp.pic.stripped.o:(.text._ZN10executorch7runtime6Method26get_num_external_constantsEv+0x0) in archive buck-out/v2/gen/fbcode/712c6d0a4cb497c7/executorch/runtime/executor/__program_no_prim_ops__/libprogram_no_prim_ops.stripped.pic.a ``` [User post](https://fb.workplace.com/groups/pytorch.edge.users/permalink/1727735561430063/) This is caused by user depending on both `program_no_prim_ops` and `program_no_prim_ops_aten`. The issue happens because both libraries define symbols like: `executorch::runtime::Method` and they transitively depend on different definitions of `Tensor` and other types, see `exec_aten.h`. The other common issue is re-registering operators: ``` buck2 test //arvr/libraries/wristband/tsn/transformers:TorchstreamTransformer -- --print-passing-details File changed: fbsource//xplat/executorch/build/fb/clients.bzl File changed: fbsource//xplat/executorch File changed: fbcode//executorch/build/fb/clients.bzl 16 additional file change events ⚠ Listing failed: fbsource//arvr/libraries/wristband/tsn/transformers:TorchstreamTransformerTestFbcode Failed to list tests. Expected exit code 0 but received: ExitStatus(unix_wait_status(134)) STDOUT: STDERR:E 00:00:00.000543 executorch:operator_registry.cpp:86] Re-registering aten::sym_size.int, from /data/sandcastle/boxes/fbsource/buck-out/v2/gen/fbsource/cfdc20bd56300721/arvr/libraries/wristband/tsn/transformers/__TorchstreamTransformerTestFbcode__/./__TorchstreamTransformerTestFbcode__shared_libs_symlink_tre$ E 00:00:00.000572 executorch:operator_registry.cpp:87] key: (null), is_fallback: true F 00:00:00.000576 executorch:operator_registry.cpp:111] In function register_kernels(), assert failed (false): Kernel registration failed with error 18, see error log for details. ``` [User post](https://fb.workplace.com/groups/pytorch.edge.users/permalink/1691696305033989/) [User post 2](https://fb.workplace.com/groups/pytorch.edge.users/permalink/1510414646495490/) This is worse than duplicate symbols because it's a runtime error. This happens because a user depends on a kernel library built with ATen tensors and a kernel library built with ET tensor at the same time. For example, if a C++ binary depends on `//executorch/kernels/prim_ops:prim_ops_registry` and `//executorch/kernels/prim_ops:prim_ops_registry_aten` then this will happen. ## My proposal Add a new namespace to the symbols in ATen mode. `executorch::runtime::Method` --> `executorch::runtime::aten::Method` This way a C++ binary is allowed to depend on an ET library with ATen mode enabled and an ET library with ATen mode disabled. This is not BC breaking for OSS users, since ATen mode was never exposed. Differential Revision: D72440313
1 parent 95d38c4 commit aaac91b

37 files changed

+287
-248
lines changed

codegen/templates/RegisterCodegenUnboxedKernels.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
// JIT op registry instead of c10 dispatcher. JIT op registry only takes boxed
2323
// kernels, so we are calling unboxing functions in UnboxingFunctions.h to cast
2424
// arguments into C++ types (instead of IValue) and delegate to unboxed kernels.
25-
using KernelSpan =
26-
::executorch::runtime::Span<const ::executorch::runtime::Kernel>;
25+
using KernelSpan = ::executorch::runtime::Span<
26+
const ::executorch::ET_RUNTIME_NAMESPACE::Kernel>;
2727
namespace torch {
2828
namespace executor {
2929
namespace function {

extension/flat_tensor/flat_tensor_data_map.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ namespace extension {
3232
/**
3333
* A NamedDataMap implementation for FlatTensor-serialized data.
3434
*/
35-
class FlatTensorDataMap final : public executorch::runtime::NamedDataMap {
35+
class FlatTensorDataMap final
36+
: public executorch::ET_RUNTIME_NAMESPACE::NamedDataMap {
3637
public:
3738
/**
3839
* Creates a new DataMap that wraps FlatTensor data.

extension/flat_tensor/targets.bzl

+23-21
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,26 @@
11
load("@fbsource//xplat/executorch/build:runtime_wrapper.bzl", "runtime")
22

33
def define_common_targets():
4-
runtime.cxx_library(
5-
name = "flat_tensor_data_map",
6-
srcs = [
7-
"flat_tensor_data_map.cpp",
8-
],
9-
exported_headers = ["flat_tensor_data_map.h"],
10-
deps = [
11-
"//executorch/runtime/core:core",
12-
"//executorch/runtime/core:evalue",
13-
"//executorch/runtime/core:named_data_map",
14-
"//executorch/runtime/core/exec_aten:lib",
15-
"//executorch/runtime/core/exec_aten/util:tensor_util",
16-
],
17-
exported_deps = [
18-
"//executorch/extension/flat_tensor/serialize:flat_tensor_header",
19-
"//executorch/extension/flat_tensor/serialize:generated_headers",
20-
],
21-
visibility = [
22-
"//executorch/...",
23-
],
24-
)
4+
for aten_mode in [True, False]:
5+
aten_suffix = "_aten" if aten_mode else ""
6+
runtime.cxx_library(
7+
name = "flat_tensor_data_map" + aten_suffix,
8+
srcs = [
9+
"flat_tensor_data_map.cpp",
10+
],
11+
exported_headers = ["flat_tensor_data_map.h"],
12+
deps = [
13+
"//executorch/runtime/core:core",
14+
"//executorch/runtime/core:evalue",
15+
"//executorch/runtime/core:named_data_map" + aten_suffix,
16+
"//executorch/runtime/core/exec_aten:lib" + aten_suffix,
17+
"//executorch/runtime/core/exec_aten/util:tensor_util",
18+
],
19+
exported_deps = [
20+
"//executorch/extension/flat_tensor/serialize:flat_tensor_header",
21+
"//executorch/extension/flat_tensor/serialize:generated_headers",
22+
],
23+
visibility = [
24+
"//executorch/...",
25+
],
26+
)

extension/module/module.cpp

+10-7
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737
namespace executorch {
3838
namespace extension {
3939

40+
using ET_RUNTIME_NAMESPACE::MethodMeta;
41+
using ET_RUNTIME_NAMESPACE::Program;
42+
4043
namespace {
4144
runtime::Result<std::unique_ptr<runtime::DataLoader>> load_file(
4245
const std::string& file_path,
@@ -113,7 +116,7 @@ Module::Module(
113116
}
114117

115118
Module::Module(
116-
std::shared_ptr<runtime::Program> program,
119+
std::shared_ptr<Program> program,
117120
std::unique_ptr<runtime::MemoryAllocator> memory_allocator,
118121
std::unique_ptr<runtime::MemoryAllocator> temp_allocator,
119122
std::unique_ptr<runtime::EventTracer> event_tracer,
@@ -131,7 +134,7 @@ Module::Module(
131134
runtime::runtime_init();
132135
}
133136

134-
runtime::Error Module::load(const runtime::Program::Verification verification) {
137+
runtime::Error Module::load(const Program::Verification verification) {
135138
if (!is_loaded()) {
136139
// Load the program
137140
if (!data_loader_) {
@@ -156,10 +159,10 @@ runtime::Error Module::load(const runtime::Program::Verification verification) {
156159
}
157160
// else: either the map itself was provided or we have no data map, either
158161
// way no work to do.
159-
auto program = ET_UNWRAP_UNIQUE(
160-
runtime::Program::load(data_loader_.get(), verification));
161-
program_ = std::shared_ptr<runtime::Program>(
162-
program.release(), [](runtime::Program* pointer) { delete pointer; });
162+
auto program =
163+
ET_UNWRAP_UNIQUE(Program::load(data_loader_.get(), verification));
164+
program_ = std::shared_ptr<Program>(
165+
program.release(), [](Program* pointer) { delete pointer; });
163166
}
164167
return runtime::Error::Ok;
165168
}
@@ -224,7 +227,7 @@ runtime::Error Module::load_method(
224227
return runtime::Error::Ok;
225228
}
226229

227-
runtime::Result<runtime::MethodMeta> Module::method_meta(
230+
runtime::Result<MethodMeta> Module::method_meta(
228231
const std::string& method_name) {
229232
ET_CHECK_OK_OR_RETURN_ERROR(load_method(method_name));
230233
return methods_.at(method_name).method->method_meta();

extension/module/module.h

+13-9
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@
1919
namespace executorch {
2020
namespace extension {
2121

22+
using ET_RUNTIME_NAMESPACE::Method;
23+
using ET_RUNTIME_NAMESPACE::MethodMeta;
24+
using ET_RUNTIME_NAMESPACE::NamedDataMap;
25+
using ET_RUNTIME_NAMESPACE::Program;
26+
2227
/**
2328
* A facade class for loading programs and executing methods within them.
2429
*/
@@ -95,7 +100,7 @@ class Module {
95100
* @param[in] data_map_loader A DataLoader used for loading external weights.
96101
*/
97102
explicit Module(
98-
std::shared_ptr<runtime::Program> program,
103+
std::shared_ptr<Program> program,
99104
std::unique_ptr<runtime::MemoryAllocator> memory_allocator = nullptr,
100105
std::unique_ptr<runtime::MemoryAllocator> temp_allocator = nullptr,
101106
std::unique_ptr<runtime::EventTracer> event_tracer = nullptr,
@@ -116,8 +121,8 @@ class Module {
116121
*/
117122
ET_NODISCARD
118123
runtime::Error load(
119-
const runtime::Program::Verification verification =
120-
runtime::Program::Verification::Minimal);
124+
const Program::Verification verification =
125+
Program::Verification::Minimal);
121126

122127
/**
123128
* Checks if the program is loaded.
@@ -134,7 +139,7 @@ class Module {
134139
*
135140
* @returns Shared pointer to the program or nullptr if it's not yet loaded.
136141
*/
137-
inline std::shared_ptr<runtime::Program> program() const {
142+
inline std::shared_ptr<Program> program() const {
138143
return program_;
139144
}
140145

@@ -224,8 +229,7 @@ class Module {
224229
* @returns A method metadata, or an error if the program or method failed to
225230
* load.
226231
*/
227-
runtime::Result<runtime::MethodMeta> method_meta(
228-
const std::string& method_name);
232+
runtime::Result<MethodMeta> method_meta(const std::string& method_name);
229233

230234
/**
231235
* Execute a specific method with the given input values and retrieve the
@@ -473,20 +477,20 @@ class Module {
473477
std::vector<runtime::Span<uint8_t>> planned_spans;
474478
std::unique_ptr<runtime::HierarchicalAllocator> planned_memory;
475479
std::unique_ptr<runtime::MemoryManager> memory_manager;
476-
std::unique_ptr<runtime::Method> method;
480+
std::unique_ptr<Method> method;
477481
std::vector<runtime::EValue> inputs;
478482
};
479483

480484
std::string file_path_;
481485
std::string data_map_path_;
482486
LoadMode load_mode_{LoadMode::MmapUseMlock};
483-
std::shared_ptr<runtime::Program> program_;
487+
std::shared_ptr<Program> program_;
484488
std::unique_ptr<runtime::DataLoader> data_loader_;
485489
std::unique_ptr<runtime::MemoryAllocator> memory_allocator_;
486490
std::unique_ptr<runtime::MemoryAllocator> temp_allocator_;
487491
std::unique_ptr<runtime::EventTracer> event_tracer_;
488492
std::unique_ptr<runtime::DataLoader> data_map_loader_;
489-
std::unique_ptr<runtime::NamedDataMap> data_map_;
493+
std::unique_ptr<NamedDataMap> data_map_;
490494

491495
protected:
492496
std::unordered_map<std::string, MethodHolder> methods_;

extension/module/targets.bzl

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def define_common_targets():
2525
"//executorch/extension/memory_allocator:malloc_memory_allocator",
2626
"//executorch/extension/data_loader:file_data_loader",
2727
"//executorch/extension/data_loader:mmap_data_loader",
28-
"//executorch/extension/flat_tensor:flat_tensor_data_map",
28+
"//executorch/extension/flat_tensor:flat_tensor_data_map" + aten_suffix,
2929
],
3030
exported_deps = [
3131
"//executorch/runtime/executor:program" + aten_suffix,

kernels/prim_ops/register_prim_ops.cpp

+3-4
Original file line numberDiff line numberDiff line change
@@ -381,14 +381,13 @@ static Kernel prim_ops[] = {
381381

382382
};
383383

384-
executorch::runtime::Span<const executorch::runtime::Kernel> kernel_span(
385-
prim_ops,
386-
prim_ops + sizeof(prim_ops) / sizeof(Kernel));
384+
executorch::runtime::Span<const executorch::ET_RUNTIME_NAMESPACE::Kernel>
385+
kernel_span(prim_ops, prim_ops + sizeof(prim_ops) / sizeof(Kernel));
387386

388387
// Return value not used. Keep the static variable assignment to register
389388
// operators in static initialization time.
390389
auto success_with_kernel_reg =
391-
executorch::runtime::register_kernels(kernel_span);
390+
executorch::ET_RUNTIME_NAMESPACE::register_kernels(kernel_span);
392391

393392
} // namespace
394393
} // namespace function

runtime/backend/backend_execution_context.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include <executorch/runtime/core/memory_allocator.h>
1313

1414
namespace executorch {
15-
namespace runtime {
15+
namespace ET_RUNTIME_NAMESPACE {
1616

1717
/**
1818
* BackendExecutionContext will be used to inject run time context.
@@ -68,13 +68,13 @@ class BackendExecutionContext final {
6868
const char* method_name_ = nullptr;
6969
};
7070

71-
} // namespace runtime
71+
} // namespace ET_RUNTIME_NAMESPACE
7272
} // namespace executorch
7373

7474
namespace torch {
7575
namespace executor {
7676
// TODO(T197294990): Remove these deprecated aliases once all users have moved
7777
// to the new `::executorch` namespaces.
78-
using ::executorch::runtime::BackendExecutionContext;
78+
using ::executorch::ET_RUNTIME_NAMESPACE::BackendExecutionContext;
7979
} // namespace executor
8080
} // namespace torch

runtime/backend/backend_init_context.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
*/
88

99
#pragma once
10+
#include <executorch/runtime/core/event_tracer.h>
1011
#include <executorch/runtime/core/memory_allocator.h>
1112
#include <executorch/runtime/core/named_data_map.h>
1213

1314
namespace executorch {
14-
namespace runtime {
15-
15+
namespace ET_RUNTIME_NAMESPACE {
1616
/**
1717
* BackendInitContext will be used to inject runtime info for to initialize
1818
* delegate.
@@ -70,13 +70,13 @@ class BackendInitContext final {
7070
const NamedDataMap* named_data_map_ = nullptr;
7171
};
7272

73-
} // namespace runtime
73+
} // namespace ET_RUNTIME_NAMESPACE
7474
} // namespace executorch
7575

7676
namespace torch {
7777
namespace executor {
7878
// TODO(T197294990): Remove these deprecated aliases once all users have moved
7979
// to the new `::executorch` namespaces.
80-
using ::executorch::runtime::BackendInitContext;
80+
using ::executorch::ET_RUNTIME_NAMESPACE::BackendInitContext;
8181
} // namespace executor
8282
} // namespace torch

runtime/backend/interface.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#include <executorch/runtime/backend/interface.h>
1010

1111
namespace executorch {
12-
namespace runtime {
12+
namespace ET_RUNTIME_NAMESPACE {
1313

1414
// Pure-virtual dtors still need an implementation.
1515
BackendInterface::~BackendInterface() {}
@@ -66,5 +66,5 @@ Result<const char*> get_backend_name(size_t index) {
6666
return registered_backends[index].name;
6767
}
6868

69-
} // namespace runtime
69+
} // namespace ET_RUNTIME_NAMESPACE
7070
} // namespace executorch

runtime/backend/interface.h

+9-9
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
#include <executorch/runtime/platform/compiler.h>
2323

2424
namespace executorch {
25-
namespace runtime {
25+
namespace ET_RUNTIME_NAMESPACE {
2626

2727
struct SizedBuffer {
2828
void* buffer;
@@ -150,19 +150,19 @@ size_t get_num_registered_backends();
150150
*/
151151
Result<const char*> get_backend_name(size_t index);
152152

153-
} // namespace runtime
153+
} // namespace ET_RUNTIME_NAMESPACE
154154
} // namespace executorch
155155

156156
namespace torch {
157157
namespace executor {
158158
// TODO(T197294990): Remove these deprecated aliases once all users have moved
159159
// to the new `::executorch` namespaces.
160-
using ::executorch::runtime::Backend;
161-
using ::executorch::runtime::CompileSpec;
162-
using ::executorch::runtime::DelegateHandle;
163-
using ::executorch::runtime::get_backend_class;
164-
using ::executorch::runtime::register_backend;
165-
using ::executorch::runtime::SizedBuffer;
166-
using PyTorchBackendInterface = ::executorch::runtime::BackendInterface;
160+
using ::executorch::ET_RUNTIME_NAMESPACE::Backend;
161+
using ::executorch::ET_RUNTIME_NAMESPACE::CompileSpec;
162+
using ::executorch::ET_RUNTIME_NAMESPACE::DelegateHandle;
163+
using ::executorch::ET_RUNTIME_NAMESPACE::get_backend_class;
164+
using ::executorch::ET_RUNTIME_NAMESPACE::register_backend;
165+
using ::executorch::ET_RUNTIME_NAMESPACE::SizedBuffer;
166+
using PyTorchBackendInterface = ::executorch::ET_RUNTIME_NAMESPACE::BackendInterface;
167167
} // namespace executor
168168
} // namespace torch

runtime/core/event_tracer_hooks.h

+23-14
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
*/
3131

3232
namespace executorch {
33-
namespace runtime {
33+
namespace ET_RUNTIME_NAMESPACE {
3434
namespace internal {
3535

3636
/**
@@ -305,26 +305,35 @@ inline void event_tracer_set_bundled_input_index(
305305
}
306306

307307
} // namespace internal
308-
} // namespace runtime
308+
} // namespace ET_RUNTIME_NAMESPACE
309309
} // namespace executorch
310310

311311
namespace torch {
312312
namespace executor {
313313
namespace internal {
314314
// TODO(T197294990): Remove these deprecated aliases once all users have moved
315315
// to the new `::executorch` namespaces.
316-
using ::executorch::runtime::internal::event_tracer_begin_profiling_event;
317-
using ::executorch::runtime::internal::event_tracer_create_event_block;
318-
using ::executorch::runtime::internal::event_tracer_end_profiling_event;
319-
using ::executorch::runtime::internal::event_tracer_log_evalue;
320-
using ::executorch::runtime::internal::event_tracer_log_evalue_output;
321-
using ::executorch::runtime::internal::event_tracer_set_bundled_input_index;
322-
using ::executorch::runtime::internal::event_tracer_track_allocation;
323-
using ::executorch::runtime::internal::event_tracer_track_allocator;
324-
using ::executorch::runtime::internal::EventTracerProfileInstructionScope;
325-
using ::executorch::runtime::internal::EventTracerProfileMethodScope;
326-
using ::executorch::runtime::internal::EventTracerProfileOpScope;
327-
using ::executorch::runtime::internal::EventTracerProfileScope;
316+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
317+
event_tracer_begin_profiling_event;
318+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
319+
event_tracer_create_event_block;
320+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
321+
event_tracer_end_profiling_event;
322+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::event_tracer_log_evalue;
323+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
324+
event_tracer_log_evalue_output;
325+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
326+
event_tracer_set_bundled_input_index;
327+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
328+
event_tracer_track_allocation;
329+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
330+
event_tracer_track_allocator;
331+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
332+
EventTracerProfileInstructionScope;
333+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::
334+
EventTracerProfileMethodScope;
335+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::EventTracerProfileOpScope;
336+
using ::executorch::ET_RUNTIME_NAMESPACE::internal::EventTracerProfileScope;
328337

329338
} // namespace internal
330339
} // namespace executor

0 commit comments

Comments
 (0)