Skip to content

Arm Backend: Add New DecomposeSilu pass to arm_pass_manager #9448

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 2 commits into from
Apr 15, 2025

Conversation

ArmRyan
Copy link
Collaborator

@ArmRyan ArmRyan commented Mar 20, 2025

  • Adds DecomposeSilu pass
  • Adds Tests for DecomposeSilu

Change-Id: Ib9f15d04c4c06d92d38cc9e6297145980052e673

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218

@ArmRyan ArmRyan requested a review from digantdesai as a code owner March 20, 2025 10:34
Copy link

pytorch-bot bot commented Mar 20, 2025

🔗 Helpful Links

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

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

❌ 2 New Failures

As of commit 684b4b3 with merge base d64c410 (image):

NEW FAILURES - The following jobs have failed:

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 Mar 20, 2025
@AdrianLundell AdrianLundell added partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm ciflow/trunk labels Mar 20, 2025
Copy link

pytorch-bot bot commented Mar 20, 2025

To add the ciflow label ciflow/trunk please first approve the workflows that are awaiting approval (scroll to the bottom of this page).

This helps ensure we don't trigger CI on this PR until it is actually authorized to do so. Please ping one of the reviewers if you do not have access to approve and run workflows.

Copy link

pytorch-bot bot commented Mar 20, 2025

To add the ciflow label ciflow/trunk please first approve the workflows that are awaiting approval (scroll to the bottom of this page).

This helps ensure we don't trigger CI on this PR until it is actually authorized to do so. Please ping one of the reviewers if you do not have access to approve and run workflows.

@digantdesai
Copy link
Contributor

Updated permissions, so you should be able to run ci and merge.

@@ -0,0 +1,149 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
Copy link
Contributor

Choose a reason for hiding this comment

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

no tests for SDPA?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There were no tests added because it was not a new function essentially, just a new call with the idea is that it is tested elsewhere. We have tested it functionally just not with a unit test

Copy link
Contributor

Choose a reason for hiding this comment

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

adding test_sdpa.py would ensure we can lower and run it. Where else it is tested? Couldn't find it in the test/ops dir.

Copy link
Collaborator Author

@ArmRyan ArmRyan Mar 21, 2025

Choose a reason for hiding this comment

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

It is tested from the QNN delegate but I can add unit tests here also! I will push that when I can ( probably Monday )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry for the delay, had some issues and went on holidays, came back to more issues so I have removed SDPA for now

Copy link
Contributor

@digantdesai digantdesai left a comment

Choose a reason for hiding this comment

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

Silu part LGTM. Add SDPA tests?

 * Adds DecomposeSilu pass
 * Adds Tests for DecomposeSilu

Signed-off-by: Ryan O'Shea <[email protected]>
Change-Id: Ib9f15d04c4c06d92d38cc9e6297145980052e673
@ArmRyan ArmRyan force-pushed the experimental/arm_silu_sdpa_passes branch from 23c11c7 to 54d7dee Compare April 15, 2025 09:52
@ArmRyan ArmRyan changed the title Arm Backend: Add New Silu and SDPA Decomp passes to arm_pass_manager Arm Backend: Add New DecomposeSilu pass to arm_pass_manager Apr 15, 2025
@ArmRyan ArmRyan requested review from digantdesai and mansnils April 15, 2025 09:56
@zingo
Copy link
Collaborator

zingo commented Apr 15, 2025

Huggingface fails seems unrelated

@zingo zingo closed this Apr 15, 2025
@zingo zingo reopened this Apr 15, 2025
@zingo zingo merged commit fc01661 into pytorch:main Apr 15, 2025
320 of 328 checks passed
keyprocedure pushed a commit to keyprocedure/executorch that referenced this pull request Apr 21, 2025
…9448)

* Adds DecomposeSilu pass
* Adds Tests for DecomposeSilu

Signed-off-by: Ryan O'Shea <[email protected]>
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. partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm topic: not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants