Skip to content
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

Add executorch_no_prim_ops target #2934

Closed
wants to merge 1 commit into from
Closed

Conversation

larryliu0820
Copy link
Contributor

Summary:
Currently libexecutorch.a always contain prim ops. This becomes a problem when a binary contains 2 "versions" of libexecutorch.a, causing a double registration of the prim ops.

For example, libA.so depends on libexecutorch.a and a binary B depends on both libA.so and libexecutorch.a. Since both libexecutorch.a and libA.so contains prim ops, they will be registered twice.

In this PR I created another library executorch_no_prim_ops for libA.so to depend on.

Differential Revision: D55907752

Copy link

pytorch-bot bot commented Apr 9, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit ec33d2d with merge base 9fd1a0e (image):
💚 Looks good so far! There are no failures yet. 💚

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 9, 2024
@facebook-github-bot
Copy link
Contributor

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

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

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

Differential Revision: D55907752
larryliu0820 added a commit that referenced this pull request Apr 9, 2024
Summary:

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

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

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

larryliu0820 added a commit that referenced this pull request Apr 9, 2024
Summary:

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

Differential Revision: D55907752
larryliu0820 added a commit that referenced this pull request Apr 9, 2024
Summary:

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

Differential Revision: D55907752
facebook-github-bot pushed a commit that referenced this pull request Apr 9, 2024
Summary:

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

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

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

Summary:

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in d309e9d.

facebook-github-bot pushed a commit that referenced this pull request Apr 10, 2024
Summary:
Fix these 3 CI job failures caused by #2934 (D55907752):

* Apple / build-frameworks-ios / macos-job
* trunk / test-arm-backend-delegation / linux-job
* trunk / test-coreml-delegate / macos-job

Differential Revision: D55950023
facebook-github-bot pushed a commit that referenced this pull request Apr 10, 2024
Summary:

Fix these 3 CI job failures caused by #2934 (D55907752):

* Apple / build-frameworks-ios / macos-job
* trunk / test-arm-backend-delegation / linux-job
* trunk / test-coreml-delegate / macos-job

Reviewed By: kirklandsign

Differential Revision: D55950023
larryliu0820 added a commit that referenced this pull request Apr 10, 2024
Summary:

Fix these 3 CI job failures caused by #2934 (D55907752):

* Apple / build-frameworks-ios / macos-job
* trunk / test-arm-backend-delegation / linux-job
* trunk / test-coreml-delegate / macos-job

Reviewed By: kirklandsign

Differential Revision: D55950023
facebook-github-bot pushed a commit that referenced this pull request Apr 10, 2024
Summary:
Pull Request resolved: #2961

Fix these 3 CI job failures caused by #2934 (D55907752):

* Apple / build-frameworks-ios / macos-job
* trunk / test-arm-backend-delegation / linux-job
* trunk / test-coreml-delegate / macos-job

Reviewed By: kirklandsign

Differential Revision: D55950023

fbshipit-source-id: 6166d9112e6d971d042df1400442395d8044c3b3
@kirklandsign
Copy link
Contributor

@pytorchbot cherry-pick --onto release/0.2

Copy link

pytorch-bot bot commented Apr 10, 2024

❌ 🤖 pytorchbot command failed:

@pytorchbot cherry-pick: error: the following arguments are required: -c/--classification

usage: @pytorchbot cherry-pick --onto ONTO [--fixes FIXES] -c
                               {regression,critical,fixnewfeature,docs,release}

Try @pytorchbot --help for more info.

@kirklandsign
Copy link
Contributor

@pytorchbot cherry-pick --onto release/0.2 critical

[1/4] Required to run QNN delegate on Android demo app

Copy link

pytorch-bot bot commented Apr 10, 2024

❌ 🤖 pytorchbot command failed:

@pytorchbot cherry-pick: error: the following arguments are required: -c/--classification

usage: @pytorchbot cherry-pick --onto ONTO [--fixes FIXES] -c
                               {regression,critical,fixnewfeature,docs,release}

Try @pytorchbot --help for more info.

@kirklandsign
Copy link
Contributor

@pytorchbot cherry-pick --onto release/0.2 -c critical

[1/4] Required to run QNN delegate on Android demo app

pytorchbot pushed a commit that referenced this pull request Apr 10, 2024
Summary:
Pull Request resolved: #2934

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

Reviewed By: cccclai, kirklandsign

Differential Revision: D55907752

fbshipit-source-id: 755a9b8d5f6f7cf44d011b83bfdc18be6da1aa05
(cherry picked from commit d309e9d)
@pytorchbot
Copy link
Collaborator

Cherry picking #2934

The cherry pick PR is at #2970 and it is recommended to link a critical cherry pick PR with an issue

Details for Dev Infra team Raised by workflow job

kirklandsign pushed a commit to kirklandsign/executorch that referenced this pull request Apr 11, 2024
Summary:
Pull Request resolved: pytorch#2934

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

Reviewed By: cccclai, kirklandsign

Differential Revision: D55907752

