slyubomirsky commented on code in PR #16687:
URL: https://github.com/apache/tvm/pull/16687#discussion_r1525679154
##########
python/tvm/script/ir_builder/relax/ir.py:
##########
@@ -222,7 +222,7 @@ def to_vdevice(data: Expr, dst_vdevice: Union[py_str,
VDevice]) -> Expr:
############################### Function ################################
-def function(is_pure: bool = True, is_private: bool = False) ->
frame.FunctionFrame:
+def function(is_pure: Optional[bool] = None, is_private: bool = False) ->
frame.FunctionFrame:
Review Comment:
The docs for `is_pure` should mention that it's inferred.
##########
src/relax/ir/expr.cc:
##########
@@ -473,6 +473,23 @@ Function::Function(Array<Var> params, Expr body,
Optional<StructInfo> ret_struct
ret_struct_info = body_sinfo;
}
+ bool is_pure = [&]() -> bool {
Review Comment:
How would this behave in the recursive or mutually recursive case? That was
the reason for not inferring purity in the first place. Detecting those cases
and warning the user that they need to be explicitly annotated would be a
reasonable approach
##########
include/tvm/relax/expr.h:
##########
@@ -983,16 +983,21 @@ class FunctionNode : public BaseFuncNode {
class Function : public BaseFunc {
public:
TVM_DLL explicit Function(Array<Var> params, Expr body, Optional<StructInfo>
ret_struct_info,
- bool is_pure = true, DictAttrs attrs =
NullValue<DictAttrs>(),
- Span span = Span());
+ bool is_pure, DictAttrs attrs =
NullValue<DictAttrs>(),
+ Span span = Span())
+ : Function(params, body, ret_struct_info, Optional<Bool>(Bool(is_pure)),
attrs, span) {}
+
+ TVM_DLL explicit Function(Array<Var> params, Expr body,
+ Optional<StructInfo> ret_struct_info = NullOpt,
+ Optional<Bool> is_pure = NullOpt,
+ DictAttrs attrs = NullValue<DictAttrs>(), Span
span = Span());
/*!
* \brief Mimics the constructor but without body Expr.
- * \note ret_struct_info is required, since it can not deduced by the body.
+ * \note `ret_struct_info` and `is_pure` are required, since it can not
deduced by the body.
Review Comment:
```suggestion
* \note `ret_struct_info` and `is_pure` are required, since they cannot
be deduced from the body.
```
Might as well correct the typo 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
