-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[SLM][Bugfix] Output debug functions as impure #16687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ffd47e6
437d510
a8414a8
73e642a
154e35d
9845d7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -441,8 +441,8 @@ TVM_REGISTER_GLOBAL("relax.SeqExpr") | |
|
|
||
| TVM_REGISTER_NODE_TYPE(FunctionNode); | ||
|
|
||
| Function::Function(Array<Var> params, Expr body, Optional<StructInfo> ret_struct_info, bool is_pure, | ||
| DictAttrs attrs, Span span) { | ||
| Function::Function(Array<Var> params, Expr body, Optional<StructInfo> ret_struct_info, | ||
| Optional<Bool> is_pure_override, DictAttrs attrs, Span span) { | ||
| // Set the function type. | ||
| // For function, we take a conservative approach and require the function type | ||
| // to be known at construction time. | ||
|
|
@@ -473,6 +473,23 @@ Function::Function(Array<Var> params, Expr body, Optional<StructInfo> ret_struct | |
| ret_struct_info = body_sinfo; | ||
| } | ||
|
|
||
| bool is_pure = [&]() -> bool { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, the struct info for a We have a similar problem with return values, where the inferred return type of a function may not omitted, resulting in incorrect struct inference in the calling scope. I think the long-term solution to both is the same: To represent the lack of information while parsing, and to infer full information as a post-proc. |
||
| if (is_pure_override.defined()) { | ||
| return is_pure_override.value()->value; | ||
| } | ||
|
|
||
| if (attrs.defined() && attrs->dict.defined()) { | ||
| if (auto opt_force_pure = attrs->dict.Get(relax::attr::kForcePure)) { | ||
| bool force_pure = Downcast<IntImm>(opt_force_pure.value())->value; | ||
| if (force_pure) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return !ContainsImpureCall(body); | ||
| }(); | ||
|
|
||
| FuncStructInfo func_sinfo(param_sinfo, ret_struct_info.value(), is_pure); | ||
|
|
||
| // set the fields | ||
|
|
@@ -490,7 +507,7 @@ Function::Function(Array<Var> params, Expr body, Optional<StructInfo> ret_struct | |
|
|
||
| TVM_REGISTER_GLOBAL("relax.Function") | ||
| .set_body_typed([](Array<Var> params, Expr body, Optional<StructInfo> ret_struct_info, | ||
| bool is_pure, DictAttrs attrs, Span span) { | ||
| Optional<Bool> is_pure, DictAttrs attrs, Span span) { | ||
| return Function(params, body, ret_struct_info, is_pure, attrs, span); | ||
| }); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs for
is_pureshould mention that it's inferred.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And updated.