fbshipit-source-id: 755a9b8d5f6f7cf44d011b83bfdc18be6da1aa05
(cherry picked from commit d309e9d)
kirklandsign pushed a commit to kirklandsign/executorch that referenced this pull request Apr 11, 2024
Summary:
Pull Request resolved: pytorch#2961

Fix these 3 CI job failures caused by pytorch#2934 (D55907752):

* Apple / build-frameworks-ios / macos-job
* trunk / test-arm-backend-delegation / linux-job
* trunk / test-coreml-delegate / macos-job

Reviewed By: kirklandsign

Differential Revision: D55950023

fbshipit-source-id: 6166d9112e6d971d042df1400442395d8044c3b3
(cherry picked from commit d993797)
larryliu0820 added a commit that referenced this pull request Apr 11, 2024
As titled. `build_apple_frameworks.sh` is copying all the exported headers out and in #2934 `//executorch/schema:program` is being moved to `exported_deps` and causing `build_apple_frameworks.sh` to not found it.

This PR fixes it by moving it back to `deps`.
facebook-github-bot pushed a commit that referenced this pull request Apr 11, 2024
Summary:
As titled. `build_apple_frameworks.sh` is copying all the exported headers out and in #2934 `//executorch/schema:program` is being moved to `exported_deps` and causing `build_apple_frameworks.sh` to not able to copy generated headers `program_generated.h` and `scalar_type_generated.h`.

This PR fixes it by moving it back to `deps`.

Pull Request resolved: #2996

Reviewed By: kirklandsign

Differential Revision: D56028952

Pulled By: larryliu0820

fbshipit-source-id: 2cd4999154877b0ac7b49cd1f54d518cba34b2f2
kirklandsign pushed a commit to kirklandsign/executorch that referenced this pull request Apr 12, 2024
Summary:
Pull Request resolved: pytorch#2934

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

Reviewed By: cccclai, kirklandsign

Differential Revision: D55907752

fbshipit-source-id: 755a9b8d5f6f7cf44d011b83bfdc18be6da1aa05
(cherry picked from commit d309e9d)
kirklandsign pushed a commit to kirklandsign/executorch that referenced this pull request Apr 12, 2024
Summary:
Pull Request resolved: pytorch#2961

Fix these 3 CI job failures caused by pytorch#2934 (D55907752):

* Apple / build-frameworks-ios / macos-job
* trunk / test-arm-backend-delegation / linux-job
* trunk / test-coreml-delegate / macos-job

Reviewed By: kirklandsign

Differential Revision: D55950023

fbshipit-source-id: 6166d9112e6d971d042df1400442395d8044c3b3
(cherry picked from commit d993797)
kirklandsign pushed a commit to kirklandsign/executorch that referenced this pull request Apr 12, 2024
Summary:
Pull Request resolved: pytorch#2934

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

Reviewed By: cccclai, kirklandsign

Differential Revision: D55907752

fbshipit-source-id: 755a9b8d5f6f7cf44d011b83bfdc18be6da1aa05
(cherry picked from commit d309e9d)
kirklandsign pushed a commit to kirklandsign/executorch that referenced this pull request Apr 12, 2024
Summary:
Pull Request resolved: pytorch#2961

Fix these 3 CI job failures caused by pytorch#2934 (D55907752):

* Apple / build-frameworks-ios / macos-job
* trunk / test-arm-backend-delegation / linux-job
* trunk / test-coreml-delegate / macos-job

Reviewed By: kirklandsign

Differential Revision: D55950023

