llvm / llvm/torch-mlir

Bug in lowering of AtenView -> Tensor::Expand_Shape

Open
#2,008 2 comments 0 reactions 1 assignee Claimed by @Abhishek-Varma View on GitHub
Dominant language
C++
Stars
1.9k
Forks
736
Avg merge
5d 22h
Merged PRs (30d)
15

Description

Input MLIR :-
```
func.func @view(%arg0: !torch.vtensor<[64,64],f16>) -> !torch.vtensor<[1,4096,1],f16>
{
%int1 = torch.constant.int 1
%int-1 = torch.constant.int -1
%shape = torch.prim.ListConstruct %int1, %int-1, %int1 : (!torch.int, !torch.int, !torch.int) -> !torch.list
%output = torch.aten.view %arg0, %shape : !torch.vtensor<[64,64],f16>, !torch.list -> !torch.vtensor<[1,4096,1],f16>
return %output : !torch.vtensor<[1,4096,1],f16>
}
```

On passing it through `--convert-torch-to-linalg`, following error is thrown :-
```
error: 'tensor.expand_shape' op expected reassociation map #0 of same rank as expanded memref(3), but got 2
note: see current operation: %expanded = "tensor.expand_shape"(%collapsed) {reassociation = [[0, 1]]} : (tensor<4096xf16>) -> tensor<1x4096x1xf16>
```

With a fix I have the correct IR gets generated : `%expanded = tensor.expand_shape %collapsed [[0, 1, 2]] : tensor<4096xf16> into tensor<1x4096x1xf16>` (Observe the `reassociation` map here)

But turns out that the current AtenViewOp's implementation seems to be reinventing the wheel for deciphering `ReassociationIndices` - I'll have to go through it a bit to make a generic patch.

This is needed to for [ToMe's Support](https://github.com/llvm/torch-mlir/issues/1989) - I believe I should anyway work on raising a patch for the fix.

After addressing the above, I saw two instances where negative dimensions were being an issue in the LLVM pipeline :-
1. `DimOfReifyRankedShapedTypeOpInterface`.
2. `FoldDimOfExpandShape`.

I've patched them up as well, but two things here :-
1. Should LLVM passes take care of Python's negative dimension cases? I believe that shouldn't be the case even though temporarily I've patched things up at `externals/llvm-project`. Let me know your thoughts here, I'd accordingly raise patch for `llvm-project` separately.
2. Ideally all the negative dimension indexing should be normalized. Is there any such pass currently in `torch-mlir` that's supposed to take care of this? If there isn't, I believe it's worth investing an effort on this front.

@powderluv @ramiro050

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.