This is an archive of the discontinued LLVM Phabricator instance.

[mlir][linalg] Vectorize linalg.pad_tensor consumed by transfer_write
ClosedPublic

Authored by springerm on May 25 2021, 10:03 PM.

Details

Summary

Vectorize linalg.pad_tensor without generating a linalg.init_tensor when consumed by a transfer_write.

Depends On D103780

Diff Detail

Event Timeline

springerm created this revision.May 25 2021, 10:03 PM
springerm requested review of this revision.May 25 2021, 10:03 PM

minor change

Please add some tests.

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
825

Missing {in_bounds = [true, true]} in the doc.

858

Note: there is probably a similar mlir::makeComposedAffineApply to be written for AffineMinOps which could reduce reliance on CSE in many cases, by construction.

863

Why not just minOp1->getOperands() == minOp2->getOperands() (or .equals, I forget) ?

springerm updated this revision to Diff 348876.May 31 2021, 9:56 PM
springerm marked 2 inline comments as done.
  • add tests
  • handle more cases (dyn/static dims) in pad_tensor + transfer_write pattern
springerm updated this revision to Diff 349449.Jun 2 2021, 9:11 PM

split into 3 smaller patterns with benefit

springerm updated this revision to Diff 349798.Jun 4 2021, 2:50 AM

use benefit as discussed

This CL feels like it's doing too much at once, can we please split the generic pattern from the 3 inittensor-less patterns ?

mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
898

More comments please.

/// These patterns are meant to apply in complementary fashion.
/// We use benefits and pattern ordering to encode the complementary case disjunction in a simpler way.
/// To handle the ordering without scattered magic constants, these patterns must be added with the following API.
mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
691

Can all this logic be replaced by:

if (isa<ConstantOp>(block.getTerminator()->getOperand(0))
  return the attribute?
return null;

?

I'd expect can then decide on the caller side if you need to clone the constant?

springerm retitled this revision from [mlir] Vectorize linalg.pad_tensor without init_tensor to [mlir][linalg] Vectorize linalg.pad_tensor without init_tensor.Jun 4 2021, 4:20 AM
springerm edited the summary of this revision. (Show Details)
springerm marked 2 inline comments as done.
springerm added inline comments.
mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
691

"Constant" means "constant within the block" here, i.e., defined outside of the block. Extended the comment to make it more clear.

springerm updated this revision to Diff 349849.Jun 4 2021, 6:03 AM
springerm marked an inline comment as done.

simplify

springerm retitled this revision from [mlir][linalg] Vectorize linalg.pad_tensor without init_tensor to [mlir][linalg] Vectorize linalg.pad_tensor consumed by transfer_write/subtensor.Jun 4 2021, 9:13 PM
springerm edited the summary of this revision. (Show Details)
springerm updated this revision to Diff 350043.Jun 5 2021, 2:32 AM

small change

springerm updated this revision to Diff 350144.Jun 6 2021, 5:40 PM

small change

springerm retitled this revision from [mlir][linalg] Vectorize linalg.pad_tensor consumed by transfer_write/subtensor to [mlir][linalg] Vectorize linalg.pad_tensor consumed by transfer_write.Jun 6 2021, 6:00 PM
springerm edited the summary of this revision. (Show Details)
springerm updated this revision to Diff 350475.Jun 7 2021, 8:00 PM

small improvements

springerm updated this revision to Diff 350551.Jun 8 2021, 2:49 AM

no change

nicolasvasilache accepted this revision.Jun 11 2021, 5:00 AM
This revision is now accepted and ready to land.Jun 11 2021, 5:00 AM
This revision was landed with ongoing or failed builds.Jun 13 2021, 6:24 PM
This revision was automatically updated to reflect the committed changes.