[mattiasbrand]

Summary:
An AOTInductor plugin is compiled separately from the program that loads it, so the two don't have to share a C++ standard library. Two places in the CUDA backend assume they do.

• Constants pass through a std::unordered_map. Cross-method weight sharing, mutable-state sessions and the CUDA graph host-constant check read a method's constants with AOTInductorModelContainerExtractConstantsMap, which hands the plugin a pointer to the host's map to fill in. That's only safe when both sides use the same standard library. It isn't safe for a Windows plugin cross-compiled with MinGW and loaded by an MSVC host, or a plugin loaded by a libc++ host. PyTorch's AOTI interface marks that call deprecated in favour of AOTInductorModelContainerExtractConstantsMapEntries, which returns a container-owned array of {name, handle} C structs. The backend now reads constants only through that call, so no C++ container crosses the plugin boundary.
• The plugin's C++ runtime has to already be in the process. On Linux an x86 plugin references libstdc++ (std::cerr, exception handling, operator new) without listing libstdc++.so.6 as a dependency. Loaded from a process that doesn't link libstdc++, dlopen fails with undefined symbol: _ZSt4cerr. When the host does link libstdc++, the plugin loads only if the host's dynamic symbol table happens to include every libstdc++ symbol the plugin uses. load_library now opens libstdc++.so.6 with RTLD_GLOBAL once before the first plugin, and returns AccessFailed if it can't, since such a plugin can't load anyway. The preload adds libstdc++ to the global lookup scope after the host, so symbols the host already defines still win.

Plugins that don't export the entries call still run, with reduced features:

• each method loads its own weights from the blob, without cross-method sharing;
• mutable-state sessions report NotSupported;
• CUDA graphs stay off, since their constants can't be checked for host memory.

Malformed results are never dereferenced: a null array with a nonzero count, a null name or handle, or an extraction error is rejected with InvalidProgram, or keeps graphs off.

test_cuda_mutable_state gains cases for the graph check (no entries call, no constants, a host constant, a device constant, malformed results) and for mutable-state extraction failures. It also builds outside fbcode as test_cuda_mutable_state_native.

Differential Revision: D129654474

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24818

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Cancelled Job

As of commit c7a73e6 with merge base dd38e81 ():

CANCELLED JOB - The following job was cancelled. Please retry:

• Test CUDA Builds / test-muse-glimmer-cuda-e2e-kquant-17gb-dflash-image-batching / linux-job (gh)
  Could not load credentials from any providers

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@mattiasbrand has exported this pull request. If you are a Meta employee, you can view the originating Diff in D129654474.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[shoumikhin]

@claude review effort=max

[claude]

Claude finished @shoumikhin's task in 58m 32s —— View job

---

PR Review: #24818

Request Changes: a plugin without AOTInductorModelContainerExtractConstantsMapEntries now silently loses cross-method weight sharing, mutable-state sessions and CUDA graphs, nothing shows the pinned PyTorch emits that symbol, and no test calls it.

Detailed review

Summary

This review covers all three commits in the stack (a42c487, 2b4739c, c7a73e6). Together they:

• read AOTI constants through a flat C ABI instead of a std::unordered_map*;
• preload libstdc++.so.6 with RTLD_GLOBAL on Linux;
• define the CUDA C++ targets in the xplat cell;
• add a cuda_lib pybinding with a GPU end-to-end test.

The blocking problem is that plugins without the new symbol now lose features without any warning. The other problems:

• a test target is built without CUDA_AVAILABLE;
• some Buck targets are missing settings that CMake or the commit messages say they have;
• no test calls the new extraction path;
• some code is duplicated or dead.

All 34 files were reviewed. Nothing was built or run. I couldn't read upstream PyTorch's interface.h from this runner, so what the entries API returns is inferred from how this PR uses it.

Code Quality

• A failed libstdc++.so.6 preload is logged only once (backends/cuda/runtime/platform/platform.cpp:82-91).
  • The result is cached in a function-local static, so only the first load_library call logs dlerror().
  • Every later call returns AccessFailed with no message, and the caller (cuda_backend.cpp:547-550) logs nothing either. So a second method's init fails with nothing in the log.
  • Fix this →
• The off-graph KV error message is now less useful (cuda_backend.cpp:599-604).
  • Every build now prints "requires an installed off-graph KV cache and a CUDA backend built with EXECUTORCH_BUILD_EXTENSION_LLM".
  • In a build that already has the LLM extension, that points at a flag that's already set. It also drops the kCacheKeyOption backend option that the old message told the caller to set. Pick the message based on the build.
  • Builds without the LLM extension now ignore a supplied cache key instead of returning NotSupported. That's harmless for programs without off-graph storage, but the description doesn't mention it.
  • Fix this →
• The same extract-and-validate loop is written three times: cuda_backend.cpp:1604-1630, cuda_mutable_state.cpp:287-308 and cuda_delegate_handle.h:367-385.
  • Each copy has its own error messages and handles a null array its own way.
  • One helper that returns a validated name → AtenTensorHandle map (or an error) would serve all three. It would also keep the SlimTensor cast out of the exported header cuda_delegate_handle.h.
  • Fix this →
