[3/6] Arm(R) Ethos(TM)-U NPU TIR compiler with conv2d support #8806

mbaret · 2021-08-20T15:00:03Z

This commit adds the lowering passes necessary to lower an NPU Relay module down to a TIR module that can be compiled for the NPU. Conv2d is supported as the first NPU operator. An intermediate TE stage between Relay and TIR allows support for scheduling the operators.

This PR implements P3 on our tracking issue for initial Ethos-U support in TVM: #8482. Our RFC can be found here: apache/tvm-rfcs#11.

jroesch · 2021-08-20T22:41:32Z

Thanks for the contribution guys!

I know how painful of a request this is, but is there any way you guys could logically split this up into smaller sets of changes? I know I am one of the worst offenders of very large PRs but an 8k PR is very hard/nearly impossible to review well in one shot.

My hope is for us to build wider understanding of these big changes so we can spread the future review and maintenance burden across many people. For example @areusch has the most context but is on vacation for the next couple of weeks.

mbaret · 2021-08-20T22:57:53Z

I appreciate this doesn't exactly trivialize the patch, but it's actually 'only' 4k lines because this is on top of #8795. I've got this as draft until that PR gets merged to give visibility, but I'm not expecting this to necessarily get reviewed until the dependent PR gets merged.

The particular problem we have splitting this PR up is that we need to introduce an operator alongside some of the compiler support so that we can test it. That's not to say it's impossible though, and if you still think 4k is too much I'm happy to explore some options to reduce it further.

jroesch · 2021-08-20T23:44:31Z

Sounds good let's focus on #8795 first and then we can revisit this one afterwards!

This commit adds the lowering passes necessary to lower an NPU Relay module down to a TIR module that can be compiled for the NPU. Conv2d is supported as the first NPU operator. An intermediate TE stage between Relay and TIR allows support for scheduling the operators. Co-authored-by: Manupa Karunaratne <Manupa.Karunaratne@arm.com>

Change-Id: I3741f9dd8bb5952590ff8c586f6b96e5c3a03795

*fixing tests Change-Id: Id4a4c80f72ce29b98fc8b3954a1413c1c7fda500

Change-Id: Iaee06017bd125d3040ce42182c4ccdb80d7fc946

Change-Id: I81513f112a42b93cfdd3bcaf8e8852dd60ffe9e9

Change-Id: I6596b62ab56e4ca8b31ef08293686f53f38454d2

Change-Id: I0aaf83fe0204c0db435692e9b92dee6e9d6997fe

manupak · 2021-09-13T17:32:53Z

a friendly ping @mbs-octoml @jroesch , this is ready for review now.

mbs-octoml · 2021-09-13T19:48:04Z

I've left 4 comments above.

jroesch

Great looks good to me, left a bunch of really minor doc nits, I am happy if you guys want to do them in a follow up pass. Feel free to click the merge button.

jroesch · 2021-09-13T20:56:03Z

python/tvm/relay/backend/contrib/ethosu/tir/compiler.py

+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name, unused-argument
+"""The integration of Arm(R) Ethos(TM)-U NPU TIR compiler"""


For all the below nits feel free to do in secondary PR, happy to land this first.

Nit: full sentence.

jroesch · 2021-09-13T20:56:52Z

python/tvm/relay/backend/contrib/ethosu/tir/compiler.py

