search

apache / tvm

Open deep learning compiler stack for cpu, gpu and specialized accelerators
https://tvm.apache.org/
Apache License 2.0
11.44k stars 3.41k forks source link

[Bug][TOPI][Relay] TOPI should never depend on Relay #13604

Open vvchernov opened 1 year ago

vvchernov commented 1 year ago

The issue origin is the problem described in the PR. During solution it became clear that topi and relay depend on each other. I've gotten the name of the issue from @junrushao statement: "Ideally, TOPI should never depend on Relay in the first place". Intuitively it looks correct. If you know any issues, docs, discussions which approve this TVM design, please reference on it here. Nevertheless there are many imports of Relay from TOPI (see list below). But the majority of scripts use TIR and TE only (relay.Expr and different relay ops are used for op description in comments for the sake of clarity). Possibly it is a concept to use relay in topi for simplifying (and it should be separated from topi somehow for correct design) or somebody used it once and it became bad practice.

I do not think that it is small problem. If Junru Shao is right, it is problem in TVM design. It means that the list should be refactored step by step and relay using on topi side should be prohibited.

Expected behavior

TOPI should not depend on Relay

Actual behavior

There are many python scripts from tvm/topi/ where relay is imported explicitly: adreno/conv2d_alter_op.py armc_cpu/bitserial_conv2d.py armc_cpu/conv2d_alter_op.py armc_cpu/qnn_alter_op.py bifrost/conv2d.py cuda/conv2d_alter_op.py cuda/conv3d_alter_op.py cuda/sparse.py cuda/tensorcore_alter_op.py generic/conv2d.py hexagon/conv2d_alter_op.py hexagon/dense_alter_op.py intel_graphics/conv2d_alter_op.py mali/conv2d.py nn/conv2d_transpose.py nn/conv3d_transpose.py sparse/utils.py x86/conv2d_alter_op.py x86/dense_alter_op.py x86/math_alter_op.py

Possibly there are more scripts which imports relay implicitly through other imports. I did not check it.

Environment

It is environment-independent issue

Steps to reproduce

Details of particular problem reproducing can be seen in PR

Triage

I add meta-scheduler as tag due to the particular issue appeared there. Possibly it is not directly related to this issue but it clarifies context and interactions between TOPI and Relay

cc @junrushao @masahi @elvin-n @jwfromm

junrushao commented 1 year ago

To share some more context: the reason that most of those files are depending on relay at the moment is that they are expected to be used by a pass "AlterOpLayout", and thus they are engineered in a way less compatible with our underlying assumption