Skip to content
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.

Sign up for GitHub

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

Use mlir_mode::fast for dot operations on Navi and reduce k threshold to 1024 #2608

Closed
wants to merge 4 commits into from
Closed

Use mlir_mode::fast for dot operations on Navi and reduce k threshold to 1024 #2608

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e69a3a
Use fast_mode and k<1024
turneram Jan 5, 2024
905bc99
Fix merge error
turneram Jan 5, 2024
9ae1a4d
Formatting
turneram Jan 5, 2024
820f131
Fix merge mistake
turneram Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/targets/gpu/fuse_mlir.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ auto is_mlir_dot(mlir_mode mode)
// auto m = a.lens()[a.lens().size() - 2];
// auto n = b.lens().back();
auto k = a.lens().back();
// Skipping GEMMs with a K dimension greater than 2048 is a course-grained strategy
// Skipping GEMMs with a K dimension greater than 1024 is a course-grained strategy
// to avoid poor-performing GEMM kernels from MLIR
// To-do: Investigate a more precise strategy
return k <= 2048;
return k < 1024;
});
}

Expand Down Expand Up @@ -389,7 +389,7 @@ MIGRAPHX_DECLARE_ENV_VAR(MIGRAPHX_MLIR_USE_SPECIFIC_OPS);

bool is_requested(std::string_view option, bool fallback = false)
{
auto string_value = string_value_of(MIGRAPHX_MLIR_USE_SPECIFIC_OPS{}, "");
auto string_value = string_value_of(MIGRAPHX_MLIR_USE_SPECIFIC_OPS{}, "");
if(string_value.empty())
return fallback;
const auto options = split_string(string_value, ',');
Expand All @@ -416,14 +416,15 @@ void fuse_mlir::apply(module_pass_manager& mpm) const
mlir_mode mode =
(enabled(MIGRAPHX_ENABLE_EXTRA_MLIR{}) or enable_extra) ? mlir_mode::fast : mlir_mode::none;

match::find_matches(mpm,
find_mlir_fused_ops{.conv_mode = get_mode("fused", mlir_mode::fast),
.dot_mode = get_mode("fused", mode)});
match::find_matches(
mpm,
find_mlir_fused_ops{.conv_mode = get_mode("fused", mlir_mode::fast),
.dot_mode = is_navi ? mlir_mode::fast : get_mode("fused", mode)});

match::find_matches(
mpm,
find_mlir_standalone_convolution_op{get_mode("convolution", mlir_mode::int8)},
find_mlir_standalone_dot_op{get_mode("dot", mlir_mode::none)});
find_mlir_standalone_convolution_op{get_mode("convolution", mlir_mode::fast)},
find_mlir_standalone_dot_op{is_navi ? mlir_mode::fast : get_mode("dot", mlir_mode::none)});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be get_mode("dot", mode) so we use MLIR for dot fusions when using the MIGRAPHX_ENABLE_EXTRA_MLIR setting.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry thats not the case. This is standalone.

#else
(void)mpm;
#endif
Expand Down
Loading