This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Vector] Fold transpose splat to splat with transposed type.
ClosedPublic

Authored by jacquesguan on Apr 14 2022, 1:35 AM.

Details

Summary

This revision folds transpose splat to a new splat with the transposed vector type. For a splat, there is no need to actually do transpose for it, it would be more effective to just build a new splat as the result, especially for a constant splat.

Diff Detail

Event Timeline

jacquesguan created this revision.Apr 14 2022, 1:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2022, 1:35 AM
jacquesguan requested review of this revision.Apr 14 2022, 1:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2022, 1:35 AM
ftynse requested changes to this revision.Apr 14 2022, 4:34 AM

Please add longer descriptions to commits that explain _why_ the change is being made. I understand that canonicalization patterns are generally useful for code simplification, but something must have prompted you to add this specific pattern.

mlir/lib/Dialect/Vector/IR/VectorOps.cpp
4408

The (result) type of the transpose op cannot be anything else than a vector, so just cast here. The code does not seem to handle the case where it is not a vector anyway...

This revision now requires changes to proceed.Apr 14 2022, 4:34 AM

address comment

jacquesguan edited the summary of this revision. (Show Details)Apr 14 2022, 5:07 AM

Please add longer descriptions to commits that explain _why_ the change is being made. I understand that canonicalization patterns are generally useful for code simplification, but something must have prompted you to add this specific pattern.

Thanks, I added the description to this revision.

mlir/lib/Dialect/Vector/IR/VectorOps.cpp
4408

Done.

ftynse added inline comments.Apr 14 2022, 5:09 AM
mlir/lib/Dialect/Vector/IR/VectorOps.cpp
4408

Use cast instead of dyn_cast, it asserts on mismatching types rather than returning null that may segfault. (https://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates)

ThomasRaoux requested changes to this revision.Apr 14 2022, 7:55 AM

The following patch just landed and should handle the constant case of the folding, can you remove it from the canonicalization?
https://reviews.llvm.org/D123595

This revision now requires changes to proceed.Apr 14 2022, 7:55 AM

address comment.

The following patch just landed and should handle the constant case of the folding, can you remove it from the canonicalization?
https://reviews.llvm.org/D123595

Done, thanks.

jacquesguan added inline comments.Apr 15 2022, 1:29 AM
mlir/lib/Dialect/Vector/IR/VectorOps.cpp
4408

Those code was removed cause of https://reviews.llvm.org/D123595 already implemented fold transpose constant splat.

ThomasRaoux accepted this revision.Apr 15 2022, 8:02 AM
ThomasRaoux added inline comments.
mlir/lib/Dialect/Vector/IR/VectorOps.cpp
4402–4407

nit: prefer early exit (https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code)
it could be:

auto splatOp =
            transposeOp.getVector().getDefiningOp<vector::SplatOp>()
if(!splatOp)
  return failure();
rewriter.replaceOpWithNewOp<vector::SplatOp>(
          transposeOp, transposeOp.getResultType(), splatOp.getInput());
return success();

Address comment.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 17 2022, 8:00 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.