• Dead code (Fix this →):
  • Unused member. AOTIDelegateHandle::extract_constants_map (aoti_delegate_handle.h:156) is still loaded at cuda_backend.cpp:303-304, but no production code reads it. Only the tests set it, to check that it isn't called.
  • Unreachable branch. cuda_backend.cpp:1572-1578 can never run, because line 1506 already takes the fallback when the symbol is null. Its Error log ("requires C-ABI constants extraction") also contradicts that silent fallback.
  • Redundant null checks. The new checks at cuda_backend.cpp:236-238 and 1500-1502, and at cuda_mutable_state.cpp:258-264, test values that can't be null there:
    • handle comes from new at line 556;
    • named_data_map is already dereferenced at line 487;
    • build_descriptors only runs after handle_has_symbols succeeds (line 604).

Infrastructure

• test_cuda_mutable_state_native is built without -DCUDA_AVAILABLE=1 (backends/cuda/runtime/targets.bzl:180-191).
  • Every fbcode CUDA test in the same file sets it (lines 211, 232, 253, 272, 290), and so does CMake's test_cuda_mutable_state (backends/cuda/CMakeLists.txt:490).
  • cuda_backend sets the define only for its own sources. The test file therefore compiles the stub DeviceTraits<CUDA> from slim/core/storage.h:204-233, which aborts, while the backend sources in the same binary compile the real one.
  • That breaks the one-definition rule. The linker keeps only one copy of each inline member, so in unoptimized builds the backend's CUDA storage calls can end up in the stub (ET_CHECK_MSG(false, "Build with CUDA_AVAILABLE=1 …")) on GPU runners.
  • Fix this →
• The rules for using the PUBLIC cuda_lib target appear only in a commit message.
  • a42c487 says two things:
    • any executable that loads a plugin needs -Wl,--export-dynamic;
    • LibTorch's aoti_torch_* definitions mean cuda_lib can't link into the same binary.
  • CMake passes the export flag on to every consumer automatically (aoti_common PUBLIC, backends/aoti/CMakeLists.txt:53-55).
  • In Buck, only run_cuda_program sets it (extension/pybindings/test/BUCK:103-104). Nothing on cuda_lib (extension/pybindings/BUCK:74-82) or common_shims_slim enforces or mentions either rule. The next binary built like run_cuda_program will fail with undefined symbol: aoti_torch_*.
  • Consider adding exported_linker_flags to //executorch/backends/aoti:common_shims_slim, or a dynamic list limited to aoti_torch_*. Also put a comment on cuda_lib about LibTorch.
