[Mlir-commits] [mlir] [mlir][tensor] Loosen restrictions on folding dynamic reshapes (PR #137963)
Ian Wood
llvmlistbot at llvm.org
Sun May 11 17:44:50 PDT 2025
https://github.com/IanWood1 requested changes to this pull request.
I think there is a bug here. For example:
```cpp
EXPECT_EQ(getReassociationIndicesForCollapse(
{ShapedType::kDynamic, 10, 10, 10, ShapedType::kDynamic},
{ShapedType::kDynamic, 10, ShapedType::kDynamic}),
std::nullopt);
```
This fails with the output `({ { 0 }, { 1 }, { 2, 3, 4 } })` (I think `std::nullopt` is correct result here)
Other examples:
```cpp
EXPECT_EQ(getReassociationIndicesForCollapse(
{ShapedType::kDynamic, 3, 4, 3, ShapedType::kDynamic},
{ShapedType::kDynamic, 12, ShapedType::kDynamic}),
std::nullopt);
EXPECT_EQ(getReassociationIndicesForCollapse(
{ShapedType::kDynamic, 8, 4, 2, 16, ShapedType::kDynamic},
{ShapedType::kDynamic, 32, ShapedType::kDynamic}),
std::nullopt);
EXPECT_EQ(getReassociationIndicesForCollapse(
{ShapedType::kDynamic, 2, 2, 2, ShapedType::kDynamic},
{ShapedType::kDynamic, 2, 2, ShapedType::kDynamic}),
std::nullopt);
```
I'm trying to think of a generalized rule for when it becomes ambiguous. One possible way might be to check against a reversed target/source shape:
```
getReassociationIndicesForCollapse(source, target) ==
reverseReassociation(
getReassociationIndicesForCollapse(reverse(source), reverse(target)))
```
The idea is that the current algorithm is "anti-greedy" and will try go minimize the size of the reassociations towards the beginning. So reversing the ordering will compare anti-greedy with greedy. The results will be the same if there is non-ambiguous reassociation.
https://github.com/llvm/llvm-project/pull/137963
More information about the Mlir-commits
mailing list