[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #13410: [ETHOSN] Add support for experimental compiler option

[GitBox <gi...@apache.org>]

ashutosh-arm commented on code in PR #13410:
URL: https://github.com/apache/tvm/pull/13410#discussion_r1024125138


##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -101,6 +101,18 @@ def is_inline_non_compute_intensive_partitions_enabled() -> bool:
     return compiler_attrs.inline_non_compute_intensive_partitions
 
 
+def is_experimental_compiler_enabled() -> bool:

Review Comment:
   Is this function useful now that the env variable is being set from NPU codegen?



##########
src/relay/backend/contrib/ethosn/codegen_ethosn.h:
##########
@@ -285,6 +286,9 @@ struct EthosnCompilerConfigNode : public tvm::AttrsNode<EthosnCompilerConfigNode
             "Ethos(TM)-N that are deemed 'non-compute-intensive'. The inlined functions will "
             "continue through TVM's standard compilation flow.")
         .set_default(true);
+    TVM_ATTR_FIELD(experimental_compiler)
+        .describe("An exprimental cascading compiler for Arm(R) Ethos(TM)-N.")

Review Comment:
   nit: can we please add more details about the experimental compiler - which use case is it meant for,  Is this a temporary knob etc?



##########
tests/python/contrib/test_ethosn/test_codegen.py:
##########
@@ -50,3 +50,30 @@ def test_compile_with_unsupported_variant():
 
     with pytest.raises(tvm.TVMError, match=r"Unknown NPU type"):
         tei.build_and_run(mod, inputs, 1, {}, True, additional_config_args=additional_config_args)
+
+
+@requires_ethosn
+def test_experimental_compiler(capfd):
+    """Test compilation with the experimental compiler."""
+    dtype = "int8"
+    input_shape = (1, 2, 2, 2)
+
+    x = relay.var("x", shape=input_shape, dtype=dtype)
+    y = relay.reshape(x, newshape=(1, 1, 1, 8))
+    mod = tei.make_ethosn_partition(y)
+
+    additional_config_args = {
+        "variant": "n78",
+        "experimental_compiler": True,

Review Comment:
   nit: a negative case (default) is needed?



##########
src/relay/backend/contrib/ethosn/codegen.cc:
##########
@@ -713,9 +713,17 @@ runtime::ethosn::OrderedCompiledNetwork EthosnCompiler::CompileEthosnFunc(const
   auto network_with_ids = ConstructNetwork(mod, gvar, func);
   // Now set the required build flags
   sl::CompilationOptions options = CreateOptions();
-  // Finally compile the network
+  // Set the experimental compiler if enabled, for now this is not part of the

Review Comment:
   Setting and unsetting the variable from here means that its being set multiple times during the compilation. Is there a better place from where it gets set/unset only once? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org