Skip to content

Conversation

@Gasoonjia
Copy link
Contributor

Summary:

Summary

This diff consolidates the backend functionality into a single target //executorch/backends/aoti:aoti_backend and simplifies the cuda backend target by making it dependent on the consolidated backend target.

The following changes are made in this diff:

  • Creation of a new target //executorch/backends/aoti:aoti_backend in fbcode/executorch/backends/aoti/targets.bzl which includes the necessary dependencies for the AOTI backend.
  • Update of the //executorch/backends/cuda:cuda_backend target in fbcode/executorch/backends/cuda/TARGETS to depend on the new //executorch/backends/aoti:aoti_backend target instead of individual AOTI backend dependencies.
  • Creation of a new file fbcode/executorch/backends/aoti/aoti_backend.py which imports the necessary dependencies and passes for the AOTI backend.
  • Simplification of the xplat/executorch/backends/cuda/cuda_backend.py file by removing unnecessary imports and using the new AotiBackend class from the aoti_backend.py file.
    ghstack-source-id: 319556735

Reviewed By: larryliu0820

Differential Revision: D85704977

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 3, 2025

🔗 Helpful Links

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

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

❌ 4 New Failures, 1 Cancelled Job

As of commit 57e8ffe with merge base d2c011e (image):

NEW FAILURES - The following jobs have failed:

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

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 3, 2025
@meta-codesync
Copy link

meta-codesync bot commented Nov 3, 2025

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

Copy link
Contributor

@larryliu0820 larryliu0820 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

@github-actions
Copy link

github-actions bot commented Nov 3, 2025

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.

