[mlir][Vector] Improve support for 0-d vectors in vector dialect lowerings

[Groverkss]

A number of patterns in vector dialect lowerings predate 0-d vector support and side step from 0-d vectors by converting them to scalars and then broadcasting them. An example patch fixing this: #112907

Some example issues:

• https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp has patterns that convert vector.load/vector.store on 0-d vectors to memref.load/memref.store on scalars:

  llvm-project/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp

  Line 524 in 0a3347d

  struct VectorLoadToMemrefLoadLowering

  .

I have tested removing these, and the llvm ir generated from llvm.store with 0-d vectors is correct. (Good first issue)

• vector.multi_reduction (https://mlir.llvm.org/docs/Dialects/Vector/#vectormulti_reduction-vectormultidimreductionop) requires the result to be a scalar when all dimensions are reduced. This causes additional corner case handling for scalars in vector.multi_reduction patterns. A better solution would be to allow 0-d vectors as result of vector.multi_reduction. (Not a good first issue, requires more understanding of vector dialect)

• Merge AnyVector and AnyVectorOfAnyRank type constraints

  llvm-project/mlir/include/mlir/IR/CommonTypeConstraints.td

  Line 661 in 721b796

  // Temporary vector type clone that allows gradual transition to 0-D vectors.

  (needs work before this can be done)

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Kunwar Grover (Groverkss)

A number of patterns in vector dialect lowerings predate 0-d vector support and side step from 0-d vectors by converting them to scalars and then broadcasting them. An example patch fixing this: https://github.com//pull/112907

Some example issues:

• https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp has patterns that convert vector.load/vector.store on 0-d vectors to memref.load/memref.store on scalars:

  llvm-project/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp

  Line 524 in 0a3347d

  struct VectorLoadToMemrefLoadLowering

  . I have tested removing these, and the llvm ir generated from llvm.store with 0-d vectors is correct.

• vector.multi_reduction (https://mlir.llvm.org/docs/Dialects/Vector/#vectormulti_reduction-vectormultidimreductionop) requires the result to be a scalar when all dimensions are reduced. This causes additional corner case handling for scalars in vector.multi_reduction patterns. A better solution would be to allow 0-d vectors as result of vector.multi_reduction.

[harrisonGPU]

Hi @Groverkss, do you mean a draft like this? #112937, to merge AnyVector and AnyVectorOfAnyRank type constraints?

[Groverkss]

Hi @Groverkss, do you mean a draft like this? #112937, to merge AnyVector and AnyVectorOfAnyRank type constraints?

Probably not. It would involve supporting 0-d vectors natively for most vector dialect operations and then merging them. My understanding is that these two type constraints exist because 0-d vector support wasn't plumbed through properly.

I think an easier, and more useful patch would be to rename them: AnyVector -> AnyNonZeroRankVector, AnyVectorOfAnyRank -> AnyVector . The current naming is very confusing, and this would be a good first step.

The next step would probably involve supporting 0-d vectors in vector.multi_reduction, vector.contract and then other ops.

[harrisonGPU]

Hi @Groverkss, do you mean a draft like this? #112937, to merge AnyVector and AnyVectorOfAnyRank type constraints?

Probably not. It would involve supporting 0-d vectors natively for most vector dialect operations and then merging them. My understanding is that these two type constraints exist because 0-d vector support wasn't plumbed through properly.

I think an easier, and more useful patch would be to rename them: AnyVector -> AnyNonZeroRankVector, AnyVectorOfAnyRank -> AnyVector . The current naming is very confusing, and this would be a good first step.

The next step would probably involve supporting 0-d vectors in vector.multi_reduction, vector.contract and then other ops.

Thanks, I will consiconsider it! :)

[meltq]

Since this issue hasn't gotten much activity in 2 months, I've begun working on it here #121454