manupa-arm commented on code in PR #11228:
URL: https://github.com/apache/tvm/pull/11228#discussion_r871459251
##########
python/tvm/relay/transform/transform.py:
##########
@@ -261,32 +261,39 @@ def LazyGradientInit():
return _ffi_api.LazyGradientInit()
-def FoldConstantExpr(expr, mod):
+def FoldConstantExpr(expr, mod, fold_qnn=False):
"""Fold the constant expressions in a Relay program.
Parameters
----------
expr: Expr
The expression to fold
mod: IRModule
The module the expr lives in (for global calls)
+ fold_qnn: bool
+ Whether to fold constants for QNN operations.
Returns
-------
new_expr: Expr
The expr after Constant Folding
"""
- return _ffi_api.FoldConstantExpr(expr, mod)
+ return _ffi_api.FoldConstantExpr(expr, mod, fold_qnn)
-def FoldConstant():
+def FoldConstant(fold_qnn=False):
"""Fold the constant expressions in a Relay program.
+ Parameters
+ ----------
+ fold_qnn: bool
Review Comment:
As for the description, I agree with content. Since you asked me :) :
Maybe a general comment just highlight if a subsequent pass requires qnn
dialect to be present even in constant sub-Expr, this flag should be set to
false and cite the example ?
the second point,
Im personally happy with a binary choice for now.
But I like the increased customizability of your suggestion. However, I dont
immediately see a requirement for it. If we do that, we'd need a bit more test
coverage as well.
Maybe its better to keep it simple at this stage and consider a follow up ?
(It might better to get comments from the wider community as to the level of
customization required. E.g. invidual qnn.ops , dialect of relay ops that get
legalized (e.g. qnn) or individual op from all Legalizable ops)
--
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]
