This is an archive of the discontinued LLVM Phabricator instance.

[mlir][VectorOps] Support vector transfer_read/write unrolling for memrefs with vector element type.
ClosedPublic

Authored by andydavis1 on Jan 17 2020, 3:54 PM.

Details

Summary

[mlir][VectorOps] Support vector transfer_read/write unrolling for memrefs with vector element type. When unrolling vector transfer read/write on memrefs with vector element type, the indices used to index the memref argument must be updated to reflect the unrolled operation. However, in the case of memrefs with vector element type, we need to be careful to only update the relevant memref indices.

For example, a vector transfer read with the following source/result types, memref<6x2x1xvector<2x4xf32>>, vector<2x1x2x4xf32>, should only update memref indices 1 and 2 during unrolling.

Diff Detail

Event Timeline

andydavis1 created this revision.Jan 17 2020, 3:54 PM
Herald added a project: Restricted Project. · View Herald Transcript

Unit tests: pass. 61909 tests passed, 0 failed and 782 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

I don't see a commit message?

andydavis1 edited the summary of this revision. (Show Details)Jan 22 2020, 2:01 PM

added commit message

aartbik requested changes to this revision.Jan 23 2020, 4:34 PM

Make sure to add the labels

[mlir] [VectorOps]

to the commit message's title

This revision now requires changes to proceed.Jan 23 2020, 4:34 PM
rriddle added inline comments.Jan 26 2020, 2:22 AM
mlir/lib/Dialect/VectorOps/VectorTransforms.cpp
462–463

While you are here can you change this to ///?

492

Remove trivial braces.

521

Use ///

527

nit: Why not use Optional<int> instead?

andydavis1 marked 4 inline comments as done.

addressing comments

addressing comments

Unit tests: pass. 62322 tests passed, 0 failed and 838 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

note that the title still needs "[mlir] [VectorOps]"

andydavis1 edited the summary of this revision. (Show Details)Jan 30 2020, 12:40 PM

Please address Aart's comment and land.

Thanks much Andy!

Would it be please possible to prefix all the differentials related to MLIR with [MLIR] prefix?
This makes reading through llvm-commits mails much less infuriating.
http://llvm.org/docs/DeveloperPolicy.html#commit-messages

* When the changes are restricted to a specific part of the code (e.g. a back-end or optimization pass),
  it is customary to add a tag to the beginning of the line in square brackets.
  For example, “[SCEV] …” or “[OpenMP] …”. This helps email filters and searches for post-commit reviews.

Would it be please possible to prefix all the differentials related to MLIR with [MLIR] prefix?
This makes reading through llvm-commits mails much less infuriating.
http://llvm.org/docs/DeveloperPolicy.html#commit-messages

* When the changes are restricted to a specific part of the code (e.g. a back-end or optimization pass),
  it is customary to add a tag to the beginning of the line in square brackets.
  For example, “[SCEV] …” or “[OpenMP] …”. This helps email filters and searches for post-commit reviews.

+1 @andydavis1 Can you please make sure to use the proper tags?

andydavis1 retitled this revision from Support vector transfer_read/write unrolling for memrefs with vector element type. to [mlir][VectorOps] Support vector transfer_read/write unrolling for memrefs with vector element type..Jan 31 2020, 3:10 PM

updated tags

aartbik accepted this revision.Jan 31 2020, 3:25 PM
This revision is now accepted and ready to land.Jan 31 2020, 3:25 PM

@lebedev.ri @rriddle if a non-enforced guideline makes it infuriating to interact with the system, maybe it is ripe for automation?

Unit tests: unknown.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

This revision was automatically updated to reflect the committed changes.