-
Notifications
You must be signed in to change notification settings - Fork 4k
[Fix][Relax] Validate non-concat axes before fusing parallel matmul biases #20243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,19 +117,26 @@ Patterns CreatePatterns(const BranchInfo& branch_info) { | |
| /*! \brief Create a rewriter for the given parallel matmul branches. */ | ||
| ffi::TypedFunction<ffi::Map<Var, Expr>(ffi::Map<DFPattern, Var>, ffi::Map<Var, Expr>)> GetRewriter( | ||
| const Patterns& patterns, const BranchInfo& branch_info, FCheck check) { | ||
| auto batch_dims_compatible = [](size_t rhs_dim, const std::vector<size_t>& indices, | ||
| const std::vector<ffi::Array<PrimExpr>>& rhs_shapes) { | ||
| arith::Analyzer ana; | ||
| for (auto ind : indices) { | ||
| TVM_FFI_ICHECK_EQ(static_cast<int>(rhs_shapes[ind].size()), rhs_dim); | ||
| // -2 for reduction and concat axes | ||
| for (size_t i = 0; i < rhs_dim - 2; ++i) { | ||
| if (!ana->CanProve(rhs_shapes[indices[0]][i] == rhs_shapes[ind][i])) { | ||
| return false; | ||
| auto shapes_compatible_excluding_trailing_axes = | ||
| [](const std::vector<ffi::Array<PrimExpr>>& shapes, size_t num_trailing_axes_excluded) { | ||
| arith::Analyzer ana; | ||
| size_t ndim = shapes[0].size(); | ||
| for (const auto& shape : shapes) { | ||
| TVM_FFI_ICHECK_EQ(shape.size(), ndim); | ||
| for (size_t i = 0; i < ndim - num_trailing_axes_excluded; ++i) { | ||
| if (!ana->CanProve(shapes[0][i] == shape[i])) { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return true; | ||
| return true; | ||
| }; | ||
| auto batch_dims_compatible = [&](const std::vector<size_t>& indices, | ||
| const std::vector<ffi::Array<PrimExpr>>& rhs_shapes) { | ||
| std::vector<ffi::Array<PrimExpr>> selected; | ||
| selected.reserve(indices.size()); | ||
| for (size_t ind : indices) selected.push_back(rhs_shapes[ind]); | ||
| return shapes_compatible_excluding_trailing_axes(selected, 2); | ||
| }; | ||
|
|
||
| return [=](ffi::Map<DFPattern, Var> matchings, ffi::Map<Var, Expr> bindings) { | ||
|
|
@@ -145,7 +152,7 @@ ffi::TypedFunction<ffi::Map<Var, Expr>(ffi::Map<DFPattern, Var>, ffi::Map<Var, E | |
| ffi::Map<Var, Expr> replacements; | ||
|
|
||
| for (const auto& [rhs_dim, indices] : GroupShapes(rhs_shapes)) { | ||
| if (indices.size() == 1 || !batch_dims_compatible(rhs_dim, indices, rhs_shapes)) continue; | ||
| if (indices.size() == 1 || !batch_dims_compatible(indices, rhs_shapes)) continue; | ||
|
|
||
| auto lhs = matchings[patterns.input]; | ||
|
|
||
|
|
@@ -210,6 +217,37 @@ ffi::TypedFunction<ffi::Map<Var, Expr>(ffi::Map<DFPattern, Var>, ffi::Map<Var, E | |
| continue; | ||
| } | ||
|
|
||
| if (branch_info.bias_dim) { | ||
| std::vector<ffi::Array<PrimExpr>> bias_shapes; | ||
| bool bias_shape_unknown = false; | ||
| for (const auto& bias_var : bias) { | ||
| auto bias_shape_opt = GetTensorType(bias_var)->GetShape(); | ||
| if (!bias_shape_opt) { | ||
| bias_shape_unknown = true; | ||
| break; | ||
| } | ||
| bias_shapes.push_back(bias_shape_opt.value()); | ||
| } | ||
| if (bias_shape_unknown) { | ||
| return ffi::Map<Var, Expr>{}; | ||
| } | ||
| if (!shapes_compatible_excluding_trailing_axes(bias_shapes, 1)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also require each bias’s last dimension to match the corresponding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, I added the check and a new regression test in a c67ceca. |
||
| continue; | ||
| } | ||
| arith::Analyzer ana; | ||
| bool bias_widths_match = true; | ||
| for (size_t i = 0; i < splits.size(); ++i) { | ||
| const auto& shape = bias_shapes[i]; | ||
| if (!ana->CanProve(shape[shape.size() - 1] == splits[i].split_size)) { | ||
| bias_widths_match = false; | ||
| break; | ||
| } | ||
| } | ||
| if (!bias_widths_match) { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| auto concat_rhs = concat(Tuple(rhs), rhs_dim - 1); | ||
| auto matmul_combined = matmul(lhs, concat_rhs, splits[0].out_dtype); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please guard against ranks smaller than
num_trailing_axes_excludedhere. Scalar biases are valid broadcast operands, andstd::optional<int>{0}still enters the bias-fusion branch. For two matmul branches with scalar biases,ndimis 0 and this call excludes one trailing axis, sondim - num_trailing_axes_excludedunderflows as asize_tand the loop accesses a nonexistent dimension. The latershape[shape.size() - 1]check has the same issue.Could this helper return
falsewhenndim < num_trailing_axes_excluded(and preferably treat rank mismatches as incompatible rather thanICHECK), with a regression test using two scalar biases? Such branches should skip bias fusion because rank-0 tensors have no concat axis.