I believe we are all aware the RFC is to support general IRBuilder-based
metaprogramming, and with this context, I would love to address your concerns
as below.
> there is no way to handle different parser rules based on context
Our design handles context-dependent parsing in a unified approach - both TIR
and Relax are context-dependent, and therefore, it is a hard requirement for
any parser.
Here is an example of context-dependent parsing in TIR:
```python
@T.prim_func
def f(a: T.handle):
A = T.match_buffer(a, shape=(128, 128), dtype="float32")
^^^^^^^^^^^^^^
# `match_buffer` here interprets buffer's data pointer to tvm.tir.Buffer
@T.prim_func
def f(...):
# An example from tensorization:
#
https://github.com/apache/tvm/blob/c2ec95616ed52e8377c42c5f3e15621841924059/tests/python/unittest/test_tir_schedule_tensorize.py#L199-L203
with T.block(...):
A_sub = T.match_buffer(A[vi * 16 : vi * 16 + 16, vk * 16 : vk * 16 + 16],
shape=(16, 16))
^^^^^^^^^^^^^^
# `match_buffer` here corresponds to `MatchBufferRegion` in TIR
# Link:
https://github.com/apache/tvm/blob/c2ec95616ed52e8377c42c5f3e15621841924059/include/tvm/tir/stmt.h#L1215-L1216
```
> The example you give for a single class is one that has all static members,
> but what if instead it was just a regular class. It would instantiate a
> single instance of said class to do parsing.
> Instantiating a class would also give the benefit of being able to maintain
> some state while parsing which is not possible with either free functions or
> a class with static methods.
Thanks for bringing up the case where some state needs maintaining during
parsing, and to be clear, that’s the exactly what an IRBuilder is capable of
handling.
For example, considering the IRBuilder program below:
```python
def example():
with Builder():
with T.block("block"):
T.block_attr({"some_attr": some_value})
^^^^^^^^^^^^
`block_attr` depends on the `T.Block` above
```
To handle this, like every IRBuilder, the IRBuilder proposed here keeps a stack
of contexts as its internal state so that information could be stored there.
The proposed parser, as a thin wrapper of the IRBuilder, does not store any
extra context-dependent data other than symbol table and those already stored
in the IRBuilder. As an example,
```python
@T.prim_func
def example():
...
with T.block("block"):
^^^^^^^
calls IRBuilder via `T.block`
T.block_attr({"some_attr": some_value})
^^^^^^^^^^^^
calls IRBuilder via `T.block_attr`
```
> If we want to consider future dialects, this also has the added benefit of
> being able to add dialects by subclassing one of the existing parsers.
Agreed that we need to consider future dialects and glad we are on the same
page about this. In our RFC, IRBuilder is IR-agnostic and extensible to
multiple dialects. Contextual information is stored in the IRBuilder so that
metaprogramming with IRBuilder is possible.
> self.tir_parser = TIRParser()
> self.tir_parser.parse(stmt)
The problem of this design is that
- To support with IRBuilder-based metaprogramming, extra work with similar
logic has to be done in the IRBuilder, which is less desirable and could cause
divergence between parser and IRBuilder;
- Here is an assumption that there is a big `RelaxParser` which knows there is
one and only one language “TIR” exists, which is less desirable, as you
mentioned, when we want to “add dialects”. Hope this
[snippet](https://github.com/tlc-pack/relax/blob/25cb42133130fb8ec75d2cc021c6c347e99f0b28/python/tvm/script/parser.py#L1372-L1391)
that comes from the exact commit you provided reveals how many underlying
assumptions there are with this approach.
It would be great if you could refer to Section [Parsing-time
evaluation](https://github.com/yelite/tvm-rfcs/blob/tvmscript-metaprogramming/rfcs/0079-tvmscript-metaprogramming.md#parse-time-evaluation)
to understand how the new parser is a thin wrap of IRBuilder.
--
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/79#issuecomment-1203374706
You are receiving this because you are subscribed to this thread.
Message ID: <apache/tvm-rfcs/pull/79/c1203374...@github.com>
