[Transform] mlir: named op layout propagation#101
Open
Conversation
yifeizh2
force-pushed
the
yifei/layout_propagation
branch
from
58f363e to
b3f5763
Compare
kurapov-peter
requested review from
LongshengDu,
Menooker and
kurapov-peter
and removed request for
LongshengDu,
Menooker and
kurapov-peter
yifeizh2
force-pushed
the
yifei/layout_propagation
branch
from
032c080 to
41cd05f
Compare
yifeizh2
force-pushed
the
yifei/layout_propagation
branch
from
ca0e2b6 to
4e7f7b9
Compare
yifeizh2
changed the title
yifeizh2
force-pushed
the
yifei/layout_propagation
branch
from
41e5251 to
7be3e15
Compare
yifeizh2
force-pushed
the
yifei/layout_propagation
branch
from
fe6f573 to
0087328
Compare
zhczhong
force-pushed
the
zhicong/deep_tile_matmul
branch
4 times, most recently
from
b3bf8dc to
23dfa97
Compare
yifeizh2
force-pushed
the
yifei/layout_propagation
branch
from
7f6defd to
1246465
Compare
zhczhong
force-pushed
the
zhicong/deep_tile_matmul
branch
3 times, most recently
from
21b4dfe to
5765bc7
Compare
yifeizh2
force-pushed
the
yifei/layout_propagation
branch
from
e249d77 to
49464e2
Compare
zhczhong
force-pushed
the
zhicong/deep_tile_matmul
branch
from
5765bc7 to
51527c0
Compare
yifeizh2
force-pushed
the
yifei/layout_propagation
branch
from
49464e2 to
6cc350f
Compare
This was referenced Sep 24, 2024
Merged
kurapov-peter
left a comment
kurapov-peter
left a comment
There was a problem hiding this comment.
I've put some comments. Haven't gone through all of it yet. A general question I have is how expensive the analysis is?
| gc_add_mlir_library(GcAnalysis | ||
| TargetDescriptionAnalysis.cpp | ||
| MatmulConfigAnalysis.cpp | ||
| GlobalAnalysis.cpp |
There was a problem hiding this comment.
Could you please give it a more descriptive name?
Comment on lines
+34
to
+37
| } else if (isa<tensor::ExpandShapeOp, tensor::CollapseShapeOp, tensor::PadOp>( | ||
| op)) | ||
| return true; | ||
| return false; |
There was a problem hiding this comment.
Suggested change
| } else if (isa<tensor::ExpandShapeOp, tensor::CollapseShapeOp, tensor::PadOp>( | |
| op)) | |
| return true; | |
| return false; | |
| } | |
| return isa<tensor::ExpandShapeOp, tensor::CollapseShapeOp, tensor::PadOp>(op); |
| return false; | ||
| } | ||
|
|
||
| bool hasAllTensorSemantics(linalg::LinalgOp linalgOp) { |
There was a problem hiding this comment.
I think this should be available in some utils already?
| ]; | ||
| } | ||
|
|
||
| def PostProcessPackUnpack : Pass<"post-process-pack-unpack"> { |
There was a problem hiding this comment.
Does it need to be a separate pass if it is a post-processing?
There was a problem hiding this comment.
I'd add tests with the smallest patterns possible just to capture the functionality for all passes and their aspects. They also help to understand what passes do.
Comment on lines
+221
to
+225
| // copied from mlir | ||
| static SmallVector<int64_t> | ||
| projectToInnerMostNonUnitDimsPos(ArrayRef<int64_t> dimsPos, | ||
| ArrayRef<ReassociationIndices> reassocIndices, | ||
| ArrayRef<int64_t> targetShape) { |
There was a problem hiding this comment.
Could we move it into utils in the upstream?
Comment on lines
+254
to
+256
| // if forceBlocking is set to true, we will unconditionally convert | ||
| // input/weight/output to blocking layout; otherwise we follow the default | ||
| // heuristic logic |
There was a problem hiding this comment.
Suggested change
| // if forceBlocking is set to true, we will unconditionally convert | |
| // input/weight/output to blocking layout; otherwise we follow the default | |
| // heuristic logic | |
| // if forceBlocking is set to true, we will unconditionally convert | |
| // input/weight/output to blocking layout; otherwise we follow the default | |
| // heuristic logic, which is ?... |
Comment on lines
+301
to
+317
| if (constantA || curInputLayouts[0].isBlocking() || (M % iim) || (K % iik) || | ||
| (elementType.isBF16() && | ||
| curInputLayouts[0] == TensorLayout({1, 0}, {}, {}))) { | ||
| ALayouts.emplace_back( | ||
| APackInfo.first, APackInfo.second, | ||
| SmallVector<OpFoldResult>{rewriter.getIndexAttr(iim), | ||
| rewriter.getIndexAttr(iik)}); | ||
| } else { | ||
| ALayouts.emplace_back(APackInfo.first, SmallVector<int64_t>{}, | ||
| SmallVector<OpFoldResult>{}); | ||
| } | ||
| if (constantB || curInputLayouts[1].isBlocking() || K % iik || N % iin || | ||
| elementType.isBF16()) { | ||
| BLayouts.emplace_back( | ||
| BPackInfo.first, BPackInfo.second, | ||
| SmallVector<OpFoldResult>{rewriter.getIndexAttr(iik), | ||
| rewriter.getIndexAttr(iin)}); |
There was a problem hiding this comment.
Can there be some abstraction here to avoid platform specificity?
Comment on lines
+216
to
+217
| } else if (isa<linalg::SoftmaxOp>(linalgOp) || isa<linalg::MapOp>(linalgOp) || | ||
| isa<linalg::YieldOp>(linalgOp) || isa<linalg::IndexOp>(linalgOp)) { |
Comment on lines
+521
to
+523
| if (failed(packVNNIMMT4D(rewriter, linalgOp))) | ||
| return failure(); | ||
| return success(); |
There was a problem hiding this comment.
Suggested change
| if (failed(packVNNIMMT4D(rewriter, linalgOp))) | |
| return failure(); | |
| return success(); | |
| return packVNNIMMT4D(rewriter, linalgOp); |
lmontigny
approved these changes
Oct 15, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add layout propagation related passes.
Tracking: #42