Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] TVMScript fails with LetStmt #13892

Closed
liangW-intellif opened this issue Feb 1, 2023 · 3 comments
Closed

[Bug] TVMScript fails with LetStmt #13892

liangW-intellif opened this issue Feb 1, 2023 · 3 comments
Labels
needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it type: bug

Comments

@liangW-intellif
Copy link
Contributor

Hi there,
I encountered an error that when generating TVMScript with LetStmt, the generated script cannot be parsed if the var in LetStmt appears the first time in the IR.

import tvm
from tvm.script import tir as T
from tvm import te


def test_let_roundtrip():
    x = te.var("x")
    y = te.var("y")
    let0 = tvm.tir.LetStmt(x, 0, tvm.tir.Evaluate(0))
    let1 = tvm.tir.LetStmt(y, 1, tvm.tir.Evaluate(0))
    body = tvm.tir.SeqStmt([let0, let1])
    func = tvm.tir.PrimFunc([], body)
    print(func.script())
    # Parse fail
    roundtrip = tvm.script.from_source(func.script())
    tvm.ir.assert_structural_equal(func, roundtrip, True)

The above case generates TVMScript as below

@T.prim_func
def main():
    with T.let(x, 0):
        T.evaluate(0)
    y: T.int32 = 1
    T.evaluate(0)

But when parsing this TVMScript, it reports an error that 'x' is an undefined variable so that the parse failed. The TVMScript with LetStmt is not round-trippable in this case.

test_let_roundtrip.py::test_let_roundtrip # from tvm.script import tir as T

@T.prim_func
def main():
    with T.let(x, 0):
        T.evaluate(0)
    y: T.int32 = 1
    T.evaluate(0)
error: Undefined variable: x
 --> <str>:5:16
   |  
 5 |      with T.let(x, 0):
   |                 ^     
FAILED

For the semantic of LetStmt, I think'x' should be identified as a definition, the script should be parsed successly.

Please let me know if there is any confusion, thanks very much!

Environment

latest TVM mainline

@junrushao @wrongtest-intellif

@liangW-intellif liangW-intellif added needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it type: bug labels Feb 1, 2023
@junrushao
Copy link
Member

CC @cyx-6 would you like to take a look? Thanks!

@cyx-6
Copy link
Contributor

cyx-6 commented Feb 1, 2023

Thanks for reporting this bug. I believe that this is due to the bug of printing LetStmt. Since we forget to print the x = T.var("int32") before the with T.let(x, 0):. This bug will be fixed in the #13900. And more details are in the description of that PR too. Hope this will solve your issue.

junrushao pushed a commit that referenced this issue Feb 3, 2023
This PR is the bug fix reported in #13892. Initially, we mix the logic of `LetStmt` docsifying method with and without concise scoping. For example, in
```python
x = T.var("int32")
with T.let(x, 0):
```
`x` in the `LetStmt` works as a right value, while in
```python
x: T.int32 = 0
```
`x` in the `LetStmt` works as a left value as result.
Our old logic mixed them together to generate the wrong code for the first case.
Meanwhile, during the fix, we found another bug in concise scoping check. For example, we have
```python
x = T.var("int32")
y = T.var("int32")
with T.let(x, y):
  with T.let(y, 0):
```
here we should not output
```python
x = T.var("int32")
y = T.var("int32")
with T.let(x, y):
  y: int32 = 0
```
becase this will define a new `y_1: int32 = 0` indeed, due the the variable shadowing logic of the parser, which is different from the `y` we define and refer to.
Our concise scoping `v: ... = ...` should launch if and only if the `v` is never defined before.
Otherwise, we use `with T.let(v, ...):` instead.
@junrushao
Copy link
Member

Closed via #13900

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it type: bug
Projects
None yet
Development

No branches or pull requests

3 participants