[Transform][Vector] Lower operation to tile (virtual) vector #252
Conversation
BRUCE11111
requested review from
zhczhong,
LongshengDu,
ZhennanQin and
yifeizh2
| #define SAFE_EXPAND(X) X | ||
| #define LDBG(X) LLVM_DEBUG(DBGS() << SAFE_EXPAND(X) << "\n") | ||
|
|
||
| #define IMPLEMENTED_MATMUL \ |
There was a problem hiding this comment.
If we need to skip all matmul/conv ops, maybe we can check ContractionOpInterfaceand ConvolutionOpInterface or use the helper provide in: https://github.com/intel/graph-compiler/pull/187/files#r1714656991.
There was a problem hiding this comment.
Once we migrate to generic matmuls.
There was a problem hiding this comment.
Okay, I will add it! Thanks for advice~
| if (isCollapseOp) { | ||
| reshapeHelper.srcVectorizedShape.emplace_back(ss); | ||
| } | ||
|
|
There was a problem hiding this comment.
Unnecessary white spaces, add comments or squash them if self explanatory.
| << concatOp << "\n"); | ||
| } | ||
| // check input operand shape type | ||
| if (not llvm::all_of(concatOp.getOperandTypes(), [](Type x) { |
There was a problem hiding this comment.
Check any_of shape is dynamic maybe faster than all_of must be static?
There was a problem hiding this comment.
Sure~ Will change it to anyof!
| }; | ||
| } // namespace | ||
|
|
||
| std::unique_ptr<Pass> createLowerTileVectorPass() { |
There was a problem hiding this comment.
I thought this will be auto generated by .td?
There was a problem hiding this comment.
createLowerToTileVectorPass()
There was a problem hiding this comment.
Please check the names for this pass and unify them.
There was a problem hiding this comment.
Yes, it's redundant. Need to remove it.
| struct ReshapeVectorizeHelper { | ||
| /// The transfer_read operation read result. We calculate this shape based on | ||
| /// the specified input vector size. | ||
| SmallVector<int64_t> srcVectorizedShape; |
There was a problem hiding this comment.
The srcVectorizedShape and srcShape seems unused and please remove them if they are useless
There was a problem hiding this comment.
Actually in getReshapeOperationVectorizeShape function is using this struct to avoid pass too many parameters. All the data is required by it.
| SmallVector<int64_t> associateIndices; | ||
|
|
||
| for (const Attribute &attr : reshapeOp.getReassociation()) { | ||
| llvm::transform( |
There was a problem hiding this comment.
Better to remove the bracket here following https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements. Same for other if/for
Part of issue148.
This is just a pass that lower operation to vector. It is just a simple part of the physical register pass.
We separate this PR to facilitate further testing and running of other PRs.