MasterJH5574 commented on PR #16732: URL: https://github.com/apache/tvm/pull/16732#issuecomment-2029063222
Hi @Lunderberg, it seems that this PR introduces new regression. The original issue came from CI in MLC-LLM https://ci.mlc.ai/blue/organizations/jenkins/mlc-llm/detail/PR-2068/1/pipeline/. And I spent some time adapting your test `test_backtrack_if_rewriter_returns_no_op` and reducing it to the minimal I can get. Running the test below ```python def test_backtrack_if_rewriter_returns_no_op(): pat_match_no_rewrite = is_op("relax.add")(wildcard(), wildcard()) pat_arg = wildcard() pat_zeros = is_op("relax.zeros")(wildcard()) pat_add = is_op("relax.add")(pat_arg, pat_zeros) pat = pat_match_no_rewrite | pat_add def rewriter(expr, matches): print(f"matching {pat} to {matches[pat]}") assert isinstance(matches[pat], rx.Call) if pat_match_no_rewrite in matches: return expr ## <<== return the expr elif pat_add in matches: return expr ## <<== also return the expr else: raise RuntimeError("Pattern matched, but neither branch matched") @R.function(private=True) def before(): with R.dataflow(): A = R.ones([64, 128], "int32") B = R.zeros([64, 128], "int32") C = R.add(A, B) R.output(C) return C after = rewrite_call(pat, rewriter, before) ``` will yield the following output: ``` matching OrPattern(Op(relax.add)(*, *) | Op(relax.add)(*, Op(relax.zeros)(*))) to R.add(A, B) matching OrPattern(Op(relax.add)(*, *) | Op(relax.add)(*, Op(relax.zeros)(*))) to R.add(A, B) matching OrPattern(Op(relax.add)(*, *) | Op(relax.add)(*, Op(relax.zeros)(*))) to C [22:36:29] /home/ruihang/Workspace/tvm/src/relax/ir/block_builder.cc:65: Warning: BlockBuilder destroyed with remaining blocks! Traceback (most recent call last): File "/home/ruihang/Workspace/tvm/tests/python/relax/test_dataflow_pattern.py", line 1949, in <module> test_backtrack_if_rewriter_returns_no_op() File "/home/ruihang/Workspace/tvm/tests/python/relax/test_dataflow_pattern.py", line 1945, in test_backtrack_if_rewriter_returns_no_op after = rewrite_call(pat, rewriter, before) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/ruihang/Workspace/tvm/python/tvm/relax/dpl/rewrite.py", line 59, in rewrite_call return ffi.rewrite_call(pattern, rewriter, func) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "tvm/_ffi/_cython/./packed_func.pxi", line 332, in tvm._ffi._cy3.core.PackedFuncBase.__call__ File "tvm/_ffi/_cython/./packed_func.pxi", line 263, in tvm._ffi._cy3.core.FuncCall File "tvm/_ffi/_cython/./packed_func.pxi", line 252, in tvm._ffi._cy3.core.FuncCall3 File "tvm/_ffi/_cython/./base.pxi", line 182, in tvm._ffi._cy3.core.CHECK_CALL File "/home/ruihang/Workspace/tvm/python/tvm/_ffi/base.py", line 481, in raise_last_ffi_error raise py_err File "tvm/_ffi/_cython/./packed_func.pxi", line 56, in tvm._ffi._cy3.core.tvm_callback File "/home/ruihang/Workspace/tvm/tests/python/relax/test_dataflow_pattern.py", line 1921, in rewriter assert isinstance(matches[pat], rx.Call) AssertionError ``` As you can see in the error message, with this PR, the pattern matcher mapped an `add` to a single Var. This breaks our assumption that what matched is always an `add`. Would you mind helping dig and fix this issue? Thanks in ahead. -- 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