+    """Lower a schedule to TIR for the Arm(R) Ethos(TM)-U NPU target.
+
+    The resulting TIR module will contain a single function
+    that comprises of a sequence of tir.extern_calls to NPU


Suggested change

that comprises of a sequence of tir.extern_calls to NPU

that consists of a sequence of tir.extern_calls to NPU

jroesch · 2021-09-13T20:57:53Z

python/tvm/relay/backend/contrib/ethosu/tir/convolution.py

+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name, unused-argument
+"""Extract information from the convolution operators in TIR."""


Explain more "information" is vague.

jroesch · 2021-09-13T20:58:18Z

python/tvm/relay/backend/contrib/ethosu/tir/dma.py

+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name, unused-argument
+"""Extract information from the DMA operators in TIR."""


Nit: same comment as above.

jroesch · 2021-09-13T20:59:03Z

python/tvm/relay/backend/contrib/ethosu/tir/scheduler.py

+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name, unused-argument
+"""Different schedulers for Arm(R) Ethos(TM)-U NPU"""


Nit: same comment as above? "Different" wrt to what exactly?

jroesch · 2021-09-13T21:06:39Z

python/tvm/relay/backend/contrib/ethosu/tir_to_cs_translator.py

+    mode,
+):
+    """This is a helper function to capture a list
+    of arguments to create Vela NpuResamplingMode object"""


Nit: punctuation in this file as well.

jroesch · 2021-09-13T21:07:23Z

python/tvm/relay/backend/contrib/ethosu/util.py

@@ -138,6 +139,12 @@ def round_up(a: int, b: int) -> int:
    return ((a + b - 1) // b) * b


+def get_accelerator_config():
+    """Get the variant of the accelerator to compile for"""


Could use some more clarification here as well.

this makes sense when you know there are multiple sizes of NPU but curious what the returned data type is and where i might go to read more about the things that could be present here.

jroesch · 2021-09-13T21:09:33Z

src/relay/backend/contrib/ethosu/to_te_graph.cc

@@ -0,0 +1,234 @@
+/*


Is it possible for us to combine this with the current code we factored out as part of the te_compiler.cc refactor? Ok to do in follow up.

jroesch · 2021-09-13T21:11:49Z

tests/python/contrib/test_ethosu/test_extract_constants.py

+# under the License.
+import pytest
+
+pytest.importorskip("ethosu.vela")


@Lunderberg is this the right way to do this?

This is the only dependency to run the test in this test source. Therefore, any platform that have dependency satisfied should be able to run the tests.

Yup, this works, and allows the test suite to run regardless of the platform. The main issue I've seen with pytest.importorskip is that the tests in the file are entirely removed from the test report, rather than showing up as explicitly being skipped.

To have the tests show up in the test report, an individual test can be marked with tvm.testing.requires_package('ethosu.vela'), and the from tvm.relay.backend.contrib.ethosu import util statement moved into the body of the test function. However, this isn't the cleanest solution and I have some ideas on how to improve the ergonomics of skipping tests this way, so pytest.importorskip is a good use in the meantime.

jroesch · 2021-09-13T21:17:48Z

python/tvm/relay/backend/contrib/ethosu/tir/passes.py

+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name, unused-argument
+"""The TIR passes to be run on Arm(R) Ethos(TM)-U NPU TIR Compiler"""


Nit: punctuation.

mbaret · 2021-09-14T09:14:39Z

I've left 4 comments above.

@mbs-octoml Not sure if this is a GitHub bug on my end, but I can't see your comments. Could you check that you've submitted the review?

mbs-octoml · 2021-08-26T16:56:37Z

src/relay/backend/contrib/ethosu/to_te_graph.cc

+
+/*!
+ * \file relay/backend/contrib/ethosu/to_te_graph.cc
+ * \brief Lower a Relay function to a TE graph.


Can I check this file (and it's dependence on lower_call in compile_engine.py) is temporary scaffolding pending a suitable hook mechanism?

Looks like this PR has overtaken the relay_to_tir hook mechanism train.

Let's make abundantly clear in the comment this is a copy of relay::tec::ScheduleBuilder for the purpose of intercepting the scheduling and this is a temporary workaround. It would be a shame if a new contributor assumed this is the pattern for controlling scheduling.

And let's file a GitHub issue or point to the existing one for the rely_to_tir hook mechanism.

mbs-octoml · 2021-09-10T14:09:02Z

src/relay/backend/contrib/ethosu/to_te_graph.cc

+    auto graph_node = make_object<TEGraphNode>();
+    for (Var param : prim_func->params) {
+      Array<tvm::te::Tensor> inputs;
+      if (const auto* ttype = param->checked_type().as<TensorTypeNode>()) {


You can first relay::FlattenTupleType then just itr over the resut.

mbs-octoml · 2021-09-10T14:45:21Z

src/relay/backend/contrib/ethosu/to_te_graph.cc

+  }
+
+  Array<te::Tensor> VisitExpr_(const CallNode* call_node) final {
+    static auto flower_call = tvm::runtime::Registry::Get("relay.backend.lower_call");


Please comment that this API is pending removal in #8775.

mbs-octoml · 2021-09-10T14:54:16Z

python/tvm/relay/backend/contrib/ethosu/tir/scheduler.py

+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name, unused-argument
+"""Different schedulers for Arm(R) Ethos(TM)-U NPU"""


I'm starting to see the forest for the trees but a few paragraphs explaining the scheduling strategy would really help (along with a pointer to the RFC). The main points I think are:

make the dma with its layout xforms explicit (which in an ideal world could be done without having to replicate the inner conv2d TE defns?)

hoist constants

prepare for cascading
Is that about right?

I'm not sure what we should say about how this plays with the new 'schedulable' TIR. We are prepared to port at some point? TVM overall is on the hook for preserving the 'legacy' scheduling API? Perhaps a GitHub issue is in order.

mbs-octoml · 2021-09-14T16:29:02Z

D'oh - forgot to hit submit.

areusch · 2021-09-14T20:29:14Z

python/tvm/relay/backend/contrib/ethosu/tir_to_cs_translator.py

+import ethosu.vela.api as vapi  # type: ignore
+
+import tvm
+from tvm.relay.backend.contrib.ethosu import vela_api


it might be nicer to call this one vela_api_utils and the true VELA api vela_api

areusch · 2021-09-14T20:38:44Z

python/tvm/relay/backend/contrib/ethosu/vela_api.py

@@ -308,3 +356,17 @@ def _calculate_hw_bias_scales(
        hw_bias_scales = [_quantize_scale(bs) for bs in bias_scales]

    return hw_bias_scales
+
+
+def get_target_accel_type():


nit: i think earlier this was called variant--good to align terminology

manupak · 2021-09-15T08:33:54Z

Hi All,

In order to unblock reviews for the rest of the PRs (4/6 --> 6/6), we are committing to address the comments in P3a as mentioned here : #8482.

Having said that, can we get this in ? @mbaret

leandron · 2021-09-15T11:12:29Z

Thanks @manupa-arm @mbaret @areusch @jroesch @mbs-octoml, this is merged now.

…#8806) * Arm(R) Ethos(TM)-U NPU TIR compiler with conv2d support This commit adds the lowering passes necessary to lower an NPU Relay module down to a TIR module that can be compiled for the NPU. Conv2d is supported as the first NPU operator. An intermediate TE stage between Relay and TIR allows support for scheduling the operators. Co-authored-by: Manupa Karunaratne <Manupa.Karunaratne@arm.com> * Fix Conv2D TIR type sensitivity Change-Id: I3741f9dd8bb5952590ff8c586f6b96e5c3a03795 * Arm(R) Ethos(TM)-U NPU TIR passes and TE for Conv2D *fixing tests Change-Id: Id4a4c80f72ce29b98fc8b3954a1413c1c7fda500 * Fix import guards for tests Change-Id: Iaee06017bd125d3040ce42182c4ccdb80d7fc946 * Fix typing failures with ignores Change-Id: I81513f112a42b93cfdd3bcaf8e8852dd60ffe9e9 * Remove unused import Change-Id: I6596b62ab56e4ca8b31ef08293686f53f38454d2 * Reintroduce get_target_accel_type Change-Id: I0aaf83fe0204c0db435692e9b92dee6e9d6997fe Co-authored-by: Manupa Karunaratne <Manupa.Karunaratne@arm.com>

manupak mentioned this pull request Aug 20, 2021

[RFC][Tracking Issue] Arm® Ethos™-U Integration #8482

Closed

8 tasks

mbaret changed the title ~~Arm(R) Ethos(TM)-U NPU TIR passes and TE for Conv2D~~ [3/6] Arm(R) Ethos(TM)-U NPU TIR passes and TE for Conv2D Aug 20, 2021

jroesch added the status: WIP label Aug 20, 2021

jroesch self-assigned this Aug 20, 2021

manupak mentioned this pull request Aug 23, 2021

[4/6] Arm(R) Ethos(TM)-U NPU TIR to CS for Conv2D #8811

Merged

jroesch added the status: need review label Aug 24, 2021

mbaret force-pushed the ethosu_upstreaming_p3 branch from e1a3b39 to b612d71 Compare August 25, 2021 09:13

manupak force-pushed the ethosu_upstreaming_p3 branch from b612d71 to 03c137c Compare August 25, 2021 11:31

manupak mentioned this pull request Aug 25, 2021

[5/6] Arm(R) Ethos(TM)-U NPU codegen integration #8849

Merged

leandron mentioned this pull request Aug 26, 2021

[6/6] Arm(R) Ethos(TM)-U NPU codegen integration with tvmc #8854

Merged

mbaret force-pushed the ethosu_upstreaming_p3 branch from 03c137c to 5e848b6 Compare August 26, 2021 09:36

manupak force-pushed the ethosu_upstreaming_p3 branch from 5e848b6 to f62c96e Compare August 26, 2021 16:34

grant-arm mentioned this pull request Sep 3, 2021

Arm(R) Cortex(R)-M55 CPU and Arm(R) Ethos(TM)-U55 NPU Demo App #8922

Merged

mbaret force-pushed the ethosu_upstreaming_p3 branch from f62c96e to c2895f6 Compare September 9, 2021 09:01

mbaret changed the title ~~[3/6] Arm(R) Ethos(TM)-U NPU TIR passes and TE for Conv2D~~ [3/6] Arm(R) Ethos(TM)-U NPU TIR compiler with conv2d support Sep 9, 2021

mbaret marked this pull request as ready for review September 9, 2021 09:23

mbaret requested review from anijain2305, areusch, comaniac, jroesch, junrushao, jwfromm, manupak, MarisaKirisame, mbrookhart, merrymercy and slyubomirsky as code owners September 9, 2021 09:23

mbaret requested review from yzhliu, zhiics and ZihengJiang as code owners September 9, 2021 09:23

mbaret force-pushed the ethosu_upstreaming_p3 branch from c2895f6 to 9923074 Compare September 9, 2021 15:30

mbaret and others added 7 commits September 13, 2021 09:25

Fix Conv2D TIR type sensitivity

09046df

Change-Id: I3741f9dd8bb5952590ff8c586f6b96e5c3a03795

Arm(R) Ethos(TM)-U NPU TIR passes and TE for Conv2D

dea5853

*fixing tests Change-Id: Id4a4c80f72ce29b98fc8b3954a1413c1c7fda500

Fix import guards for tests

36bde1e

Change-Id: Iaee06017bd125d3040ce42182c4ccdb80d7fc946

Fix typing failures with ignores

df9ec83

Change-Id: I81513f112a42b93cfdd3bcaf8e8852dd60ffe9e9

Remove unused import

2f165fc

Change-Id: I6596b62ab56e4ca8b31ef08293686f53f38454d2

Reintroduce get_target_accel_type

7f0e9bc

Change-Id: I0aaf83fe0204c0db435692e9b92dee6e9d6997fe

Mousius force-pushed the ethosu_upstreaming_p3 branch from 532bd2c to 7f0e9bc Compare September 13, 2021 08:31

mbaret removed the status: WIP label Sep 13, 2021

jroesch approved these changes Sep 13, 2021

View reviewed changes

mbs-octoml reviewed Sep 14, 2021

View reviewed changes

areusch reviewed Sep 14, 2021

View reviewed changes

leandron merged commit 354019d into apache:main Sep 15, 2021

mikepapadim mentioned this pull request Sep 20, 2021

[hot-fix][CI]Disable mypy for ethosu #9050

Closed

areusch mentioned this pull request Sep 20, 2021

Add back missing __init__.py to unbreak CI. #9052

Merged

mbaret mentioned this pull request Sep 30, 2021

Address review comments on Arm(R) Ethos(TM)-U PR 3/6 #9159

Merged

junrushao mentioned this pull request Nov 1, 2021

Apache TVM v0.8 Release Note Candidate #9416

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3/6] Arm(R) Ethos(TM)-U NPU TIR compiler with conv2d support #8806

[3/6] Arm(R) Ethos(TM)-U NPU TIR compiler with conv2d support #8806

mbaret commented Aug 20, 2021 •

edited

Loading

jroesch commented Aug 20, 2021

mbaret commented Aug 20, 2021

jroesch commented Aug 20, 2021

manupak commented Sep 13, 2021

mbs-octoml commented Sep 13, 2021

jroesch left a comment

jroesch Sep 13, 2021

jroesch Sep 13, 2021

jroesch Sep 13, 2021

jroesch Sep 13, 2021

jroesch Sep 13, 2021

jroesch Sep 13, 2021

jroesch Sep 13, 2021

areusch Sep 14, 2021

jroesch Sep 13, 2021

jroesch Sep 13, 2021

manupak Sep 14, 2021 •

edited

Loading

Lunderberg Sep 14, 2021

jroesch Sep 13, 2021

mbaret commented Sep 14, 2021

mbs-octoml Aug 26, 2021

mbs-octoml Sep 10, 2021

mbs-octoml Sep 10, 2021

mbs-octoml Sep 10, 2021

mbs-octoml Sep 10, 2021

mbs-octoml commented Sep 14, 2021

areusch Sep 14, 2021

areusch Sep 14, 2021

manupak commented Sep 15, 2021

leandron commented Sep 15, 2021

	that comprises of a sequence of tir.extern_calls to NPU
	that consists of a sequence of tir.extern_calls to NPU

[3/6] Arm(R) Ethos(TM)-U NPU TIR compiler with conv2d support #8806

[3/6] Arm(R) Ethos(TM)-U NPU TIR compiler with conv2d support #8806

Conversation

mbaret commented Aug 20, 2021 • edited Loading

jroesch commented Aug 20, 2021

mbaret commented Aug 20, 2021

jroesch commented Aug 20, 2021

manupak commented Sep 13, 2021

mbs-octoml commented Sep 13, 2021

jroesch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak Sep 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbaret commented Sep 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbs-octoml commented Sep 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak commented Sep 15, 2021

leandron commented Sep 15, 2021

mbaret commented Aug 20, 2021 •

edited

Loading

manupak Sep 14, 2021 •

edited

Loading