[Codegen] Skinny mm/bmm/mv : use to vector distribute#21679
Conversation
newling
requested review from
Groverkss,
MaheshRavishankar,
kuhar and
qedawkins
as code owners
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
Seems like many tests are failing?
| const auto loopRanges = op.getStaticLoopRanges(); | ||
| const auto loopTypes = op.getIteratorTypesArray(); | ||
| const auto indexingMaps = op.getIndexingMapsArray(); |
There was a problem hiding this comment.
Please spell these out: https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
There was a problem hiding this comment.
Also, we probably don't need const on most of the local variables in this file, especially Types/Values/etc.
| // | ||
| // false is returned. These maps appear in convolution/pooling ops, which are | ||
| // not currently supported by the vector distribute pipeline. | ||
| for (const auto m : indexingMaps) { |
| } | ||
|
|
||
| // Check for non-unit reduction dim. | ||
| for (const auto [type, range] : llvm::zip(loopTypes, loopRanges)) { |
There was a problem hiding this comment.
Use zip_equal if these have the same length
Yeah I thought I'd found a good solution but some e2e tests didn't compile. Pipeline selection is quite a slippery path! Curious, what is your approach for testing before pushing a PR @kuhar and others? I generally just do And if that passes then I'll make a PR to check if there are other tests which fail in CI. Obvs if I am checking numerics I then compile an e2e test and run that locally on a GPU. But in general I assume that if lit tests pass, and numerics are fine. Ideally an e2e numerical test will never be the first place we see a compilation failure, ideally it should be caught in a lit test. Wondering if there's a way to get all numerical e2e tests just compile locally. |
I think you want to run e2e lit tests: https://iree.dev/developers/general/testing-guide/#code-coverage |
My go-to test command is: |
|
This isn't needed, the way to go is #21720 |
3 participants
Before this PR, use of vector distribute was gated on this ad hoc constraint:
For this PR I wanted to relax that gate to just
But that failed because there is a test
pooling_dynamicin nvvm_pipeline.mlirthat, when it goes down vector distribution, hits an assertion failure in
configure-tensor-layouts about one of the operands not having a projection
permutation.
I assume that's because layout inference scheme doesn't work well unless the maps
are just permutations/projections?
This PR works around this by gating on not having any non-projecting maps.