Skip to content

Arm backend: Add shared device to VGF backend. - #24795

Open
wwwind wants to merge 12 commits into
pytorch:mainfrom
wwwind:phase4_unified
Open

wwwind wants to merge 12 commits into
pytorch:mainfrom
wwwind:phase4_unified

Conversation

@wwwind

@wwwind wwwind commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This PR implements Phase 4 of [RFC #19298: Unified Vulkan and VGF GPU backend path in ExecuTorch](#19298).

Phase 4 integrates the Arm VGF backend with the shared Vulkan runtime, enabling VGF and Vulkan delegates to execute within a single ExecuTorch program using the same Vulkan device and queue.

Changes

  • Integrates VGF with the SharedVulkanDevice registry and shared-device configuration.
  • Enables VGF to reuse an existing Vulkan device while validating the required extensions, enabled features, and queue capabilities.
  • Adds synchronized access to the shared Vulkan queue.
  • Maintains separate command pools and resources for each VGF delegate, with correct device ownership and lifetime management.
  • Preserves standalone VGF execution when shared-device mode is not enabled.
  • Adds an end-to-end example demonstrating mixed VGF/Vulkan execution.

End-to-End Example

The new example in examples/arm/vgf_vulkan_shared/ demonstrates the following execution sequence:

Input [1, 3, 8, 8]
       |
       v
  avg_pool2d       VGF
       |
       v
    sigmoid        Vulkan
       |
       v
  x * x + 0.25     VGF
       |
       v
Output [1, 3, 4, 4]

The exporter generates a single .pte containing:

VgfBackend -> VulkanBackend -> VgfBackend

The example includes model export, a native/Android runner, shared-device identity verification, and numerical comparison against PyTorch reference outputs.

Both backends share the same VkDevice and queue, while retaining independent delegate execution. Inter-delegate zero-copy and fused GPU submissions are outside the scope of this PR.

Testing

  • Tested the mixed VGF/Vulkan example on a Xiaomi Android phone.
  • Validated the expected VGF → Vulkan → VGF execution sequence without CPU operator fallback.
  • Verified shared Vulkan device usage and numerical correctness against PyTorch reference outputs.

Reference

[RFC #19298 — Unified Vulkan and VGF GPU backend path in ExecuTorch](#19298)

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @rascani

Signed-off-by: Elena Zhelezina <elena.zhelezina@arm.com>
Change-Id: Idef95270b41ab60464d1027530d3a8ed760962a5
@pytorch-bot

pytorch-bot Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

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

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

❌ 5 New Failures, 10 Unrelated Failures

As of commit f183976 with merge base 94e7ca0 (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 9, 2026
@github-actions github-actions Bot added ciflow/trunk module: arm Issues related to arm backend labels Nov 9, 2026
@wwwind wwwind added release notes: arm Changes to the ARM backend delegate and removed release notes: arm Changes to the ARM backend delegate module: arm Issues related to arm backend labels Nov 9, 2026
Change-Id: I7793095a25e959507c5ae8a2d4acfe85db28cb30
@github-actions github-actions Bot added the module: arm Issues related to arm backend label Nov 9, 2026
@wwwind wwwind removed the module: arm Issues related to arm backend label Nov 9, 2026
@github-actions github-actions Bot added the module: arm Issues related to arm backend label Nov 9, 2026
@wwwind wwwind added the release notes: arm Changes to the ARM backend delegate label Nov 10, 2026
Change-Id: I89bf74c0ae52b98e1abdbbed03f9b0e2d8914b1e
@digantdesai

Copy link
Copy Markdown
Contributor

@claude review this PR.

@claude

claude Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Claude finished @digantdesai's task in 5m 25s —— View job


Summary: The shared-device integration is well structured, but CI never compiles the shared path (EXECUTORCH_VGF_SHARED_DEVICE=1), sharing only works if a VGF-capable device is registered before any delegate loads, and combined builds change standalone VGF behavior — so I recommend Request Changes.

Detailed review

PR Review: #24795

Summary

This PR moves the VGF backend onto the SharedVulkanDevice registry so VGF and Vulkan delegates in one program share a VkDevice and queue. There are four main problems:

  • CI never builds or tests the shared code path.
  • Mixed programs only work if a device is registered by hand before loading.
  • Combined builds change standalone VGF behavior, which the PR says it preserves.
  • The Vulkan-only instance version now depends on whether VGF is also built.

Three independent reviewers covered the diff. I then re-read each finding below against the code. The example README.md and export.py were only skimmed.

Infrastructure

  • CI never compiles the shared-device path. backends/arm/test/test_arm_backend.sh:527 configures -DEXECUTORCH_BUILD_VULKAN=OFF, so vgf_backend gets EXECUTORCH_VGF_SHARED_DEVICE=0 (backends/arm/CMakeLists.txt). Buck hard-codes =0 too (backends/arm/runtime/targets.bzl:137-141). As a result, CI never compiles or runs:

    • validate_device, create_device, with_shared_queue, the per-delegate command pool, the shared branch of vkml_allocate_basics, and the Runtime.cpp 1.3 switch;
    • the only behavioral test, LookupOnlyMissingDoesNotInitializeVulkan (inside #if, vgf_shared_device_test.cpp:115-122).

    What does run only covers option parsing and the vgf_shared_features_compatible check. EXECUTORCH_BUILD_VGF_SHARED_EXAMPLE defaults to OFF, and nothing in CI turns it on. The only evidence for VGF→Vulkan→VGF is one manual phone run. Please add a ctest configuration with VGF=ON VULKAN=ON that builds the shared code and the example runner, plus tests for rejecting an incompatible device, nested-lease detection, and destroy order. Fix this →

  • vgf_backend now always pulls in Vulkan dependencies, even in VGF-only builds. It links executorch_vulkan_shared_runtime PUBLIC unconditionally (backends/arm/CMakeLists.txt), and VGFSharedDevice.h (included from the public VGFSetup.h) includes backends/vulkan/runtime/vk_api/vk_api.h. VGF-only embedders now have to ship another DSO, and no wheel test covers loading it in a VGF-only build. In Buck, exported_deps lists only //executorch/backends/vulkan_shared:runtime and not the target that provides vk_api.h, so strict header checking will likely fail (suspected; not built).

  • The new option is declared in the wrong place. option(EXECUTORCH_BUILD_VGF_SHARED_EXAMPLE ...) sits inside backends/arm/CMakeLists.txt under if(EXECUTORCH_BUILD_VGF), not in tools/cmake/preset/default.cmake with the other build options. The example is also added from inside backends/arm, even though it depends on targets defined later (vulkan_backend, extension_module).

Thread Safety

  • VGF holds the shared queue lock for its whole execution.

    • execute() (VGFBackend.cpp:740-747) wraps all of execute_locked in with_shared_queue: input copies, debug dumps, submit, the fence wait and output copies.
    • init() (:695) likewise holds the lock across all of process_vgf, including pipeline creation.
    • The Vulkan Adapter takes the same mutex for its submits, so a Vulkan delegate on another thread is blocked for the full VGF run, not just the submit.
    • The backend-wide mutex_ is taken first. The lock order (mutex_ then queue mutex) is not documented anywhere.

    Narrowing the lock to vkQueueSubmit/fence handling would match "synchronized access to the shared queue".

  • Destroying a VGF delegate can stall on, or abort because of, Vulkan's work. ~VgfRepr (VGFSetup.h:318-329) always calls wait_for_device_idle_locked(), which in shared mode is vkQueueWaitIdle on the shared queue (:303-316). So every destroy, and every failed init, waits for in-flight Vulkan delegate work. ET_CHECK_MSG then aborts the process on any result other than VK_SUCCESS or VK_ERROR_DEVICE_LOST (for example VK_ERROR_OUT_OF_HOST_MEMORY) instead of returning an error.

Backward Compatibility

  • Combined builds change standalone VGF behavior. backends/arm/scripts/build_executor_runner_vkml.sh builds with both EXECUTORCH_BUILD_VULKAN=ON and EXECUTORCH_BUILD_VGF=ON, so the default VKML runner now always gets EXECUTORCH_VGF_SHARED_DEVICE=1, including VGF-only models with no shared options. In that mode:

    • vkml_allocate_basics (VGFBackend.cpp:1350-1366) uses vkcompute::vkapi::runtime_owner()'s instance. init_global_vulkan_runtime sets init_default_device = true (Runtime.cpp:332, :363-367), so the process creates an extra Vulkan Adapter and VkDevice that VGF never uses. That costs memory and startup time on mobile, and contradicts the is_available() comment about avoiding "a redundant VkDevice".
    • VGF also inherits the Vulkan runtime's instance setup (validation layers in VULKAN_DEBUG) instead of its own.
    • is_available() (:579-585) always returns true, even when shared mode is off. On a GPU without VK_ARM_tensors, the failure moves from the availability check to init().

    Consider choosing shared vs. legacy at runtime from the compile spec / runtime options, not from build flags alone.

  • Standalone VGF now accepts fewer devices and drivers, even with shared mode off. This applies whatever the build flags. These changes are not mentioned in the PR description:

    • The queue family must now have both COMPUTE and DATA_GRAPH_ARM (before, either was enough).
    • apiVersion >= 1.3 is now required.
    • A missing VK_KHR_maintenance5 or VK_KHR_deferred_host_operations used to log at Info and continue. It now returns VK_ERROR_EXTENSION_NOT_PRESENT (VGFBackend.cpp:1810-1818).
    • volkLoadDevice was removed for VGF-only builds too (:1838), so device calls go through loader dispatch. The reason for the removal (one global Volk table shared by several devices) only applies to combined builds.

    If these are intended fixes, please call them out. Otherwise, gate them on the shared path.

  • Building VGF changes the Vulkan backend's instance version. backends/vulkan/CMakeLists.txt adds the PRIVATE define EXECUTORCH_VGF_SHARED_DEVICE=1 to vulkan_compute_api, which switches Runtime.cpp:92-96 from VK_API_VERSION_1_1 to 1_3. Vulkan-only programs therefore behave differently depending on whether an unrelated backend was compiled in, and a 1.0 loader fails vkCreateInstance with VK_ERROR_INCOMPATIBLE_DRIVER. A RuntimeConfig field set by the caller would be cleaner than a cross-backend build macro.

API Design

  • Sharing depends on initialization order, and the required setup is undocumented.

    • With the default lookup_or_create mode, if a Vulkan delegate initializes first, ResolveSharedDevice.cpp (unchanged) registers the Vulkan Adapter's device. That device does not enable VK_ARM_tensors or VK_ARM_data_graph, so VGF's validate_device (VGFSharedDevice.cpp:76-87) fails the whole load with DelegateInvalidCompatibility.
    • It only works when VGF is first in the delegate table, or when the app pre-creates the device with create_only and loads both delegates with lookup_only, as runner.cpp:70-74 does.

    This makes manual pre-registration an implicit part of the contract. Either have Vulkan's create path request VGF's extensions and features when VGF is built in, or document (and enforce, with a clear error) that a VGF-capable device must be registered first.

Performance

  • Zero-copy is decided at build time on the shared path. create_device (VGFSharedDevice.cpp:163, :175) enables VK_EXT_external_memory_host based only on vgf_zero_copy_io_build_enabled() and ignores the ET_VGF_ZERO_COPY runtime opt-out that the standalone path honors. Separately, get_mapped_boundary_buffer returns NotSupported when two VGF delegates share an allocator (VGFBackend.cpp:470-478), which is exactly the case in the example's VGF→Vulkan→VGF program. Please note in the README that the example never uses zero-copy boundaries.

Recommendation

Request Changes

The shared-device path ships without any CI build or test. It needs a VGF=ON VULKAN=ON CI configuration and tests before merging. The order-dependent lookup_or_create behavior and the standalone-VGF behavior changes in combined builds (extra VkDevice, is_available() always true, stricter device requirements) should be fixed or explicitly scoped.

@wwwind wwwind added the help wanted Extra attention is needed label Nov 10, 2026
Change-Id: Icd2ef08c627f8e3437156613bb994b6a73b23d6a
Change-Id: Id06cc50c8e22a7ba3425a4eb8e75f5a2a624aa9f

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. help wanted Extra attention is needed module: arm Issues related to arm backend release notes: arm Changes to the ARM backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants