areusch commented on code in PR #63: URL: https://github.com/apache/tvm-rfcs/pull/63#discussion_r846518316
########## rfcs/0063-clarifying-buffer-declaration-and-access.md: ########## @@ -84,8 +92,9 @@ imposes a cost for buffer aliasing both to compile times and development complex **Discussion: When it is safe to transform a buffer** We would like to discuss some examples of when it is safe to transform a buffer w.r.t. aliasing rules: - -(1) reshape, (2) layout transform (e.g. swap indices), (3) compact. +- (1) reshape Review Comment: i think proper numbered list syntax is like ``` 1. reshape 2. layout transform ... 3. compact ``` ########## rfcs/0063-clarifying-buffer-declaration-and-access.md: ########## @@ -0,0 +1,232 @@ +- Feature Name: Clarifying Buffer Declaration and Access +- Author: Wuwei Lin (@vinx13), Eric Lunderberg (@Lunderberg) +- Start Date: 2022-03-18 +- RFC PR: [apache/tvm-rfcs#63](https://github.com/apache/tvm-rfcs/pull/63) +- GitHub Issue: [apache/tvm#10505](https://github.com/apache/tvm/issues/10505) + +# Summary +[summary]: #summary + +In https://github.com/apache/tvm/pull/9727 and +[RFC#39](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0039-buffer-physical-layout.md), we +deprecated `Load` and `Store` to use `BufferLoad` and `BufferStore` instead in order to support +generalized multi-dimensional physical buffer access. Here we document necessary clarifications, +implications about the new buffer convention, as well as the post-hoc pass checklist. + +# Motivation +[motivation]: #motivation + +The main goal of this RFC is to summarize the existing buffer convention and the IR changes in +https://github.com/apache/tvm/pull/9727 which have a broader impact. There are no new semantics +proposed in this RFC. + +# Reference - level explanation +[reference-level-explanation]: #reference-level-explanation + +**What’s a buffer?** + +Buffer is a compile-time object that groups runtime objects(data pointer and shape variables) +structurally. A Buffer needs to be declared and allocated before it can be used. + +**Declaration of buffer** + +Buffer can be declared in the following ways: + +- Buffer map of `PrimFunc`. This specifies how opaque parameters of type `T.handle` should be +interpreted as a buffer inside the `PrimFunc`. `T.handle` represents an opaque pointer (`void *`). +- `T.alloc_buffer` is used `S-TIR` to create and allocate a buffer. +- `T.buffer_decl` can be used to create a buffer alias by specifying the underlying data variable to +reuse the data from another buffer. It can also be used to reinterpret the data type of the buffer. +`T.buffer_decl` can also be used to create a buffer alias with a different `elem_offset`. +`elem_offset` should be handled during the lowering process. + +Examples of `T.buffer_decl` is shown below. +``` [email protected]_func +def buffer_alias(A: T.Buffer[(16,), "float"]): + A_vector = T.buffer_decl([4], "float32x4", data=A.data) + [email protected]_func +def buffer_alloc(): + A = T.buffer_decl([4, 4], "float32") + Allocate(A.data, [16], "float32") +``` + +**Allocation of buffer** + +In low-level TIR, `Allocate` is used to allocate a data variable with given shapes. `Allocate` +doesn’t operate on the buffer-level. The result of `Allocate` is a data variable, which may be +reinterpreted with a different shape or data type in buffer declaration. + +**Explicit `DeclBuffer` IR construct** + +`T.buffer_decl` is not an explicit statement in TIR - There is no such node in TIR. +The current behavior of `TVMScriptPrinter` is to implicitly print a `T.buffer_decl` at the beginning +of `PrimFunc` for any undefined buffers. The implicit behavior can be error-prone. In light of the +migration, we should consider an explicit `DeclBuffer` as part of the IR. This will be furthur +discussed in a separate RFC. + +**Buffer Aliasing** + +`T.buffer_decl` creates a buffer alias if the underlying data variable (`.data` field) overlaps with +another buffer. Buffer created via `alloc_buffer` always do not alias. Buffer aliases do not need +`Allocate` to create the data variable. Each data variable can be allocated via `Allocate` once. Review Comment: ok, can you update this in the doc? you could simply state "Buffer aliases do not need `Allocate` to create the data variable--they may simply reuse the data variable from the Buffer being aliased." ########## rfcs/0063-clarifying-buffer-declaration-and-access.md: ########## @@ -0,0 +1,232 @@ +- Feature Name: Clarifying Buffer Declaration and Access +- Author: Wuwei Lin (@vinx13), Eric Lunderberg (@Lunderberg) +- Start Date: 2022-03-18 +- RFC PR: [apache/tvm-rfcs#63](https://github.com/apache/tvm-rfcs/pull/63) +- GitHub Issue: [apache/tvm#10505](https://github.com/apache/tvm/issues/10505) + +# Summary +[summary]: #summary + +In https://github.com/apache/tvm/pull/9727 and +[RFC#39](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0039-buffer-physical-layout.md), we +deprecated `Load` and `Store` to use `BufferLoad` and `BufferStore` instead in order to support +generalized multi-dimensional physical buffer access. Here we document necessary clarifications, +implications about the new buffer convention, as well as the post-hoc pass checklist. + +# Motivation +[motivation]: #motivation + +The main goal of this RFC is to summarize the existing buffer convention and the IR changes in +https://github.com/apache/tvm/pull/9727 which have a broader impact. There are no new semantics +proposed in this RFC. + +# Reference - level explanation +[reference-level-explanation]: #reference-level-explanation + +**What’s a buffer?** + +Buffer is a compile-time object that groups runtime objects(data pointer and shape variables) +structurally. A Buffer needs to be declared and allocated before it can be used. + +**Declaration of buffer** + +Buffer can be declared in the following ways: + +- Buffer map of `PrimFunc`. This specifies how opaque parameters of type `T.handle` should be +interpreted as a buffer inside the `PrimFunc`. `T.handle` represents an opaque pointer (`void *`). +- `T.alloc_buffer` is used `S-TIR` to create and allocate a buffer. +- `T.buffer_decl` can be used to create a buffer alias by specifying the underlying data variable to Review Comment: ok, if you're using `T.` notation here, you mean TVMScript, right? in which case, can you update all occurrences to match what you would grep for if using TVMScript? right now it's a dead-end. ########## rfcs/0063-clarifying-buffer-declaration-and-access.md: ########## @@ -0,0 +1,232 @@ +- Feature Name: Clarifying Buffer Declaration and Access +- Author: Wuwei Lin (@vinx13), Eric Lunderberg (@Lunderberg) +- Start Date: 2022-03-18 +- RFC PR: [apache/tvm-rfcs#63](https://github.com/apache/tvm-rfcs/pull/63) +- GitHub Issue: [apache/tvm#10505](https://github.com/apache/tvm/issues/10505) + +# Summary +[summary]: #summary + +In https://github.com/apache/tvm/pull/9727 and +[RFC#39](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0039-buffer-physical-layout.md), we +deprecated `Load` and `Store` to use `BufferLoad` and `BufferStore` instead in order to support +generalized multi-dimensional physical buffer access. Here we document necessary clarifications, +implications about the new buffer convention, as well as the post-hoc pass checklist. + +# Motivation +[motivation]: #motivation + +The main goal of this RFC is to summarize the existing buffer convention and the IR changes in +https://github.com/apache/tvm/pull/9727 which have a broader impact. There are no new semantics +proposed in this RFC. + +# Reference - level explanation +[reference-level-explanation]: #reference-level-explanation + +**What’s a buffer?** + +Buffer is a compile-time object that groups runtime objects(data pointer and shape variables) +structurally. A Buffer needs to be declared and allocated before it can be used. + +**Declaration of buffer** + +Buffer can be declared in the following ways: + +- Buffer map of `PrimFunc`. This specifies how opaque parameters of type `T.handle` should be +interpreted as a buffer inside the `PrimFunc`. `T.handle` represents an opaque pointer (`void *`). +- `T.alloc_buffer` is used `S-TIR` to create and allocate a buffer. +- `T.buffer_decl` can be used to create a buffer alias by specifying the underlying data variable to +reuse the data from another buffer. It can also be used to reinterpret the data type of the buffer. +`T.buffer_decl` can also be used to create a buffer alias with a different `elem_offset`. +`elem_offset` should be handled during the lowering process. + +Examples of `T.buffer_decl` is shown below. +``` [email protected]_func +def buffer_alias(A: T.Buffer[(16,), "float"]): + A_vector = T.buffer_decl([4], "float32x4", data=A.data) + [email protected]_func +def buffer_alloc(): + A = T.buffer_decl([4, 4], "float32") + Allocate(A.data, [16], "float32") +``` + +**Allocation of buffer** + +In low-level TIR, `Allocate` is used to allocate a data variable with given shapes. `Allocate` +doesn’t operate on the buffer-level. The result of `Allocate` is a data variable, which may be +reinterpreted with a different shape or data type in buffer declaration. + +**Explicit `DeclBuffer` IR construct** + +`T.buffer_decl` is not an explicit statement in TIR - There is no such node in TIR. +The current behavior of `TVMScriptPrinter` is to implicitly print a `T.buffer_decl` at the beginning +of `PrimFunc` for any undefined buffers. The implicit behavior can be error-prone. In light of the +migration, we should consider an explicit `DeclBuffer` as part of the IR. This will be furthur +discussed in a separate RFC. + +**Buffer Aliasing** + +`T.buffer_decl` creates a buffer alias if the underlying data variable (`.data` field) overlaps with +another buffer. Buffer created via `alloc_buffer` always do not alias. Buffer aliases do not need +`Allocate` to create the data variable. Each data variable can be allocated via `Allocate` once. +If a transformation would produce multiple allocations of the same buffer var (e.g. unrolling a loop +that contains an allocation), the transform should update the allocations to be unique using +`tvm::tir::ConvertSSA` + +Buffers should not alias each other unless necessary, because aliased buffers increase complexity +for TIR transformations. Passes that rewrite buffers should clearly indicate how aliased buffers +are handled. For example, when changing the underlying layout of stored elements in a buffer, all +buffer aliases must also be updated. While buffer aliasing is typically free at runtime, this +imposes a cost for buffer aliasing both to compile times and development complexity. + +**Discussion: When it is safe to transform a buffer** + +We would like to discuss some examples of when it is safe to transform a buffer w.r.t. aliasing rules: + +(1) reshape, (2) layout transform (e.g. swap indices), (3) compact. + +(1) is fine under aliasing as long as the low level memory is shared. (2) and (3) would need more Review Comment: yeah that makes sense, can you update the RFC? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
