Skip to content

Conversation

@Lunderberg
Copy link
Contributor

Prior to this commit, the shape argument to R.reshape was required to either be an in-line relax::ShapeExpr, or a variable that had been bound to a relax::ShapeExpr within the current function. As a result, shapes that were provided as function arguments or that were produced by another operation (e.g. R.tensor_to_shape) would unnecessarily trigger an error.

This commit updates the VMBuiltinLower pass to instead check that the argument has relax::ShapeStructInfo.

Closes #17217

Prior to this commit, the `shape` argument to `R.reshape` was required
to either be an in-line `relax::ShapeExpr`, or a variable that had
been bound to a `relax::ShapeExpr` within the current function.  As a
result, shapes that were provided as function arguments or that were
produced by another operation (e.g. `R.tensor_to_shape`) would
unnecessarily trigger an error.

This commit updates the `VMBuiltinLower` pass to instead check that
the argument has `relax::ShapeStructInfo`.

Closes apache#17217
@Lunderberg Lunderberg force-pushed the relax_dynamic_reshape_argument branch from 053edcb to 0ea564e Compare August 26, 2024 13:08
@Lunderberg
Copy link
Contributor Author

Thank you for the review, and rebased onto main to resolve conflicts.

@Lunderberg
Copy link
Contributor Author

Lunderberg commented Sep 3, 2024

@tvm-bot rerun

Rerunnning since the CI results were a week old, and will merge into main after CI.

@Lunderberg Lunderberg merged commit 5627357 into apache:main Sep 4, 2024
@Lunderberg Lunderberg deleted the relax_dynamic_reshape_argument branch September 4, 2024 15:51
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.

[Bug] [Relax] VMBuiltinLower expects bound value to be a ShapeExpr

2 participants