Lunderberg commented on code in PR #16569:
URL: https://github.com/apache/tvm/pull/16569#discussion_r1489637958
##########
python/tvm/script/parser/core/entry.py:
##########
@@ -77,4 +95,15 @@ def parse(program: Union[doc.AST, Any, str], extra_vars:
Dict[str, Any] = None)
parser.parse(extra_vars=extra_vars)
except ParserError as err:
parser.report_error(err.node, err.args[0])
- return builder.get()
+ ret = builder.get()
+ # well-formedness check will ignore any non-Relax functions
+ if isinstance(ret, IRModule):
Review Comment:
Can this be applied to a single relax function as well? If the result is
something other than an `IRModule`, we could construct
`IRModule.from_expr(ret)`, and apply the well-formed check there.
##########
python/tvm/script/parser/core/entry.py:
##########
@@ -77,4 +95,15 @@ def parse(program: Union[doc.AST, Any, str], extra_vars:
Dict[str, Any] = None)
parser.parse(extra_vars=extra_vars)
except ParserError as err:
parser.report_error(err.node, err.args[0])
- return builder.get()
+ ret = builder.get()
+ # well-formedness check will ignore any non-Relax functions
+ if isinstance(ret, IRModule):
+ # note: use the walrus operator (:=) once the project Python version
+ # supports it, would be more concise
+ source_ast = source.as_ast()
+ if find_decorator_annotation(source_ast, "check_well_formed") and not
well_formed(ret):
Review Comment:
Can we add a unit test validated that
`@I.ir_module(check_well_formed=False)` disables the well-formed check? This
is the only location where I see `check_well_formed` in this PR.
##########
python/tvm/script/parser/core/entry.py:
##########
@@ -77,4 +95,15 @@ def parse(program: Union[doc.AST, Any, str], extra_vars:
Dict[str, Any] = None)
parser.parse(extra_vars=extra_vars)
except ParserError as err:
parser.report_error(err.node, err.args[0])
- return builder.get()
+ ret = builder.get()
+ # well-formedness check will ignore any non-Relax functions
Review Comment:
Instead of ignoring the non-Relax functions, can we run the TIR equivalent
`tvm.tir.analysis.verify_well_formed` function?
##########
tests/python/relax/test_transform_normalize_global_var.py:
##########
@@ -15,31 +15,31 @@
# specific language governing permissions and limitations
# under the License.
import pytest
+import numpy as np
import tvm
import tvm.testing
from tvm import relax
-from tvm import tir
from tvm.ir.base import assert_structural_equal
import tvm.script
from tvm.script import tir as T, relax as R, ir as I
-@pytest.mark.skip_well_formed_check_before_transform
def test_normalize_relax_function():
- @I.ir_module
- class Before:
- @R.function(private=True)
- def f():
- return R.const(1, "int32")
-
- @R.function
- def f1():
- R.func_attr({"global_symbol": "f"})
- cls = Before
- gv: R.Tensor((), dtype="int32") = cls.f()
- return gv
+ # parser will check well-formedness so we can't use it to construct this
example
Review Comment:
Instead of changing to the `relax.BlockBuilder`, can we use the
`check_well_formed=False` argument?
##########
tests/python/relax/test_transform_operator_specific_normalization.py:
##########
@@ -261,9 +303,9 @@ def multiply_by_two(A: T.Buffer(16, "float32"), B:
T.Buffer(16, "float32")):
def test_normalize_to_inline_tuple_for_call_tir_inplace(custom_op):
"""FNormalize in-lines the argument tuple for R.call_tir_inplace"""
- # The CallTIRInplaceAttrs cannot be constructed from the Python
- # API. Therefore, declaring the Expected output first, so that
- # the attributes can be used for the non-normalized Before.
Review Comment:
Good point. It's definitely more tedious than I'd like, and (I think)
requires going through the `make_node` API rather than having a python
constructor.
--
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: commits-unsubscr...@tvm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
