manupa-arm commented on a change in pull request #8509:
URL: https://github.com/apache/tvm/pull/8509#discussion_r790598211
##########
File path: include/tvm/ir/module.h
##########
@@ -349,6 +393,9 @@ class IRModuleNode : public Object {
*/
std::unordered_set<String> import_set_;
friend class IRModule;
+
+ public:
+ void ExtractPrimFuncConstants(tir::PrimFunc func);
Review comment:
Just a suggestion : Since this is part of the unified IR module
definition, should we keep it for BaseFunc for now ? and implement it for
tir::PrimFunc for this PR. Thus, the community might able to do a similiar for
relay::Function to complete the implementation. This way we might able to
remove the forward declarations as well.
##########
File path: include/tvm/tir/function.h
##########
@@ -151,42 +151,6 @@ class PrimFunc : public BaseFunc {
TVM_DEFINE_OBJECT_REF_COW_METHOD(PrimFuncNode);
};
-/*!
- * \brief Describes one parameter that should be linked into the generated
module.
- *
- * When parameters are to be linked in with generated code (i.e. on
target_host-compatible
- * backends), Relay attaches instances of this object to a global TIR
function. Code-generators
- * use the information contained in this node to include the parameter data in
the generated
- * module.
- */
-class LinkedParamNode : public Object {
Review comment:
I would like to understand more why this movement is needed
##########
File path: include/tvm/ir/module.h
##########
@@ -39,8 +39,52 @@
#include <utility>
#include <vector>
+namespace tvm {
+namespace tir {
+// Forward declarations from tvm::tir
+class PrimFunc;
+
+/*!
+ * \brief Describes one parameter that should be linked into the generated
module.
+ *
+ * When parameters are to be linked in with generated code (i.e. on
target_host-compatible
+ * backends), Relay attaches instances of this object to a global TIR
function. Code-generators
+ * use the information contained in this node to include the parameter data in
the generated
+ * module.
+ */
+class LinkedParamNode : public Object {
Review comment:
For my understanding : Why is this moved in this PR ? Can we keep it in
a codegen associated class ?
##########
File path: src/relay/transforms/fuse_ops.cc
##########
@@ -1017,18 +1050,30 @@ class FuseMutator : private MixedModeMutator {
}
};
-Expr FuseOps(const Expr& expr, int fuse_opt_level, size_t max_fuse_depth,
const IRModule& module) {
- return FuseMutator().Transform(expr, fuse_opt_level, max_fuse_depth);
+Expr FuseOps(const Expr& expr, int fuse_opt_level, size_t max_fuse_depth, bool
link_params,
+ const IRModule& module) {
+ return FuseMutator()
+ .SetFuseOptLevel(fuse_opt_level)
+ ->SetMaxFuseDepth(max_fuse_depth)
+ ->SetLinkParams(link_params)
+ ->Transform(expr);
}
namespace transform {
Pass FuseOps(int fuse_opt_level) {
runtime::TypedPackedFunc<Function(Function, IRModule, PassContext)>
pass_func =
[=](Function f, IRModule m, PassContext pc) {
+ bool link_params = false;
+ Executor executor =
+
m->GetAttr<Executor>(tvm::attr::kExecutor).value_or(NullValue<Executor>());
+ link_params = executor.defined()
+ ?
executor->attrs.GetAttr<Bool>("link-params").value_or(Bool(link_params))
+ : link_params;
+ link_params = pc->GetConfig("relay.FuseOps.link_params",
Bool(link_params)).value();
Review comment:
Why are we defining two ways to link_params here ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
