-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[aievec] to-llvm flow for i8xi8, i16xi16, bf16xbf16 elementwise multiplication #1139
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic job so far, James. Thank you! 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, you may want to split these into a bunch of separate patches, one per added intrinsic, for instance. Quicker turn-around, and it's easier to review.
It looks good to me, a lot of great work. Thanks, James!
std::stringstream ss; | ||
ss << "llvm.aie." << getVectorTypeString(resultType) << ".undef"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be replaced with an XLLVM intrinsic. In another patch, but this definitely needs clean-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will clean up this in the next PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} | ||
|
||
LogicalResult | ||
matchAndRewrite(aievec::MulElemOp op, OpAdaptor adaptor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamestcl-amd , @jsetoain : This function is not doing any checking on the number of vector lanes or alternatively the total number of bits of the operands or result. Looking at
mlir-aie/lib/Dialect/AIEVec/IR/AIEVecOps.cpp
Line 903 in 11822b0
ParseResult parseMulFMAElemOp(OpAsmParser &parser, OperationState &result, |
New updates before merging this PR:
|
This PR includes the e2e tests for i8xi8, i16xi16, bf16xbf16 elementwise multiplication going through the to-llvm flow.
Changes:
FoldAIECastOps
pattern inAIEVecToLLVM
pass. This pattern folds all theaievec.cast
ops that are introduced in the vector->aievec pass. Taking this shortcut, we do not need to conditionally introduceaievec.cast
ops in the vector->aievec conversion patterns.aievec.cast
op, and remove theFoldAIECastOps
pattern originally introduced in theAIEVecOptimizations
pass.