You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/06/07 23:16:47 UTC
[GitHub] [tvm] mbs-octoml opened a new pull request, #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
mbs-octoml opened a new pull request, #11619:
URL: https://github.com/apache/tvm/pull/11619
In #11474 I got ready to switch CUTLASS from function-at-a-time to IRModule-at-a-time compilation.
However my approach didn't handle dynamic shape functions, so I adjust it here.
The idea is still that such passes will leave behind
calls to 'extern' functions. However, converting those
calls to 'call_lowered' form in
MarkCompilerFunctionsAsExtern is too soon since only
the TECompiler knows how to capture all the attributes
necessary to support dynamic shape functions.
So stop doing that in MarkCompilerFunctionsAsExtern and
instead support this case properly in the TECompiler.
While there try to chip away at the chronic lack of structure in te_compiler.cc. Every little bit helps.
There are no unit tests for te_compiler I can build on...
--
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
[GitHub] [tvm] masahi merged pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
masahi merged PR #11619:
URL: https://github.com/apache/tvm/pull/11619
--
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
[GitHub] [tvm] mbs-octoml commented on pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on PR #11619:
URL: https://github.com/apache/tvm/pull/11619#issuecomment-1150284125
Thanks @areusch , I've added a large intro comment to capture what I've reverse engineered.
--
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
[GitHub] [tvm] areusch commented on a diff in pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11619:
URL: https://github.com/apache/tvm/pull/11619#discussion_r894020909
##########
tests/python/relay/backend/test_pass_lower_te.py:
##########
@@ -0,0 +1,241 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Exercises the LowerTE pass.
+
+import tvm
+import tvm.testing
+import logging
+
+logging.basicConfig()
+logger = logging.getLogger("test_pass_lower_te")
+logger.setLevel(logging.INFO)
+
+# Since the TE compiler needs a good refactor it has not been exposed as a 'standard' pass
+# in relay.transform. For testing grab it directly.
+LowerTE = tvm._ffi.get_global_func("relay.tec.LowerTE")
+
+
+def transform(mod):
+ logger.info("Starting module:\n%s", mod)
+ host_target = tvm.target.Target("llvm")
+ prim_target = tvm.target.Target("llvm", host=host_target)
+ ctxt = tvm.transform.PassContext()
+ config = tvm.target.make_compilation_config(ctxt, prim_target)
+ mod = tvm.relay.transform.PlanDevices(config)(mod)
+ mod = tvm.relay.transform.InferType()(mod)
+ mod = LowerTE("test", config)(mod)
+ mod = tvm.relay.transform.InferType()(mod)
+ logger.info("After LowerTE:\n%s", mod)
+ return mod
+
+
+# All attempts to use structural equalty tests against an expected IRModule parsed from
+# Relay text were thwarted by the difficulty of setting up the expected call_lower attributes
+# with the right GlobalVar instances. So the following assert structural correctness the hard way.
+
+
+def test_lower_primitive():
+ input_mod = tvm.parser.parse(
+ """
+ #[version = "0.0.5"]
+ def @main(%a: Tensor[(5, 7), float32]) -> Tensor[(5, 7), float32] {
+ %0 = fn(%x : Tensor[(5, 7), float32], %y : Tensor[(5, 7), float32], Primitive=1) -> Tensor[(5, 7), float32] {
+ add(%x, %y)
+ };
+ %0(%a, %a)
+ }
+ """,
+ "from_string",
+ None,
+ None,
+ )
+
+ actual_mod = transform(input_mod)
+
+ # Expected:
+ # def @main(%a: Tensor[(5, 7), float32]) -> Tensor[(5, 7), float32] {
+ # %0 = (%a, %a);
+ # call_lowered(@test_fused_add, %0, metadata={relay_attrs={Primitive=1},all_prim_fn_vars=[@test_fused_add]})
+ # }
+ # def @test_fused_add = <lowered PrimFunc>
+
+ main = actual_mod["main"]
+ call = main.body
+ assert call.op.name == "call_lowered"
Review Comment:
i am sad :(
--
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
[GitHub] [tvm] mbs-octoml commented on pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on PR #11619:
URL: https://github.com/apache/tvm/pull/11619#issuecomment-1151572684
@areusch PTAL, added a bare-bones-but-at-least-it's-a-start unit test in test_pass_lower_te.py.
Spent way too much time trying to make the tests of the form assert_equals(output, parse("...")) but gave up due to difficulties with matching GlobalVars.
--
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
[GitHub] [tvm] areusch commented on a diff in pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #11619:
URL: https://github.com/apache/tvm/pull/11619#discussion_r892630449
##########
src/relay/backend/te_compiler.cc:
##########
@@ -697,43 +725,51 @@ class LowerTensorExprMutator : public DeviceAwareExprMutator {
}
Expr DeviceAwareVisitExpr_(const CallNode* call_node) override {
- // We can see five forms of calls:
- // 1. A 'normal' Relay call to a Function with the "primitive" attribute. We will need
- // to lower that to a global PrimFunc and rewrite the call to:
+ // We can see six forms of calls:
+ // 1. A 'normal' Relay call to a Function with the "Primitive" attribute and not "Compiler"
+ // attribute. We will need to lower that to a global PrimFunc and rewrite the call to:
// call_lowered(@new_global, (arg1, ..., argn), <attributes>)
- // However there are a few special forms which are excluded from this treatment, see
- // below.
- // 2. A 'normal' Relay call to a Function with the "compiler" attribute. We will need
- // to invoke the appropriate BYOC toolchain function to yield a runtime module and
- // rewrite the call to the same form as above.
- // 3. A 'normal' Relay call to a PrimFunc which has already been supplied via a global
- // definition. We rewrite to use the call_lowered form, but otherwise nothing else
+ // If needed, the call needs to be cross-linked with any dynamic shape functions.
Review Comment:
what do you mean by cross-linked here?
##########
src/relay/backend/te_compiler.cc:
##########
@@ -566,100 +566,128 @@ class LowerTensorExprMutator : public DeviceAwareExprMutator {
return itr->second;
}
} else if (const auto* function_node = expr.as<FunctionNode>()) {
- if (!function_node->HasNonzeroAttr(attr::kPrimitive)) {
- // Not marked as primitive by FuseOps.
- return {};
- }
- if (const auto* call_node = function_node->body.as<CallNode>()) {
- if (call_node->op == debug_op_) {
- // Debug 'primitives' are not lowered.
- return {};
+ if (function_node->HasNonzeroAttr(attr::kExtern)) {
Review Comment:
does this functionality need test?
##########
src/relay/backend/te_compiler.cc:
##########
@@ -566,100 +566,128 @@ class LowerTensorExprMutator : public DeviceAwareExprMutator {
return itr->second;
}
} else if (const auto* function_node = expr.as<FunctionNode>()) {
- if (!function_node->HasNonzeroAttr(attr::kPrimitive)) {
- // Not marked as primitive by FuseOps.
- return {};
- }
- if (const auto* call_node = function_node->body.as<CallNode>()) {
- if (call_node->op == debug_op_) {
- // Debug 'primitives' are not lowered.
- return {};
+ if (function_node->HasNonzeroAttr(attr::kExtern)) {
+ // We have a regular call to an 'extern' function. The call itself needs to be rewritten
+ // to call_lowered form, and any required dynamic shape functions generated and
+ // cross-linked.
+ return GetRef<Function>(function_node);
+ } else if (function_node->HasNonzeroAttr(attr::kPrimitive)) {
+ if (const auto* call_node = function_node->body.as<CallNode>()) {
+ if (call_node->op == debug_op_) {
+ // Debug 'primitives' are not lowered.
+ return {};
+ }
}
+ // We have a regular call to a 'primitive' function (possibly with a 'Compiler' attribute).
+ // We need to lower and rewrite the call.
+ return GetRef<Function>(function_node);
+ } else {
+ // Not marked as primitive during partitioning or TVM fusion.
+ return {};
}
- return GetRef<Function>(function_node);
} else {
return {};
}
}
/*!
- * \brief Lowers the primitive function \p func to TIR for ultimate execution
- * on a device with configuration \p target. Returns the global var bound
- * to the TIR implementation, and attributes to attach to the call to identify it as
- * a TIR call.
+ * \brief Returns a 'call_lowered' call to \p prim_fn_var with \p args and \p span with all the
+ * required attributes filled in. Generally \p prim_fn_var will correspond to the lowered or
+ * externally codegen-ed form of \p original_function, where \p lowered_functions binds all
+ * the required lowered functions.
+ *
+ * The call's attributes will capture:
+ * - Any attributes on the original_function.
+ * - All the lowered functions.
+ * TODO(mbs): Pretty sure that's no longer needed.
+ * - Details needed to cross-link the call to it's dynamic shape function, if any.
*/
- Expr MakeLoweredCall(Function func, Array<Expr> visited_args, Span span, Target target) {
- CCacheKey key = CCacheKey(func, target);
- CachedFunc cfunc = compiler_->Lower(key, module_name_);
- ICHECK(cfunc.defined());
-
- auto opt_compiler = func->GetAttr<String>(attr::kCompiler);
+ Expr MakeLoweredCall(const BaseFunc& original_function, const GlobalVar& prim_fn_var,
+ Array<Expr> args, Span span, const Target& target,
+ const Map<GlobalVar, BaseFunc>& lowered_functions) {
+ auto opt_compiler = original_function->GetAttr<String>(attr::kCompiler);
// Add some metadata on top of the *original function* and invoke the callback so it can
// be captured.
// TODO(@areusch, @jroesch): this metadata is for AOT, this should be our interface for AOT
Map<GlobalVar, tir::PrimFunc> prim_fns;
Array<GlobalVar> all_prim_fn_vars;
- for (const auto& kv : cfunc->funcs->functions) {
+ for (const auto& kv : lowered_functions) {
if (opt_compiler) {
- // We expect just the original func but with just the ExternalSymbol attribute signaling
- // the function (will be) compiled externally.
+ // We expect the original function to have just the "Extern" attribute signaling the
+ // function (will be) compiled externally.
ICHECK(kv.second.as<FunctionNode>())
<< PrettyPrint(kv.first) << " must be bound to an (external) Function";
} else {
- // We expect one or more PrimFuncs, one of which corresponds to 'the' lowered primitive
- // (and the rest in support of that via tir::Calls).
+ // We expect one or more PrimFuncs, one of which corresponds to 'the' lowered primitive,
+ // and the rest are in support of that via tir::Calls.
ICHECK(kv.second.as<tir::PrimFuncNode>())
<< PrettyPrint(kv.first) << " must be bound to a PrimFunc";
prim_fns.Set(kv.first, Downcast<tir::PrimFunc>(kv.second));
all_prim_fn_vars.push_back(kv.first);
}
}
- Function func_with_metadata = func;
- func_with_metadata = WithAttr(func_with_metadata, "prim_fn_var", cfunc->prim_fn_var);
- func_with_metadata = WithAttr(func_with_metadata, "prim_funcs", prim_fns);
- func_with_metadata = WithAttr(func_with_metadata, tvm::attr::kTarget, cfunc->target);
- this->process_fn_(func_with_metadata);
+ // Alas, WithAttr cannot work with base classes.
Review Comment:
i wanted to see if a templated lambda would work here, but alas that's in C++20
##########
src/relay/transforms/compiler_function_utils.cc:
##########
@@ -167,20 +131,6 @@ transform::Pass MarkCompilerFunctionsAsExtern(std::string compiler_filter) {
runtime::TypedPackedFunc<IRModule(IRModule, transform::PassContext)> pass_func =
[compiler_filter = std::move(compiler_filter)](IRModule mod, transform::PassContext ctx) {
IRModule output_mod = mod->ShallowCopy();
-
- // First pass, rewrite the calls.
- // We have to do this before marking functions as 'extern' to know which calls to rewrite!
Review Comment:
checking my understanding: this is no longer needed because LowerTensorExprMutator handles it?
--
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
[GitHub] [tvm] mbs-octoml commented on pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on PR #11619:
URL: https://github.com/apache/tvm/pull/11619#issuecomment-1151778506
@tvm-bot merge
--
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
[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11619:
URL: https://github.com/apache/tvm/pull/11619#discussion_r892705385
##########
src/relay/transforms/compiler_function_utils.cc:
##########
@@ -167,20 +131,6 @@ transform::Pass MarkCompilerFunctionsAsExtern(std::string compiler_filter) {
runtime::TypedPackedFunc<IRModule(IRModule, transform::PassContext)> pass_func =
[compiler_filter = std::move(compiler_filter)](IRModule mod, transform::PassContext ctx) {
IRModule output_mod = mod->ShallowCopy();
-
- // First pass, rewrite the calls.
- // We have to do this before marking functions as 'extern' to know which calls to rewrite!
Review Comment:
That's right -- I thought I was doing the right thing by converting to extern AND rewriting the calls all at once, but the second must be left to te_compiler.cc.
--
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
[GitHub] [tvm] mbs-octoml commented on pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on PR #11619:
URL: https://github.com/apache/tvm/pull/11619#issuecomment-1150432776
As discussed offline I'm going to pay down some testing tech dept and ping back.
--
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
[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11619:
URL: https://github.com/apache/tvm/pull/11619#discussion_r892703621
##########
src/relay/backend/te_compiler.cc:
##########
@@ -566,100 +566,128 @@ class LowerTensorExprMutator : public DeviceAwareExprMutator {
return itr->second;
}
} else if (const auto* function_node = expr.as<FunctionNode>()) {
- if (!function_node->HasNonzeroAttr(attr::kPrimitive)) {
- // Not marked as primitive by FuseOps.
- return {};
- }
- if (const auto* call_node = function_node->body.as<CallNode>()) {
- if (call_node->op == debug_op_) {
- // Debug 'primitives' are not lowered.
- return {};
+ if (function_node->HasNonzeroAttr(attr::kExtern)) {
+ // We have a regular call to an 'extern' function. The call itself needs to be rewritten
+ // to call_lowered form, and any required dynamic shape functions generated and
+ // cross-linked.
+ return GetRef<Function>(function_node);
+ } else if (function_node->HasNonzeroAttr(attr::kPrimitive)) {
+ if (const auto* call_node = function_node->body.as<CallNode>()) {
+ if (call_node->op == debug_op_) {
+ // Debug 'primitives' are not lowered.
+ return {};
+ }
}
+ // We have a regular call to a 'primitive' function (possibly with a 'Compiler' attribute).
+ // We need to lower and rewrite the call.
+ return GetRef<Function>(function_node);
+ } else {
+ // Not marked as primitive during partitioning or TVM fusion.
+ return {};
}
- return GetRef<Function>(function_node);
} else {
return {};
}
}
/*!
- * \brief Lowers the primitive function \p func to TIR for ultimate execution
- * on a device with configuration \p target. Returns the global var bound
- * to the TIR implementation, and attributes to attach to the call to identify it as
- * a TIR call.
+ * \brief Returns a 'call_lowered' call to \p prim_fn_var with \p args and \p span with all the
+ * required attributes filled in. Generally \p prim_fn_var will correspond to the lowered or
+ * externally codegen-ed form of \p original_function, where \p lowered_functions binds all
+ * the required lowered functions.
+ *
+ * The call's attributes will capture:
+ * - Any attributes on the original_function.
+ * - All the lowered functions.
+ * TODO(mbs): Pretty sure that's no longer needed.
+ * - Details needed to cross-link the call to it's dynamic shape function, if any.
*/
- Expr MakeLoweredCall(Function func, Array<Expr> visited_args, Span span, Target target) {
- CCacheKey key = CCacheKey(func, target);
- CachedFunc cfunc = compiler_->Lower(key, module_name_);
- ICHECK(cfunc.defined());
-
- auto opt_compiler = func->GetAttr<String>(attr::kCompiler);
+ Expr MakeLoweredCall(const BaseFunc& original_function, const GlobalVar& prim_fn_var,
+ Array<Expr> args, Span span, const Target& target,
+ const Map<GlobalVar, BaseFunc>& lowered_functions) {
+ auto opt_compiler = original_function->GetAttr<String>(attr::kCompiler);
// Add some metadata on top of the *original function* and invoke the callback so it can
// be captured.
// TODO(@areusch, @jroesch): this metadata is for AOT, this should be our interface for AOT
Map<GlobalVar, tir::PrimFunc> prim_fns;
Array<GlobalVar> all_prim_fn_vars;
- for (const auto& kv : cfunc->funcs->functions) {
+ for (const auto& kv : lowered_functions) {
if (opt_compiler) {
- // We expect just the original func but with just the ExternalSymbol attribute signaling
- // the function (will be) compiled externally.
+ // We expect the original function to have just the "Extern" attribute signaling the
+ // function (will be) compiled externally.
ICHECK(kv.second.as<FunctionNode>())
<< PrettyPrint(kv.first) << " must be bound to an (external) Function";
} else {
- // We expect one or more PrimFuncs, one of which corresponds to 'the' lowered primitive
- // (and the rest in support of that via tir::Calls).
+ // We expect one or more PrimFuncs, one of which corresponds to 'the' lowered primitive,
+ // and the rest are in support of that via tir::Calls.
ICHECK(kv.second.as<tir::PrimFuncNode>())
<< PrettyPrint(kv.first) << " must be bound to a PrimFunc";
prim_fns.Set(kv.first, Downcast<tir::PrimFunc>(kv.second));
all_prim_fn_vars.push_back(kv.first);
}
}
- Function func_with_metadata = func;
- func_with_metadata = WithAttr(func_with_metadata, "prim_fn_var", cfunc->prim_fn_var);
- func_with_metadata = WithAttr(func_with_metadata, "prim_funcs", prim_fns);
- func_with_metadata = WithAttr(func_with_metadata, tvm::attr::kTarget, cfunc->target);
- this->process_fn_(func_with_metadata);
+ // Alas, WithAttr cannot work with base classes.
Review Comment:
yeah i was disappointed by this too
--
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
[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11619:
URL: https://github.com/apache/tvm/pull/11619#discussion_r892704432
##########
src/relay/backend/te_compiler.cc:
##########
@@ -697,43 +725,51 @@ class LowerTensorExprMutator : public DeviceAwareExprMutator {
}
Expr DeviceAwareVisitExpr_(const CallNode* call_node) override {
- // We can see five forms of calls:
- // 1. A 'normal' Relay call to a Function with the "primitive" attribute. We will need
- // to lower that to a global PrimFunc and rewrite the call to:
+ // We can see six forms of calls:
+ // 1. A 'normal' Relay call to a Function with the "Primitive" attribute and not "Compiler"
+ // attribute. We will need to lower that to a global PrimFunc and rewrite the call to:
// call_lowered(@new_global, (arg1, ..., argn), <attributes>)
- // However there are a few special forms which are excluded from this treatment, see
- // below.
- // 2. A 'normal' Relay call to a Function with the "compiler" attribute. We will need
- // to invoke the appropriate BYOC toolchain function to yield a runtime module and
- // rewrite the call to the same form as above.
- // 3. A 'normal' Relay call to a PrimFunc which has already been supplied via a global
- // definition. We rewrite to use the call_lowered form, but otherwise nothing else
+ // If needed, the call needs to be cross-linked with any dynamic shape functions.
Review Comment:
I expanded the comment. It just means attaching attributes so that we can handle the dynamic shape when converting to DPS in memory_alloc.cc
--
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
[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11619:
URL: https://github.com/apache/tvm/pull/11619#discussion_r892706236
##########
src/relay/backend/te_compiler.cc:
##########
@@ -566,100 +566,128 @@ class LowerTensorExprMutator : public DeviceAwareExprMutator {
return itr->second;
}
} else if (const auto* function_node = expr.as<FunctionNode>()) {
- if (!function_node->HasNonzeroAttr(attr::kPrimitive)) {
- // Not marked as primitive by FuseOps.
- return {};
- }
- if (const auto* call_node = function_node->body.as<CallNode>()) {
- if (call_node->op == debug_op_) {
- // Debug 'primitives' are not lowered.
- return {};
+ if (function_node->HasNonzeroAttr(attr::kExtern)) {
Review Comment:
Everything needs testing, but there's nothing to build on so we keep kicking the can ahead of us.How about as a compromise I put some love into the comments so we at least know what needs to be tested?
--
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
[GitHub] [tvm] mbs-octoml commented on pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on PR #11619:
URL: https://github.com/apache/tvm/pull/11619#issuecomment-1149262976
(Will give this a run through ci in draft form).
--
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
[GitHub] [tvm] areusch commented on pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
areusch commented on PR #11619:
URL: https://github.com/apache/tvm/pull/11619#issuecomment-1151737195
tagging issue describing test challenges to cross-reference it. https://github.com/apache/tvm/issues/11661
--
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
[GitHub] [tvm] github-actions[bot] commented on pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11619:
URL: https://github.com/apache/tvm/pull/11619#issuecomment-1151778919
Cannot merge, these CI jobs are not successful on 78d92c8cad87814aecb83a5cbc5634c39b3f7120:
* [tvm-bot / run-tvm-bot (`NEUTRAL`)](https://github.com/apache/tvm/runs/6822895414)
--
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
[GitHub] [tvm] mbs-octoml commented on a diff in pull request #11619: [BYOC] RelayToTIR custom codegen passes can still depend on dynamic shape functions
Posted by GitBox <gi...@apache.org>.
mbs-octoml commented on code in PR #11619:
URL: https://github.com/apache/tvm/pull/11619#discussion_r892704837
##########
src/relay/backend/te_compiler.cc:
##########
@@ -697,43 +725,51 @@ class LowerTensorExprMutator : public DeviceAwareExprMutator {
}
Expr DeviceAwareVisitExpr_(const CallNode* call_node) override {
- // We can see five forms of calls:
- // 1. A 'normal' Relay call to a Function with the "primitive" attribute. We will need
- // to lower that to a global PrimFunc and rewrite the call to:
+ // We can see six forms of calls:
+ // 1. A 'normal' Relay call to a Function with the "Primitive" attribute and not "Compiler"
+ // attribute. We will need to lower that to a global PrimFunc and rewrite the call to:
// call_lowered(@new_global, (arg1, ..., argn), <attributes>)
- // However there are a few special forms which are excluded from this treatment, see
- // below.
- // 2. A 'normal' Relay call to a Function with the "compiler" attribute. We will need
- // to invoke the appropriate BYOC toolchain function to yield a runtime module and
- // rewrite the call to the same form as above.
- // 3. A 'normal' Relay call to a PrimFunc which has already been supplied via a global
- // definition. We rewrite to use the call_lowered form, but otherwise nothing else
+ // If needed, the call needs to be cross-linked with any dynamic shape functions.
Review Comment:
Actually, since I've just paged all this in let me spend some quality time and comment the whole file.
--
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