-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add concat fusions #2460
Add concat fusions #2460
Conversation
This build is not recommended to merge 🔴 |
🔴distilgpt2_fp16: FAILED: MIGraphX is not within tolerance - check verbose output |
src/fuse_concat.cpp
Outdated
/* | ||
* The MIT License (MIT) | ||
* | ||
* Copyright (c) 2015-2022 Advanced Micro Devices, Inc. All rights reserved. |
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.
Is the old license stamper script being run? The date should be 2023
src/include/migraphx/fuse_concat.hpp
Outdated
/* | ||
* The MIT License (MIT) | ||
* | ||
* Copyright (c) 2015-2022 Advanced Micro Devices, Inc. All rights reserved. |
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.
same as the other file
test/fuse_concat.cpp
Outdated
/* | ||
* The MIT License (MIT) | ||
* | ||
* Copyright (c) 2015-2022 Advanced Micro Devices, Inc. All rights reserved. |
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.
same as the other file
/* | ||
* The MIT License (MIT) | ||
* | ||
* Copyright (c) 2015-2022 Advanced Micro Devices, Inc. All rights reserved. |
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.
same as the other file
@@ -167,6 +168,8 @@ std::vector<pass> target::get_passes(migraphx::context& gctx, const compile_opti | |||
dead_code_elimination{}, | |||
enable_pass(not enabled(MIGRAPHX_DISABLE_REDUCE_FUSION{}), fuse_reduce{}), | |||
dead_code_elimination{}, | |||
fuse_concat{}, |
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.
Asking the question here more out of curiosity. I assume order doesn't matter here for fuse_pointwise vs fuse_concat? Is there a benefit to swapping order ever around the fuse_reduce?
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.
It needs to run after fuse_pointwise
otherwise there will be no pointwise modules to fuse with.
Is there a benefit to swapping order ever around the fuse_reduce?
I dont think so. In either case we will have two kernels that need to be run if these two passes overlap.
Fix CI but otherwise I get the idea. |
This will fuse the inputs to concat if they are pointwise operators.