Refactor ArmBackend #6495

per · 2024-10-25T13:30:41Z

Split up ArmBackend into separate TOSA and Ethos-U backends.

Signed-off-by: Per Åstrand [email protected]

Change-Id: I0f03442a0e161c97699fbbbe7dab1731289c8ffa

cc @digantdesai @freddan80 @zingo @oscarandersson8218

pytorch-bot · 2024-10-25T13:30:45Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

❌ 2 New Failures

As of commit 33b90f1 with merge base 8ad15f3 ():

NEW FAILURES - The following jobs have failed:

trunk / test-llama-runner-mac (fp32, xnnpack+kv+custom) / macos-job (gh)
RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 1
trunk / test-pybind-build-macos (cmake) / macos-job (gh)
RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 1

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

robell · 2024-10-28T12:02:20Z

What's the view on removing the ArmBackend and having ArmEthosUBackend or ArmTOSABackend - we only really test the ArmPartitioner path right now so the fact we can no longer call a generic ArmBackend (as it has to be final) isn't a particular problem for our expected use. But I'd like to understand that from the ExecuTorch interface perspective. I suppose it's not a problem for Partitioners and Backends not to be 1:1?

digantdesai · 2024-10-28T16:27:32Z

backends/arm/arm_partitioner.py

 class ArmPartitioner(Partitioner):
    def __init__(self, compile_spec: List[CompileSpec]) -> None:
-        self.delegation_spec = DelegationSpec(ArmBackend.__name__, compile_spec)
+        self.delegation_spec = ArmBackendSelector.get_delegation_spec(compile_spec)


I like this, that said what is the rationale for keeping the partitioner class shared?

In my mind, quantizer, partitioner, and backend go as a "group". I.e. there is an inherent contract between these from what the target is capable of, from ET design point of view. But we never had a TOSA like IR before (at least not as open).

If sharing code is the primary motivation given TOSA IR, does it make sense to have different quantizer or partitioner as well as a wrapper, while keeping the code shared underneath? Purely from UX point of view. I.e. I just care about Ethos so I will do,

Ethos quantizer -> Ethos Parititoner -> Ethos backend -> .. -> Ethos runtime

That said I also like this UX where we have,

TOSA_complient_generic_quantizer -> TOSA_generic_artitioner_with_flags -> TOSA_complient_user_specified_backend -> .. -> Backend Runtime

We should try to avoid making quantizer aware of backend specific details by passing in some flags, because then it will be a clunky UX. If we find a need for that then we should just have wrappers.

I like this, that said what is the rationale for keeping the partitioner class shared?

The idea is to keep the ArmPartitioner as a generic entrypoint for TOSA backends. Both to share code and to separate the Ethos-U backend from the TOSA backend. But not using the TOSA name since that name maybe isn't super clear to users.

We should try to avoid making quantizer aware of backend specific details by passing in some flags, because then it will be a clunky UX. If we find a need for that then we should just have wrappers.

I do think the quantizer also needs to have some flags to know how to quantize according to the TOSA specification the hardware supports. What is it that you think get's clunky passing it in to a generic TOSA/Arm quantizer?
Not as part of this PR, so let's take that discussion of line.

From, UX side I think it is much cleaner if you have a different front ends for each TOSA compliant hardware backend i.e. using ethos and vulkan as proxy tosa backends for an example,

Option-A

q = VulkanQuantizer() ... em = to_backend(VulkanPartitioner(vulkan_flag=1)) ... vulkan_runtime.c --- different user --- q = EthosQuantizer() ... em = to_backend(EthosPartitioner(ethos_flag=1)) ... ethos_runtime.c

Option-B

q = ArmQuantizer(flag=Vulkan) ... em = to_backend(ArmPartitioner(flag=Vulkan, vulkan_flag=1)) ... vulkan_runtime.c --- different user --- q = ArmQuantizer(flag=Ethos) ... em = to_backend(ArmPartitioner(flag=Ethos, ethos_flag=1)) ... ethos_runtime.c

Purely from UX point of view I like Option-A, I see that as namespaces or inheritance of classes. TOSA is the backbone, which we want to hide from people (per your comment, which I agree) can be behind the scene as an implementation detail.

Also Option-A allows you to do more vulkan or ethos specific things without messing with the other i.e. if you want to add vela specific flags, it just a noise for Vulkan users.
Moreover, the overlap between the set of users is not much i.e. there is no reason for Ethos users to be "aware" of Vulkan specific things or even the existence of Vulkan for that matter.

On the flip side, if Quantizer and Partitioners are both targeting TOSA spec then you can argue that there might not be much specific to different TOSA backend. I agree to an extend, however there will always be things like hardware or runtime related creeps in.