• The xplat CUDA targets lack compatible_with: extension/cuda/targets.bzl, backends/aoti/slim/cuda/targets.bzl and backends/aoti/slim/c10/cuda/targets.bzl.
  • Their BUCK files now call define_common_targets() in both cells, but none of these targets sets compatible_with = ["ovr_config//cuda:has_cuda"].
  • The 2b4739c message says they have it, and the cuda/runtime targets do get it, through cuda_kwargs.
  • The comment this PR removes from extension/cuda/BUCK warned that in xplat caller_stream "cannot compile ("cuda_runtime.h not found")".
  • Unless the xplat target behind cuda-lazy is itself limited to has_cuda (I couldn't check), CPU-only wildcard builds will try to compile caller_stream.cpp, current_stream.cpp, device_guard.cpp, cuda_allocator.cpp and guard.cpp.
  • Fix this →

Testing

• No test runs the new extraction path in load_constants_with_cache (cuda_backend.cpp:1504-1630).
  • Neither the fallback for a missing symbol nor the InvalidProgram rejections at lines 1614-1627 is tested.
  • The fake-handle tests cover only describe_host_constants_for_graph and mutable state.
• The end-to-end test never calls the new symbol.
  • test_pybindings_cuda_lib.py exports one method with no backend options. So weight_sharing_across_methods_ stays false (cuda_backend.cpp:1367), init takes load_constants_legacy, and CUDA graphs and mutable state stay off.
  • The test would still pass if the pinned PyTorch didn't emit AOTInductorModelContainerExtractConstantsMapEntries.
  • Add a two-method case with weight sharing on, or at least check that a freshly exported AOTI .so exports the symbol, so a PyTorch pin without it fails CI.
  • Fix this →

API Design

• The new C ABI is described in a single line (backends/aoti/aoti_delegate_handle.h:117): "Extracts borrowed entries through a flat C ABI; the container owns the array." The code relies on two things that line doesn't say:

  • entries[i].name is the original FQN.
    • cuda_backend.cpp:1637 and cuda_mutable_state.cpp:313 look entries up by FQN. But the same AOTInductorConstantMapEntry struct carries internal names into UpdateUserManagedConstantBufferPairs (cuda_backend.cpp:1696).
    • If the extraction returned internal names, every lookup at line 1637 would miss and hit the continue at line 1641. Sharing would then switch off without any error.
  • How long the array stays valid: until the next extract, a constants update, or deletion of the container. The comment also doesn't say whether folded constants are included.

  Document both, and cite the upstream PyTorch declaration this mirrors. My confidence here is medium, since I couldn't read upstream. Fix this →

Backward Compatibility

• Plugins without AOTInductorModelContainerExtractConstantsMapEntries lose three features, with no warning. The description calls these "reduced features", but at runtime nothing reports them. No code reads the old map API any more. So a plugin compiled by a PyTorch that doesn't emit the new symbol now behaves as follows (this includes the plugin inside any .pte exported before the symbol existed):

  • Weights are no longer shared, and nothing is logged.
    • cuda_backend.cpp:1504-1510 falls back to load_constants_legacy, so every method uploads its own copy of the weights.
    • Weight sharing is turned on by qwen35_moe_engine.cpp:125, gemma4_31b_engine.cpp:143 and muse_glimmer_engine.cpp:319, and by cuda_executor.h:69, where it defaults to true.
    • For those multi-method models this puts a second copy of the weights in device memory. That is likely to run out of memory, with nothing pointing at the cause.
  • Mutable-state sessions fail with NotSupported, and nothing is logged (cuda_mutable_state.cpp:103-107, 227-229).
  • CUDA graphs are off. The only report is an Info log saying "its constants could not be listed" (cuda_delegate_handle.h:364-366, cuda_backend.cpp:643-648).

  This also applies on libstdc++ hosts loading libstdc++ plugins, where the old map API was safe.

  Neither torch_pin.py nor .ci/docker/ci_commit_pins/pytorch.txt changes, and nothing in the repo shows which PyTorch version emits the symbol. Please:

  • confirm that the pinned nightly emits it, and bump the pin if it doesn't;
  • log a Warning at each fallback, naming the missing symbol and the first PyTorch version that has it.

  If older artifacts matter, the map path could also stay as the fallback when __GLIBCXX__ is defined.

  Fix this →

Specific Comments

• backends/cuda/runtime/cuda_delegate_handle.h:360: "Compiled plugins return flat entries so C++ containers stay host-owned" describes the extraction ABI, not this function. It belongs on the typedef.
• backends/aoti/aoti_delegate_handle.h:157-158: extract_constants_map_entries{nullptr} is the only member with a default initializer. Give all the function pointers one, or none.
• backends/cuda/runtime/cuda_kv_cache.h:196-199: attach_offgraph_kv_cache is declared outside the #if at line 201, although its comment says it's defined only with the LLM extension. Moving it inside turns a link error into a compile error.
• backends/cuda/runtime/platform/platform.cpp:80-81:
  • The comment doesn't say why the runtime is loaded RTLD_GLOBAL rather than left to the plugin.
  • Line 85 should name libstdc++.so.6.
  • The description talks about x86 plugins, but the guard is __linux__, so it also runs on aarch64 (Jetson). Say whether those plugins need it too.
• backends/cuda/runtime/test/test_cuda_mutable_state.cpp:224, 334, 348: the test names call the same API both "SafeExtraction" and "EntryExtraction".
• backends/aoti/tests/test_common_shims_slim_link.cpp:17-46: the hardcoded list of 28 shims will drift from common_shims_slim.h. Either say it's a sample or point at the header.
• backends/cuda/runtime/targets.bzl:44: "use scalar Half conversions, not host SIMD" leaves out the real reason given in 2b4739c. c10 exports CPU_CAPABILITY_AVX2/AVX512 on x86, and Half.h then pulls host AVX2 code into nvcc compiles.
• backends/cuda/CMakeLists.txt:422-424 and backends/cuda/runtime/targets.bzl:133-135: these export EXECUTORCH_BUILD_EXTENSION_LLM, which is a build-option name, to every dependent. Any #if on it that a dependent adds later will flip too. A macro specific to this backend would avoid that.
• Description and commit messages:
  • c7a73e6 says an extraction error is rejected with InvalidProgram. But both call sites return the extractor's own error, and InvalidEntryExtractionPreventsSessions expects Error::Internal (test_cuda_mutable_state.cpp:392-393).
  • 2b4739c says "the CMake build is untouched" right after it describes the PRIVATE → PUBLIC change.
  • 2b4739c also says "fbcode builds keep the same … flags". Yet fbcode dependents of cuda_backend now inherit -DEXECUTORCH_BUILD_EXTENSION_LLM through exported_preprocessor_flags.

Recommendation

Request Changes.

• Blocking (Backward Compatibility): confirm the pinned PyTorch emits AOTInductorModelContainerExtractConstantsMapEntries, and log a warning at each of the three fallbacks. If the pin already emits it, only the warnings remain.
• Should land with it: the one-line CUDA_AVAILABLE fix for test_cuda_mutable_state_native, and a test that calls the entries path.
• Not blocking: everything else.

---