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

[RFC] TIR Language Specification #101

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

slyubomirsky
Copy link

@slyubomirsky slyubomirsky commented Jun 1, 2023

Rendered view.

This RFC proposes including a language specification (draft included) for TIR in TVM's documentation.

Many thanks to all who reviewed and commented on earlier drafts (see the acknowledgements in the RFC text), especially @junrushao, @vinx13, and @Lunderberg, who were my primary guides to TIR during the initial drafting process.

@slyubomirsky
Copy link
Author

slyubomirsky commented Jun 15, 2023

Requesting reviews from some in the community who are knowledgeable about TIR (including those who assisted in the draft): @Hzfengsy @tqchen @junrushao @Lunderberg @vinx13 @cyx-6 @MasterJH5574

I would also welcome suggestions on other TIR contributors whose attention should be drawn to this RFC.

Perhaps it might make sense to set up a channel on the TVM Discord to allow for some real-time discussion.

@slyubomirsky
Copy link
Author

@kparzysz-quic @yzh119 Perhaps you might also be interested in reviewing (feel free to tag others as well).

@slyubomirsky
Copy link
Author

I've added an update for recently added inter-PrimFunc calls, please have a look!

@@ -522,7 +528,7 @@ Unlike `PrimExprs`, statements do not return values. Instead, they operate by mo
* If `t->strides` is not null, then `len(t->strides)` must match `len(b->strides)` or else an error is raised. For all `i` from 0 to `len(b->strides) - 1`, `b->strides[i]` must be an `IntImm` whose value matches `t->strides[i]` (or else an error is raised), an already bound `Var` whose bound value is `t->strides[i]` (or else an error is raised), or an unbound `Var` (in which case, it is bound with the value `t->strides[i]`).
* `len(t->shape)` and `len(b->shape)` must match or else an error is raised. For all `i` from 0 to `len(b->shape) - 1`, `b->shape[i]` must be an `IntImm` whose value matches `t->shape[i]` (or else an error is raised), an already bound `Var` whose bound value is `t->shape[i]` (or else an error is raised), or an unbound `Var` (in which case, it is bound with the value `t->shape[i]`).
5. One further condition that `PrimFunc`s expect of their `DLTensor` arguments: No two `DLTensor` arguments are permitted to alias each other.
6. Next, `body` is executed. The `PrimFunc` produces outputs by mutating values in buffers passed as the inputs; these changes can be observed by the caller via the `DLTensor` representations passed in step i.
6. Next, `body` is executed. The `PrimFunc` produces outputs by mutating values in buffers passed as the inputs; these changes can be observed by the caller via the `DLTensor` representations passed in step i. If a `tir.ret` builtin was executed, the `PrimFunc` returns the value that was passed to `tir.ret`; otherwise, the `PrimFunc` returns a void value. Upon returning (whether after encountering a call to `tir.ret` or the end of `body`), the current scope is popped and all values allocated within the `PrimFunc` are freed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it, I'm not sure on the "all values allocated within the PrimFunc are freed" is true.

If there is an Allocate node, it gets lowered to a matched pair of to the PackedFuncs registered as "tir.TVMBackendAllocWorkspace" and "tir.TVMBackendFreeWorkspace". If the body of the Allocate node contains an explicit return through T.ret, or an implicit return through an Assert node, then the free wouldn't occur.

Though, that's probably best considered a bug, and not intended behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you don't think it's the intended behavior, I think we should call that a bug then. Happy to hear others' thoughts.

edit: Alternatively, this can be a case of builtins not following the rules of the rest of the language. I can add a clause pointing to the existence of such complexities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants