Re: [PR] [Relax] Allow composition of DFPattern replacements [tvm]

["MasterJH5574 (via GitHub)" <gi...@apache.org>]

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-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org