[Relax][BlockBuilder] Use PrimValue to provide tir_vars#17087
Closed
Lunderberg wants to merge 1 commit intoapache:mainfrom
Closed
[Relax][BlockBuilder] Use PrimValue to provide tir_vars#17087Lunderberg wants to merge 1 commit intoapache:mainfrom
Lunderberg wants to merge 1 commit intoapache:mainfrom
Conversation
Member
Lunderberg
force-pushed
the
relax_block_builder_prim_value
branch
from
b463cba to
080defd
Compare
Contributor
Author
|
Rebased onto main to avoid stale CI results. |
Prior to this commit, if a TIR variable was required to compute the output of `BlockBuilder.call_te`, but that TIR variable could not be inferred from the shape of any tensor arguments, it would be provided in an optional `tir_vars` argument to `R.call_tir`. In C++, this would be then be accessed as an optional `call->args[2].as<ShapeExprNode>()`. This extra argument can cause unexpected bugs. For example, `RewriteDataflowReshape` identifies the output buffer using `prim_func->buffer_map.Get(prim_func->params.back())`, which is only correct if `tir_vars` is empty. Rather than fixing these issues as they come up, it would be better to make the general Relax guarantees stronger by removing the `tir_vars` argument altogether. Use of extra `R.shape` parameter to specify additional `tir_vars` predates the existence of `relax::PrimValue`, and is no longer required. This commit updates `BlockBuilder.call_te` to use additional `relax.PrimValue` arguments to handle symbolic values that cannot be inferred from tensor shapes, rather than `tir_vars`.
Lunderberg
force-pushed
the
relax_block_builder_prim_value
branch
from
080defd to
95e018d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prior to this commit, if a TIR variable was required to compute the output of
BlockBuilder.call_te, but that TIR variable could not be inferred from the shape of any tensor arguments, it would be provided in an optionaltir_varsargument toR.call_tir. In C++, this would be then be accessed as an optionalcall->args[2].as<ShapeExprNode>().This extra argument can cause unexpected bugs. For example, the bug that was fixed in #17086 was caused by
RewriteDataflowReshapeidentifying the output buffer usingprim_func->buffer_map.Get(prim_func->params.back()), which is only correct iftir_varsis empty. Rather than fixing these issues as they come up, it would be better to make the general Relax guarantees stronger by removing thetir_varsargument altogether.Use of extra
R.shapeparameter to specify additionaltir_varspredates the existence ofrelax::PrimValue, and is no longer required. This commit updatesBlockBuilder.call_teto use additionalrelax.PrimValuearguments to handle symbolic values that cannot be inferred from tensor shapes, rather thantir_vars.