Arm backend: Add shared GPU runtime

[wwwind]

This is Phase 2 of RFC #19298: shared runtime support for allowing
the Vulkan and VGF delegates to resolve and safely share a Vulkan context.

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• linux.dgx.b200 runner queuing exceeds 4 hours and dgxb200-05 "CUDA not available"

⏳ No Failures, 2 Pending

As of commit f6309b9 with merge base bb2683b ():
💚 Looks good so far! There are no failures yet. 💚

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

[digantdesai]

@claude review this PR.

[claude]

Claude finished @digantdesai's task in 4m 11s —— View job

---

PR Review: Arm backend — Add shared GPU runtime

• Gather context (diff, changed files)
• Read runtime source (SharedGpuContext, Registry, RuntimeConfig)
• Read build files (CMake, BUCK, targets.bzl)
• Read tests
• Post review feedback

This is Phase 1 of the shared-GPU RFC (#19298): a backend-neutral, process-wide registry that lets the VGF and Vulkan delegates share one VkInstance/VkDevice/VkQueue. Overall this is a clean, well-documented, thoughtfully-tested contribution — the concurrency invariants are explicitly stated and the tests exercise them directly (creator-runs-once, lifetime-anchor released outside the lock, queue serialization). Nice work.

I found one correctness inconsistency worth addressing, plus a few minor points.

---

Correctness

1. Stale context released while holding mutex_ in lookup_or_create() — backends/gpu_shared/runtime/SharedGpuContextRegistry.cpp:87

The class invariant is stated at SharedGpuContextRegistry.h:65-66: "Never release a SharedGpuContext/lifetime_anchor while this mutex is held: backend teardown may re-enter the registry." Both lookup() (lines 52-61) and unregister_context() (lines 179-186) carefully move the context out and reset() it only after unlocking.

But in lookup_or_create() the reset happens inside the unique_lock:

    if (entry->context && entry->context->is_valid()) {
      return entry->context;
    }

    entry->context.reset();   // line 87 — runs under unique_lock(mutex_)
    entry->creating = true;

If the entry holds a stale (invalidated) context and this is its last reference, the SharedGpuContext destructor — and therefore the lifetime_anchor destructor, which the header says "may perform backend/application Vulkan teardown" — runs under mutex_. Since mutex_ is a non-recursive std::mutex, any re-entry into the registry from that teardown (the exact scenario the comments warn about) self-deadlocks.

Suggested fix: move the stale context into a local before reset(), and let it drop after the lock is released (mirroring the stale_context pattern already used in lookup()):

    SharedGpuContextPtr stale = std::move(entry->context);
    entry->creating = true;
  }  // release lock
  stale.reset();

Fix this →

---

Minor / questions

2. clear_for_testing() can orphan an in-flight creator — SharedGpuContextRegistry.cpp:190

If a lookup_or_create() is mid-create_fn (so creating == true) when clear_for_testing() runs, it sets creating = false, notifies, then swaps registry_ out. The creator then reacquires the lock and writes entry->context into an Entry that is no longer in registry_, so the result is silently dropped. This is test-only and the tests serialize around it, so it's not a production concern — but a one-line comment noting clear_for_testing() must not race live creators would prevent a confusing future test flake.

3. Hash combine reuses token_hash in the mix term — SharedGpuContextRegistry.cpp:24-31

return token_hash ^
    (group_hash + 0x9e3779b9 + (token_hash << 6) + (token_hash >> 2));

The boost hash_combine idiom shifts the accumulator (here group_hash), not the incoming value. As written the group_id contribution is weakly mixed. Functionally correct (equality is still exact), just slightly weaker distribution — worth a glance if keys with identical tokens but different group_ids are common.

4. Installing the full Vulkan-Headers tree into the package include root — backends/gpu_shared/CMakeLists.txt:87-89

install(DIRECTORY .../Vulkan-Headers/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) drops the entire vendored Vulkan header set into the installed package's top-level include dir. The rationale (public installed header includes <vulkan/vulkan.h>) is sound, but since SharedGpuContext.h is a public header exposing VkInstance/VkQueue in its API, consider whether this is the intended coupling — a consumer of the installed ExecuTorch package now gets a full Vulkan SDK in its include path, potentially shadowing a system one. Might be worth a note in the RFC/follow-up on how this composes with the Vulkan backend's own header handling.

5. Root gate EXECUTORCH_BUILD_VULKAN OR EXECUTORCH_BUILD_VGF — CMakeLists.txt:823

Good that it's gated. Since the subdir hard-FATAL_ERRORs when the Vulkan-Headers submodule is missing (backends/gpu_shared/CMakeLists.txt:19-26), this now makes the submodule a hard requirement for any VGF-only build too. That matches backends/arm/CMakeLists.txt which already requires it for VGF, so this is consistent — just calling it out.

---

Things done well

• Lifetime/ownership contract is explicit and matches the tests (UnregisterKeepsLifetimeAnchorAliveWhileContextIsReferenced, UnregisterDestroysLifetimeAnchorOutsideRegistryLock).
• parse_shared_gpu_runtime_config correctly distinguishes NotFound (keep default) from other errors (propagate), and validates empty-token-when-enabled.
• Runtime-vs-CompileSpec separation is clearly documented in both the header and README — a good call for a deployment-time concern.
• The EXECUTORCH_GPU_SHARED_API visibility handling + SHARED-on-purpose rationale correctly addresses the "one registry per process across DSOs" requirement.

Only item I'd consider blocking is #1 (the release-under-lock), since it can deadlock exactly the re-entrant teardown path the design is built around. The rest are minor.
· branch unified_gpu

[wwwind]

@digantdesai Do you want me to fix this one blocking issue that is found by Claude or all minor issues as well ?

[digantdesai]

LGTM at a high level, left some comments. RFC #2 right, not #1? Let's change the name though. Re. Claude comments, seems relevant and useful, your call on how many to fix, seem high to low value to me.

Is the long term goal also to remove delegate specific stuff like "set_and_get_external_adapter" in favor of this even when not shared?

[wwwind]

@digantdesai Thank you for the review. I addressed the blocking issues. Otherwise seems irrelevant in general or insignificant.

Re: set_and_get_external_adapter - Yes, in my view, long term these concepts should converge probably. But I don't change Vulkan ET in this PR deliberately. It is going to be Phase 3.

[digantdesai]

sorry I forgot to press submit :(

[wwwind]

@digantdesai I think everything is ready for the re-review.
I run all tests locally and they are green.

[digantdesai]

Thanks.