-
Notifications
You must be signed in to change notification settings - Fork 641
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
[Codegen][CPU] Eliminate all-true vector masks after vectorization #18190
Conversation
3e1fe9a
to
6dea91f
Compare
This enables an upstream transform that eliminates all true `vector.create_mask` ops. This is particularly beneficial for scalable vectors, which use dynamic tensor types, which results in masks that otherwise would not fold away till much later, preventing some optimizations. Depends on #99314. Signed-off-by: Benjamin Maxwell <benjamin.maxwell@arm.com>
6dea91f
to
9f25223
Compare
Abbreviated Benchmark Summary@ commit 06255e62aec4827e77b30d6aed67f1f158154310 (vs. base c71fe1a7aa8a1c360e6418043b2a1165861d72f9) Data-Tiling Comparison TableClick to show
No improved or regressed benchmarks 🏖️ No improved or regressed compilation metrics 🏖️ For more information: |
Thanks! LG, but a test would be nice :) |
28e0d74
to
09525ba
Compare
Done 👍 |
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.
LG, just one nit about cleanup.
/// Converts from iree_compiler::VscaleRange to vector::VscaleRange. | ||
static std::optional<vector::VscaleRange> | ||
toVectorVscaleRange(std::optional<iree_compiler::VscaleRange> vscaleRange) { | ||
if (!vscaleRange.has_value()) | ||
return std::nullopt; | ||
return vector::VscaleRange{vscaleRange->min, vscaleRange->max}; | ||
} |
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.
I was confused that why can't we just use vector::VscaleRagnge
, then I found that it's added to upstream recently (llvm/llvm-project@9b06e25). Can you help remove the IREE version, and switch to use upstream version? It can be done in a follow-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.
I'll do this in a follow up 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.
also LGTM cheers
compiler/src/iree/compiler/Codegen/Common/test/generic_vectorization.mlir
Outdated
Show resolved
Hide resolved
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, thanks!
Signed-off-by: Benjamin Maxwell <benjamin.maxwell@arm.com>
Quick follow-up to iree-org#18190, as I realized the test may also pass if the conv fails to vectorize.
Quick follow-up to iree-org#18190, as I realized the test may also pass if the conv fails to vectorize. Signed-off-by: Benjamin Maxwell <benjamin.maxwell@arm.com>
Quick follow-up to iree-org#18190, as I realized the test may also pass if the conv fails to vectorize. Signed-off-by: Benjamin Maxwell <benjamin.maxwell@arm.com>
…#18218) Follow up to #18190 (comment) Signed-off-by: Benjamin Maxwell <benjamin.maxwell@arm.com>
Quick follow-up to #18190, as I realized the test may also pass if the conv fails to vectorize. Signed-off-by: Benjamin Maxwell <benjamin.maxwell@arm.com>
This enables an upstream transform that eliminates all true
vector.create_mask
ops. This is particularly beneficial for scalable vectors, which use dynamic tensor types, which results in masks that otherwise would not fold away till much later, preventing some optimizations.Depends on llvm/llvm-project#99314.