backends/arm/arm_ethosu_backend.py

digantdesai

LGTM. I feel this is much cleaner, and scalable. Thank you @per.

Left some nit comments, but stamping to unblock you from moving forward.

backends/arm/arm_ethosu_backend.py

backends/arm/arm_partitioner.py

backends/arm/arm_tosa_backend.py

digantdesai · 2025-02-12T04:40:22Z

backends/arm/runtime/ArmEthosUBackend.cpp


 typedef struct {
  FreeableBuffer* processed;
 } ExecutionHandle;


In theory we can have a runtime wrapping tosa reference model too, which works on host/arm,x86? This is if we want an e2e set up for TOSA-only flow too.

Yes! And also the possibility to 'recompile' a TOSA backend .pte as long as you use the same TosaSpecification for the recompile.

digantdesai · 2025-02-12T04:46:34Z

backends/arm/quantizer/arm_quantizer.py

+
+
+class ArmEthosUQuantizer(ArmTOSAQuantizer):
+    def __init__(self) -> None:


This should take TosaSpecification?

I guess this can be somewhat of a sticking point, i.e partitioner gets the compile_spec to decide u55 vs. u85, now we need to tell quantizer too (if/when necessary). This is something we can document, and cross check for now through some metadata insertion in the graph.

It should!
What about rework this a bit to have the compile spec as the common argument to both partitioner and quantizer (for EthosU)?
The compile spec builder probably should also be split up to respective backends, but I'll hold of on that.

examples/arm/aot_arm_compiler.py

digantdesai

LGTM. I feel this is much cleaner, and scalable. Thank you @per.

Left some nit comments, but stamping to unblock you from moving forward.

digantdesai · 2025-02-12T04:50:02Z

cc @mergennachin - FYI

Split up ArmBackend into separate TOSA and Ethos-U classes. Move partitioner and backend to separate files and remove Arm prefix. Add compile_spec argument to EthosUQuantizer Signed-off-by: Per Åstrand <[email protected]> Change-Id: I0f03442a0e161c97699fbbbe7dab1731289c8ffa

per · 2025-02-13T13:24:50Z

Failing on MacOS with HTTP error 520 from https://conda.anaconda.org/.

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 Oct 25, 2024

per added partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm ciflow/trunk labels Oct 25, 2024

per force-pushed the arm_backend_refactoring branch from 80c50b0 to bc061ed Compare October 28, 2024 07:24

robell requested a review from digantdesai October 28, 2024 11:57

per force-pushed the arm_backend_refactoring branch from bc061ed to 4c71b47 Compare October 29, 2024 07:50

digantdesai reviewed Oct 29, 2024

View reviewed changes

per marked this pull request as draft November 11, 2024 15:03

per force-pushed the arm_backend_refactoring branch from 4c71b47 to e6a7d39 Compare February 7, 2025 10:13

per added the release notes: arm Changes to the ARM backend delegate label Feb 7, 2025

per requested a review from digantdesai February 7, 2025 10:27

per marked this pull request as ready for review February 7, 2025 10:27

per force-pushed the arm_backend_refactoring branch 3 times, most recently from 952253d to 4ebd08a Compare February 11, 2025 21:14

digantdesai approved these changes Feb 12, 2025

View reviewed changes

per force-pushed the arm_backend_refactoring branch from 4ebd08a to e95cba4 Compare February 13, 2025 13:06

per force-pushed the arm_backend_refactoring branch from e95cba4 to 33b90f1 Compare February 13, 2025 13:11

per requested a review from zingo February 13, 2025 13:25

zingo merged commit f8c345b into pytorch:main Feb 13, 2025
107 of 109 checks passed

per deleted the arm_backend_refactoring branch February 17, 2025 09:07



		class ArmEthosUQuantizer(ArmTOSAQuantizer):
		def __init__(self) -> None:

Refactor ArmBackend #6495

Refactor ArmBackend #6495

Uh oh!

Conversation

per commented Oct 25, 2024 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

❌ 2 New Failures

Uh oh!

robell commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

digantdesai Oct 28, 2024

Choose a reason for hiding this comment

Uh oh!

per Oct 30, 2024

Choose a reason for hiding this comment

Uh oh!

digantdesai Nov 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

digantdesai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

digantdesai Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

per Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

digantdesai Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

per Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

digantdesai left a comment

Choose a reason for hiding this comment

Uh oh!

digantdesai commented Feb 12, 2025

Uh oh!

per commented Feb 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

per commented Oct 25, 2024 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Oct 25, 2024 •

edited

Loading

robell commented Oct 28, 2024 •

edited

Loading

per Feb 12, 2025 •

edited

Loading