@Gasoonjia Gasoonjia changed the title backend.py (#15430) [aoti-backend-consolidation 2/3] backend.py Nov 3, 2025
blob_data = f.read()

named_data_store.add_named_data(
method_name + "_weights_blob", blob_data, 1, "aoti_metal_blob"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the new name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh i called it mps now instead of metal; thanks let me solve it

named_data_store.add_named_data(method_name + "_so_blob", so_data, 1, None)
weights_blob_data_type = f"aoti_{device_name}_blob"
named_data_store.add_named_data(
method_name + "_weights_blob", blob_data, 1, weights_blob_data_type
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's the blob name in the new setting @manuelcandales


# Add SO and weights blob separately
named_data_store.add_named_data(method_name + "_so_blob", so_data, 1, None)
weights_blob_data_type = f"aoti_{device_name}_blob"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, this will now be aoti_mps_blob, so, need to update this in metal.yml (uses aoti_metal_blob)

subclasses.update(_get_all_final_backend_details_subclasses(subclass))
return subclasses

backend_name_to_subclass = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember it was discussed at the beginning of the delegate meeting - the backend implementation should be final and we didn't want to get involve in nested backends

# All backend implementation are final, so we don't need to consider nested subclasses.
it's hard to guard in python though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the example we put, we did try to put a final

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are shared logic, maybe put up a shared folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for comments. I make the AOTIBackend as a collector of sharing logics but not a actual backend, and make CUDABackend and MetalBackend inherit from both AOTIBackend and BackendDetails to avoid making any update on backend_api.py.

@experimental(
"This API and all of aoti-driven backend related functionality are experimental."
)
class AotiBackend(ABC):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is AotiBackend an actual class or an abstract class?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading, it seems like we will just have metal backend and cuda backend as the actual backend?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it is just a abstract class not a real backend for meet backend_details' requirement

setup.py Outdated

if is_linux_x86():
os.environ["EXECUTORCH_BUILDING_WHEEL"] = "1"
from backends.qualcomm.scripts.download_qnn_sdk import _download_qnn_sdk
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mengwei has this PR #15546 maybe can coordinate how to land together or maybe after he merged his change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have reverted my changes

cccclai added a commit to cccclai/executorch-1 that referenced this pull request Nov 14, 2025
Summary: pytorch#15528 initially wanted to subclass a backend.. It was currently already guarded by https://github.com/pytorch/executorch/blob/main/exir/backend/backend_api.py#L111-L112 meaning that subclass will not show up.  However it's not super obvious so we want to guard by disallowing subclass at all

Differential Revision: D87105211
cccclai added a commit to cccclai/executorch-1 that referenced this pull request Nov 14, 2025
Summary:

pytorch#15528 initially wanted to subclass a backend.. It was currently already guarded by https://github.com/pytorch/executorch/blob/main/exir/backend/backend_api.py#L111-L112 meaning that subclass will not show up.  However it's not super obvious so we want to guard by disallowing subclass at all

Reviewed By: Gasoonjia

Differential Revision: D87105211
cccclai added a commit that referenced this pull request Nov 15, 2025
Summary: #15528 initially
wanted to subclass a backend.. It was currently already guarded by
https://github.com/pytorch/executorch/blob/main/exir/backend/backend_api.py#L111-L112
meaning that subclass will not show up. However it's not super obvious
so we want to guard by disallowing subclass at all

Differential Revision: D87105211
@mergennachin mergennachin requested a review from Copilot November 17, 2025 16:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR consolidates AOTI (AOT Inductor) backend functionality by creating a shared base class (AotiBackend) that can be reused across different device backends (CUDA, Metal). The refactoring reduces code duplication and standardizes the compilation workflow.

Key Changes:

  • Created a new AotiBackend mixin class that provides common AOTI compilation logic
  • Refactored CUDA and Metal backends to inherit from AotiBackend instead of duplicating code
  • Updated build targets to reference the consolidated backend dependency

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
backends/aoti/aoti_backend.py New base class providing common AOTI compilation functionality
backends/cuda/cuda_backend.py Simplified to use AotiBackend with CUDA-specific configuration
backends/apple/metal/metal_backend.py Simplified to use AotiBackend with Metal-specific configuration
backends/aoti/targets.bzl Added build target for the new aoti_backend module
backends/cuda/TARGETS Updated to depend on consolidated aoti_backend target
setup.py Moved platform detection function to avoid import errors
examples/models/whisper/CMakeLists.txt Updated target name from aoti_cuda to aoti_cuda_backend
examples/models/gemma3/CMakeLists.txt Updated target name from aoti_cuda to aoti_cuda_backend
extension/llm/tokenizers Updated subproject commit reference

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@staticmethod
def get_supported_fallback_kernels() -> Dict[str, Any]:
return {
"aoti_torch_mps_addmm_out": None,
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kernel 'aoti_torch_mps_addmm_out' was added to the supported fallback kernels but was not present in the original code. Verify this kernel is intentionally supported and not an accidental addition during refactoring.

Suggested change
"aoti_torch_mps_addmm_out": None,

Copilot uses AI. Check for mistakes.
Summary:

This diff consolidates the backend functionality into a single target `//executorch/backends/aoti:aoti_backend` and simplifies the cuda backend target by making it dependent on the consolidated backend target.

The following changes are made in this diff:

*   Creation of a new target `//executorch/backends/aoti:aoti_backend` in `fbcode/executorch/backends/aoti/targets.bzl` which includes the necessary dependencies for the AOTI backend.
*   Update of the `//executorch/backends/cuda:cuda_backend` target in `fbcode/executorch/backends/cuda/TARGETS` to depend on the new `//executorch/backends/aoti:aoti_backend` target instead of individual AOTI backend dependencies.
*   Creation of a new file `fbcode/executorch/backends/aoti/aoti_backend.py` which imports the necessary dependencies and passes for the AOTI backend.
*   Simplification of the `xplat/executorch/backends/cuda/cuda_backend.py` file by removing unnecessary imports and using the new `AotiBackend` class from the `aoti_backend.py` file.
ghstack-source-id: 319556735

Reviewed By: larryliu0820

Differential Revision: D85704977
@meta-codesync
Copy link

meta-codesync bot commented Nov 20, 2025

@Gasoonjia has imported this pull request. If you are a Meta employee, you can view this in D85704977.

Gasoonjia and others added 2 commits November 20, 2025 17:05
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Gasoonjia Gasoonjia merged commit 9e7e17c into main Nov 21, 2025
161 of 168 checks passed
@Gasoonjia Gasoonjia deleted the export-D85704977 branch November 21, 2025 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants