Skip to content

Add aten_lib to executorch_llama #10307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 20, 2025
Merged

Add aten_lib to executorch_llama #10307

merged 1 commit into from
Apr 20, 2025

Conversation

larryliu0820
Copy link
Contributor

Summary:
We now can link both ATen lib and portable lib into the same binary/.so file.

This makes us easy to use aten_mode lib as well as portable lib in the same bento kernel.

Reviewed By: tarun292

Differential Revision: D73282334

Copy link

pytorch-bot bot commented Apr 18, 2025

🔗 Helpful Links

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

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:

❌ 1 New Failure, 4 Unrelated Failures

As of commit 79e1a8f with merge base 1afb25e (image):

NEW FAILURE - The following job has failed:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

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.

@facebook-github-bot facebook-github-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 Apr 18, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D73282334

facebook-github-bot pushed a commit that referenced this pull request Apr 18, 2025
Summary:

We now can link both ATen lib and portable lib into the same binary/.so file.

This makes us easy to use aten_mode lib as well as portable lib in the same bento kernel.

Differential Revision: D73282334
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D73282334

facebook-github-bot pushed a commit that referenced this pull request Apr 19, 2025
Summary:

We now can link both ATen lib and portable lib into the same binary/.so file.

This makes us easy to use aten_mode lib as well as portable lib in the same bento kernel.

Reviewed By: Gasoonjia

Differential Revision: D73282334
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D73282334

Summary:

We now can link both ATen lib and portable lib into the same binary/.so file.

This makes us easy to use aten_mode lib as well as portable lib in the same bento kernel.

Reviewed By: Gasoonjia

Differential Revision: D73282334
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D73282334

@facebook-github-bot facebook-github-bot merged commit 06f912d into main Apr 20, 2025
79 of 86 checks passed
@facebook-github-bot facebook-github-bot deleted the export-D73282334 branch April 20, 2025 03:22
keyprocedure pushed a commit to keyprocedure/executorch that referenced this pull request Apr 21, 2025
Differential Revision: D73282334

Pull Request resolved: pytorch#10307
zhenyan-zhang-meta added a commit that referenced this pull request Apr 30, 2025
…-Aten

Separate `extension.Module` Namespaces to be `executorch::extension::module` and `executorch::extension::module_aten`, otherwise in the future there will be issues like P1799454769.

It's similar to what we already did for bundled_program in #10307

Differential Revision: [D73903870](https://our.internmc.facebook.com/intern/diff/D73903870/)

[ghstack-poisoned]
zhenyan-zhang-meta added a commit that referenced this pull request Apr 30, 2025
…ten and non-Aten"

Separate `extension.Module` Namespaces to be `executorch::extension::module` and `executorch::extension::module_aten`, otherwise in the future there will be issues like P1799454769.

It's similar to what we already did for bundled_program in #10307

Differential Revision: [D73903870](https://our.internmc.facebook.com/intern/diff/D73903870/)

[ghstack-poisoned]
zhenyan-zhang-meta added a commit that referenced this pull request Apr 30, 2025
…ten and non-Aten"


# Context

Separate `extension.Module` Namespaces to be `executorch::extension::module` and `executorch::extension::module::aten`, otherwise if a package relies on both aten and non-aten of the same implementation and the namespace is the same, there will be duplicate symbol issue like:

```
ld.lld: error: duplicate symbol: vtable for executorch::extension::Module
>>> defined at {redacted}/executorch/extension/module/__module__/__stripped__/module.cpp.pic.stripped.o:(vtable for executorch::extension::Module)
>>> defined at {redacted}/executorch/extension/module/__module_aten__/__stripped__/module.cpp.pic.stripped.o:
```

# Proposal

Doing something similar to what we already did for `bundled_program` in #10307

Since `extension.Module` is a public API, we introduce a namespace alias, so that existing use cases won't get affected. Since namespace alias doesn't create additional symbols, there won't be duplicate symbol issue.

Differential Revision: [D73903870](https://our.internmc.facebook.com/intern/diff/D73903870/)

[ghstack-poisoned]
zhenyan-zhang-meta added a commit that referenced this pull request May 5, 2025
…amespaces from Aten and non-Aten"


# Context

Separate `extension.Module` Namespaces to be `executorch::extension::module` and `executorch::extension::module::aten`, otherwise if a package relies on both aten and non-aten of the same implementation and the namespace is the same, there will be duplicate symbol issue like:

```
ld.lld: error: duplicate symbol: vtable for executorch::extension::Module
>>> defined at {redacted}/executorch/extension/module/__module__/__stripped__/module.cpp.pic.stripped.o:(vtable for executorch::extension::Module)
>>> defined at {redacted}/executorch/extension/module/__module_aten__/__stripped__/module.cpp.pic.stripped.o:
```

# Proposal

Doing something similar to what we already did for `bundled_program` in #10307

Since `extension.Module` is a public API, we introduce a namespace alias, so that existing use cases won't get affected. Since namespace alias doesn't create additional symbols, there won't be duplicate symbol issue.

Differential Revision: [D73903870](https://our.internmc.facebook.com/intern/diff/D73903870/)

[ghstack-poisoned]
zhenyan-zhang-meta added a commit that referenced this pull request May 5, 2025
…ten and non-Aten"


# Context

Separate `extension.Module` Namespaces to be `executorch::extension::module` and `executorch::extension::module::aten`, otherwise if a package relies on both aten and non-aten of the same implementation and the namespace is the same, there will be duplicate symbol issue like:

```
ld.lld: error: duplicate symbol: vtable for executorch::extension::Module
>>> defined at {redacted}/executorch/extension/module/__module__/__stripped__/module.cpp.pic.stripped.o:(vtable for executorch::extension::Module)
>>> defined at {redacted}/executorch/extension/module/__module_aten__/__stripped__/module.cpp.pic.stripped.o:
```

# Proposal

Doing something similar to what we already did for `bundled_program` in #10307

Since `extension.Module` is a public API, we introduce a namespace alias, so that existing use cases won't get affected. Since namespace alias doesn't create additional symbols, there won't be duplicate symbol issue.

Differential Revision: [D73903870](https://our.internmc.facebook.com/intern/diff/D73903870/)

[ghstack-poisoned]
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 topic: not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants