-
Notifications
You must be signed in to change notification settings - Fork 646
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
Fuse Generic Ops Generated by gather
Lowering
#17341
Conversation
Abbreviated Benchmark Summary@ commit 14d5d644869c3413c979f1f7e5bff3a96558d7f8 (vs. base 428adf2fe6c3318ad1a6f8c6ae945f700802323e) Data-Tiling Comparison TableClick to show
Regressed Latencies 🚩
Improved Latencies 🎉
[Top 3 out of 4 results showed] No improved or regressed compilation metrics 🏖️ For more information: |
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
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.
Looks mostly OK, just one comment on using isDequantizationOp
method.
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
@MaheshRavishankar, I added the check for the number of uses because |
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.
Leaving some comments. Land this when all the tests pass.
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/test/fusion_preprocessing.mlir
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/test/fusion_preprocessing.mlir
Outdated
Show resolved
Hide resolved
I changed matchAndRewrite to match against
|
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.
few style nits
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
@hanhanW I think we should incorporate @benvanik 's comment llvm/torch-mlir#3277 (comment) |
Yes, but that is not something we can control. This needs to be fixed higher up in the stack then. |
Signed-off-by: Lubo Litchev <lubol@google.com>
#17226 (comment)