[TIR] Require buffer declaration (DeclBuffer) before use #14778

Lunderberg · 2023-05-05T16:49:42Z

Part of RFC#70, requiring buffers to be declared prior to their usage. This is currently a draft PR, to see what errors are found by the CI.

tvm-bot · 2023-05-05T16:49:45Z

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

cc @Hzfengsy, @junrushao, @quic-sanirudh, @shingjan _{See #10317 for details}

_{Generated by tvm-bot}

junrushao · 2023-05-06T06:17:47Z

Super excited about this feature!

Lunderberg · 2023-05-16T21:23:37Z

@tvm-bot rerun

github-actions · 2023-05-16T21:23:58Z

Failed to re-run CI in https://github.com/apache/tvm/actions/runs/4996801890

Traceback (most recent call last):
  File "/home/runner/work/tvm/tvm/ci/scripts/jenkins/git_utils.py", line 121, in _request
    with request.urlopen(req, data) as response:
  File "/usr/lib/python3.8/urllib/request.py", line 222, in urlopen
    return opener.open(url, data, timeout)
  File "/usr/lib/python3.8/urllib/request.py", line 531, in open
    response = meth(req, response)
  File "/usr/lib/python3.8/urllib/request.py", line 640, in http_response
    response = self.parent.error(
  File "/usr/lib/python3.8/urllib/request.py", line 569, in error
    return self._call_chain(*args)
  File "/usr/lib/python3.8/urllib/request.py", line 502, in _call_chain
    result = func(*args)
  File "/usr/lib/python3.8/urllib/request.py", line 649, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 403: Forbidden

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "ci/scripts/github/github_tvmbot.py", line 594, in comment_failure
    raise item
  File "ci/scripts/github/github_tvmbot.py", line 706, in run
    pr.rerun_github_actions()
  File "ci/scripts/github/github_tvmbot.py", line 583, in rerun_github_actions
    raise e
  File "ci/scripts/github/github_tvmbot.py", line 575, in rerun_github_actions
    actions_github.post(f"actions/runs/{workflow_id}/rerun-failed-jobs", data={})
  File "/home/runner/work/tvm/tvm/ci/scripts/jenkins/git_utils.py", line 143, in post
    return self._request(self.base + url, data, method="POST")
  File "/home/runner/work/tvm/tvm/ci/scripts/jenkins/git_utils.py", line 126, in _request
    raise RuntimeError(f"Error response: {msg}\n{error_data}")
RuntimeError: Error response: HTTP Error 403: Forbidden
{"message":"This workflow is already running","documentation_url":"https://docs.github.com/rest/reference/actions#re-run-workflow-failed-jobs"}

Prior to this commit, any trivial let binding of `var1 = var2` is inlined. However, buffer definitions are not updated, so this can result in dangling `tir::Var` instances. This commit updates the `tir.Simplify` pass to keep trivial let bindings if they are used as part of a buffer definition. Ideally, the trivial `LetStmt` variable would be inlined into the buffer definition as well as other expressions. However, because a buffer may be implicitly declared, the first usage may be within a constrained context. If that happens, the simplified shape/strides expression cannot be used to update the buffer definition, as that simplification is not valid at all possible usage points of the buffer. ```python for i in range(n): elem_offset = i view = T.Buffer(1, data=buf, elem_offset = elem_offset) if i == 0: # First occurrence in TIR is here, where elem_offset would # simplify to zero. view[0] = 1 else: # But the same buffer is used here, where elem_offset doesn't # simplify to zero. view[0] = 2 ``` This will be resolvable after apache#14778 lands, requiring all buffers to be declared with `DeclBuffer` prior to usage. ```python for i in range(n): elem_offset = i # All variables used by the DeclBuffer are valid across the entire # body of the DeclBuffer. view = T.decl_buffer(1, data=buf, elem_offset = elem_offset) if i == 0: view[0] = 1 else: view[0] = 2 ```

…14951) Prior to this commit, any trivial let binding of `var1 = var2` is inlined. However, buffer definitions are not updated, so this can result in dangling `tir::Var` instances. This commit updates the `tir.Simplify` pass to keep trivial let bindings if they are used as part of a buffer definition. Ideally, the trivial `LetStmt` variable would be inlined into the buffer definition as well as other expressions. However, because a buffer may be implicitly declared, the first usage may be within a constrained context. If that happens, the simplified shape/strides expression cannot be used to update the buffer definition, as that simplification is not valid at all possible usage points of the buffer. ```python for i in range(n): elem_offset = i view = T.Buffer(1, data=buf, elem_offset = elem_offset) if i == 0: # First occurrence in TIR is here, where elem_offset would # simplify to zero. view[0] = 1 else: # But the same buffer is used here, where elem_offset doesn't # simplify to zero. view[0] = 2 ``` This will be resolvable after #14778 lands, requiring all buffers to be declared with `DeclBuffer` prior to usage. ```python for i in range(n): elem_offset = i # All variables used by the DeclBuffer are valid across the entire # body of the DeclBuffer. view = T.decl_buffer(1, data=buf, elem_offset = elem_offset) if i == 0: view[0] = 1 else: view[0] = 2 ```

Part of changes being split out from apache#14778 into independent portions. This commit allows DeclBuffer to occur in the lowered TIR passed to CodeGenSPIRV.

Part of changes being split out from apache#14778 into independent portions. This commit allows DeclBuffer to occur in the lowered TIR passed to CodeGenStackVM.

Part of changes being split out from apache#14778 into independent portions. This commit allows TIR `cache_read` and `cache_write` schedule primitives to preserve `DeclBuffer` nodes.

Part of changes being split out from apache#14778 into independent portions. This commit allows TIR `compute_inline`, `compute_at`, and `reverse_compute_at` schedule primitives to preserve `DeclBuffer` nodes.

Preserve DeclBuffer node when transforming with `LowerCustomDatatypes` This is a subset of changes, being split out from apache#14778 into independent portions.

This is a subset of changes, being split out from apache#14778 into independent portions.

Previously, `PoolAllocationToOffsetConverter` did not remap buffer objects occurring in `DeclBuffer` nodes. This commit updates `PoolAllocationToOffsetConverter` to handle `DeclBuffer` nodes. This is a subset of changes, being split out from apache#14778 into independent portions.

Previously, each bound constant resulted in a `AllocateConst` node. Now, each constant results in a `AllocateConst` node containing a `DeclBuffer` node.

This updates the older `tvm.tir.ir_builder.IRBuilder`, not the newer one that parses TVMScript. To maintain backwards compatibility with the older IRBuilder, the `ib.allocate` method also inserts a `DeclBuffer`. This is a subset of the changes made in apache#14778, broken out for ease of testing and review.

Implemented as part of VerifyWellFormed. Currently, fails after FlattenStorage.

Since it is no longer valid to omit the `DeclBuffer`, these test cases are no longer required.

Prior to this commit, the functions in `python/tvm/relay/backend/contrib/ethosu` tracked buffers based on `tir.Var`, typically determined from `buffer_load.buffer.data`. This commit updates these funcitons to instead track based on `tir.Buffer`, now determined as `buffer_load.buffer`. This change allows for tracking of buffer objects, to determine which `DeclBuffer` statements should be removed.

Previously, the `StorageRewrite` would hoist a single `DeclBuffer` node that was directly underneath a hoisted `Allocate` node. This commit updates `StorageRewrite` to hoist `DeclBuffer` nodes belonging to each allocation begin merged, and to merge the buffers themselves if several identical allocations are being merged.

In MergeConstants, hoist all `DeclBuffer` instances that use a merged allocation.

* [TIR] Output DeclBuffer in SplitHostDevice If the generated device function uses a buffer, generate a DeclBuffer for the buffer at the top of the device function. This is a subset of the changes made in #14778, broken out for ease of testing and review. * Updated thread sync test to account for DeclBuffer * Updated LowerWarp unit tests to find Allocate in PrimFunc

For any local buffers allocated as part of `LowerThreadAllreduce`, generate a `DeclBuffer`. This is a subset of the changes made in apache#14778, broken out for ease of testing and review.

For any local buffers allocated as part of `LowerThreadAllreduce`, generate a `DeclBuffer`. This is a subset of the changes made in #14778, broken out for ease of testing and review.

If a flattened buffer is produced for use in `BufferLoad` and `BufferStore` statements, generate a `DeclBuffer`. This is a subset of the changes made in apache#14778, broken out for ease of testing and review.

When producing a flattened buffer for use in `BufferLoad` and `BufferStore` nodes, generate a `DeclBuffer` for the flattened buffer. This is a subset of the changes made in apache#14778, broken out for ease of testing and review.

If a flattened buffer is produced for use in `BufferLoad` and `BufferStore` statements, generate a `DeclBuffer`. This is a subset of the changes made in apache#14778, broken out for ease of testing and review.

When producing a flattened buffer for use in `BufferLoad` and `BufferStore` nodes, generate a `DeclBuffer` for the flattened buffer. This is a subset of the changes made in apache#14778, broken out for ease of testing and review.

If a flattened buffer is produced for use in `BufferLoad` and `BufferStore` statements, generate a `DeclBuffer`. This is a subset of the changes made in apache#14778, broken out for ease of testing and review.

When producing a flattened buffer for use in `BufferLoad` and `BufferStore` nodes, generate a `DeclBuffer` for the flattened buffer. This is a subset of the changes made in apache#14778, broken out for ease of testing and review.

Lunderberg mentioned this pull request May 5, 2023

[TIR] Handle implicit buffers in PrimFunc specialization #14565

Closed

Lunderberg marked this pull request as ready for review May 9, 2023 20:59

Lunderberg changed the title ~~[Draft][TIR] Require buffer declaration (DeclBuffer) before use~~ [TIR] Require buffer declaration (DeclBuffer) before use May 9, 2023

Lunderberg mentioned this pull request May 15, 2023

[TVMScript] Print Executor, Runtime, and FunctionInfo as metadata #14840

Merged

Lunderberg mentioned this pull request May 25, 2023

[TIR] Keep trivial LetStmt in tir.Simplify when used in buffer decl #14951

Merged

Lunderberg force-pushed the require_tir_buffer_declaration branch from f8e0b4d to 72ed1d4 Compare June 6, 2023 15:36

Lunderberg mentioned this pull request Jun 6, 2023

[Vulkan] Allow DeclBuffer in CodeGenSPIRV #15035

Merged

Lunderberg mentioned this pull request Jun 6, 2023

[StackVM] Updated CodeGenStackVM to handle DeclBuffer #15036

Merged

Lunderberg mentioned this pull request Jun 6, 2023

[TIR] Handle DeclBuffer in CacheReadWrite schedule primitive #15037

Merged

This was referenced Jun 6, 2023

[TIR] Handle DeclBuffer in Inline/ComputeAt/ReverseComputeAt #15038

Merged

[TIR] Preserve object equality in Buffer::GetFlattenedBuffer #15040

Merged

Lunderberg mentioned this pull request Jun 6, 2023

[TIR] Handle DeclBuffer in LowerCustomDatatypes #15041

Merged

Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 6, 2023

[TIR] Handle DeclBuffer in StorageFlatten's input

dbe5249

This is a subset of changes, being split out from apache#14778 into independent portions.

Lunderberg mentioned this pull request Jun 6, 2023

[TIR] Handle DeclBuffer in StorageFlatten's input #15042

Merged

Lunderberg mentioned this pull request Jun 6, 2023

[TIR][USMP] Preserve DeclBuffer in PoolAllocationToOffsetConverter #15044

Merged

Lunderberg added 16 commits August 15, 2023 11:18

[TIR] Updated BindParams to output DeclBuffer

3df4379

Previously, each bound constant resulted in a `AllocateConst` node. Now, each constant results in a `AllocateConst` node containing a `DeclBuffer` node.

Update test_lower_build.py tests for new decl_buffer behavior

acfc2a7

Updated coproc sync tests to skip past DeclBuffer

786d207

Updated ir_builder unit test to expect DeclBuffer

2f733c5

unit tests, te schedule

032834e

unit tests, te hybrid script

29f7416

unit tests, te build lower

61423c4

Updated test case for TE lower

c96bef1

unit tests, lift attr scope

388015e

[TIR] Require Buffer definition before use

b463202

Implemented as part of VerifyWellFormed. Currently, fails after FlattenStorage.

Remove test of TIR schedule primitives with implicit buffers

a02ddc4

Since it is no longer valid to omit the `DeclBuffer`, these test cases are no longer required.

[EthosU] Update lowering passes to handle DeclBuffer

b4d5188

In MergeConstants, hoist all `DeclBuffer` instances that use a merged allocation.

Update EthosU unit tests to expect DeclBuffer nodes

6b4a4e0

Lunderberg force-pushed the require_tir_buffer_declaration branch from 8b1a87f to 6b4a4e0 Compare August 15, 2023 16:18

Lunderberg mentioned this pull request Aug 30, 2023

[TIR] Output DeclBuffer in LowerThreadAllreduce #15646

Merged

Lunderberg mentioned this pull request Sep 15, 2023

[TIR] Output DeclBuffer in FlattenBuffer #15757

Open

Lunderberg mentioned this pull request Jan 16, 2024

[Unity][Transform] Replace eligible operators with in-place versions in dataflow blocks #16129

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TIR] Require buffer declaration (DeclBuffer) before use #14778

[TIR] Require buffer declaration (DeclBuffer) before use #14778

Lunderberg commented May 5, 2023

tvm-bot commented May 5, 2023 •

edited

junrushao commented May 6, 2023

Lunderberg commented May 16, 2023

github-actions bot commented May 16, 2023

[TIR] Require buffer declaration (DeclBuffer) before use #14778

Are you sure you want to change the base?

[TIR] Require buffer declaration (DeclBuffer) before use #14778

Conversation

Lunderberg commented May 5, 2023

tvm-bot commented May 5, 2023 • edited

junrushao commented May 6, 2023

Lunderberg commented May 16, 2023

github-actions bot commented May 16, 2023

tvm-bot commented May 5, 2023 •

edited