Lunderberg commented on code in PR #16641:
URL: https://github.com/apache/tvm/pull/16641#discussion_r1503572139
##########
python/tvm/script/parser/relax/parser.py:
##########
@@ -274,7 +274,21 @@ def post_visit_local_function(self: Parser, node:
doc.Expr) -> None:
@dispatch.register(token="relax", type_name="Expr")
def visit_expr_stmt(self: Parser, node: doc.Expr) -> None:
value = self.eval_expr(node.value)
- if value is not None:
+ if isinstance(value, relax.Expr):
+ var = R.emit(value)
+ IRBuilder.name("_", var)
+ is_void_value = (
+ isinstance(var.struct_info, relax.TupleStructInfo) and
len(var.struct_info.fields) == 0
+ )
+
+ if not is_void_value:
+ self.report_error(
+ node,
+ f"Non-void relax expressions must be bound to a variable, "
+ f"but expression of type {var.struct_info} was used as a
statement.",
+ )
Review Comment:
At the moment, because I wanted to make the minimal change that would
support common cases. I think it would be good to remove the restriction
altogether, but for the first step, I wanted to make the restriction be
explicit.
There's a couple of concerns I could see with allowing non-void return value
to be implicitly ignored.
* Prevent accidentally unused values. If two IRModule instances An in-place
operator that performs `a = a + b` may be represented as `cls.add(a, b)`. This
woul
* Round-trip TVMScript -> Relax -> TVMScript without a pre-processing pass.
Checking if a value has void type can done while printing the IR. Checking
whether a non-void variable could be omitted would require a pre-processing
step to find any downstream users.
I don't think either of those are definitive arguments, but I figured I'd
handle the unambiguous beneficial cases first, with a follow-up PR to relax the
restriction.
--
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]