fbshipit-source-id: 6166d9112e6d971d042df1400442395d8044c3b3
(cherry picked from commit d993797)
guangy10 pushed a commit that referenced this pull request Apr 17, 2024
* Add executorch_no_prim_ops target (#2934)

Summary:
Pull Request resolved: #2934

Currently `libexecutorch.a` always contain prim ops. This becomes a problem when a binary contains 2 "versions" of `libexecutorch.a`, causing a double registration of the prim ops.

For example, `libA.so` depends on `libexecutorch.a` and a binary `B` depends on both `libA.so` and `libexecutorch.a`. Since both `libexecutorch.a` and `libA.so` contains prim ops, they will be registered twice.

In this PR I created another library `executorch_no_prim_ops` for `libA.so` to depend on.

Reviewed By: cccclai, kirklandsign

Differential Revision: D55907752

fbshipit-source-id: 755a9b8d5f6f7cf44d011b83bfdc18be6da1aa05
(cherry picked from commit d309e9d)

* Fix failing CI jobs caused by #2934 (#2961)

Summary:
Pull Request resolved: #2961

Fix these 3 CI job failures caused by #2934 (D55907752):

* Apple / build-frameworks-ios / macos-job
* trunk / test-arm-backend-delegation / linux-job
* trunk / test-coreml-delegate / macos-job

Reviewed By: kirklandsign

Differential Revision: D55950023

fbshipit-source-id: 6166d9112e6d971d042df1400442395d8044c3b3
(cherry picked from commit d993797)

* [NOT-CLEAN-CP] Fix 3 CI jobs (#3006)

Summary:
* [NOT APPLICABLE IN RELEASE] Apple / build-frameworks-ios / macos-job

We removed libcustom_ops_lib.a in #2916 so need to remove it from `build_apple_frameworks.sh`.

* [NOT APPLICABLE IN RELEASE] Lint / lintrunner / linux-job

Remove extra line in backends/qualcomm/quantizer/utils.py

* pull / unittest / macos (buck2) / macos-job

Fix it by using `executorch_no_prim_ops` instead of `executorch` in MPS and CoreML.

Pull Request resolved: #3006

Reviewed By: lucylq

Differential Revision: D56048430

Pulled By: larryliu0820

fbshipit-source-id: 9dcb476eea446ea3aba566d595167c691fb00eec
(cherry picked from commit 5b7c4ba)

---------

Co-authored-by: Mengwei Liu <larryliu@meta.com>
Co-authored-by: Mengwei Liu <larryliu@fb.com>
pytorchbot pushed a commit that referenced this pull request Apr 20, 2024
Summary:
As titled. `build_apple_frameworks.sh` is copying all the exported headers out and in #2934 `//executorch/schema:program` is being moved to `exported_deps` and causing `build_apple_frameworks.sh` to not able to copy generated headers `program_generated.h` and `scalar_type_generated.h`.

This PR fixes it by moving it back to `deps`.

Pull Request resolved: #2996

Reviewed By: kirklandsign

Differential Revision: D56028952

Pulled By: larryliu0820

fbshipit-source-id: 2cd4999154877b0ac7b49cd1f54d518cba34b2f2
(cherry picked from commit 3b727a7)
larryliu0820 added a commit that referenced this pull request Apr 20, 2024
Summary:
As titled. `build_apple_frameworks.sh` is copying all the exported headers out and in #2934 `//executorch/schema:program` is being moved to `exported_deps` and causing `build_apple_frameworks.sh` to not able to copy generated headers `program_generated.h` and `scalar_type_generated.h`.

This PR fixes it by moving it back to `deps`.

Pull Request resolved: #2996

Reviewed By: kirklandsign

Differential Revision: D56028952

Pulled By: larryliu0820

fbshipit-source-id: 2cd4999154877b0ac7b49cd1f54d518cba34b2f2
(cherry picked from commit 3b727a7)
huydhn pushed a commit that referenced this pull request Apr 21, 2024
Summary:
As titled. `build_apple_frameworks.sh` is copying all the exported headers out and in #2934 `//executorch/schema:program` is being moved to `exported_deps` and causing `build_apple_frameworks.sh` to not able to copy generated headers `program_generated.h` and `scalar_type_generated.h`.

This PR fixes it by moving it back to `deps`.

Pull Request resolved: #2996

Reviewed By: kirklandsign

Differential Revision: D56028952

Pulled By: larryliu0820

fbshipit-source-id: 2cd4999154877b0ac7b49cd1f54d518cba34b2f2
guangy10 pushed a commit that referenced this pull request Apr 22, 2024
Summary:
As titled. `build_apple_frameworks.sh` is copying all the exported headers out and in #2934 `//executorch/schema:program` is being moved to `exported_deps` and causing `build_apple_frameworks.sh` to not able to copy generated headers `program_generated.h` and `scalar_type_generated.h`.

This PR fixes it by moving it back to `deps`.

Pull Request resolved: #2996

Reviewed By: kirklandsign

Differential Revision: D56028952

Pulled By: larryliu0820

fbshipit-source-id: 2cd4999154877b0ac7b49cd1f54d518cba34b2f2
(cherry picked from commit 3b727a7)
huydhn added a commit that referenced this pull request Apr 22, 2024
* Fix build-framework-ios CI job (#2996)

Summary:
As titled. `build_apple_frameworks.sh` is copying all the exported headers out and in #2934 `//executorch/schema:program` is being moved to `exported_deps` and causing `build_apple_frameworks.sh` to not able to copy generated headers `program_generated.h` and `scalar_type_generated.h`.

This PR fixes it by moving it back to `deps`.

Pull Request resolved: #2996

Reviewed By: kirklandsign

Differential Revision: D56028952

Pulled By: larryliu0820

fbshipit-source-id: 2cd4999154877b0ac7b49cd1f54d518cba34b2f2

* Fix 3 CI jobs (#3006)

Summary:
* Apple / build-frameworks-ios / macos-job

We removed libcustom_ops_lib.a in #2916 so need to remove it from `build_apple_frameworks.sh`.

* Lint / lintrunner / linux-job

Remove extra line in backends/qualcomm/quantizer/utils.py

* pull / unittest / macos (buck2) / macos-job

Fix it by using `executorch_no_prim_ops` instead of `executorch` in MPS and CoreML.

Pull Request resolved: #3006

Reviewed By: lucylq

Differential Revision: D56048430

Pulled By: larryliu0820

fbshipit-source-id: 9dcb476eea446ea3aba566d595167c691fb00eec

* Revert "Fix build-framework-ios CI job (#2996)"

This reverts commit e365c5b.

---------

Co-authored-by: Mengwei Liu <larryliu@fb.com>
@mergennachin mergennachin mentioned this pull request Apr 26, 2024
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 Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants