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.

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

#15824 Workaround LLK issue in max_pool #16849

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pavlejosipovic
Copy link
Contributor

@pavlejosipovic pavlejosipovic commented Jan 17, 2025

pack_untilize_dst is causing issues on GS and WH
in case where reduce op is called after max_pool.

Workaround is to use default value for block_ct_dim in pack_untilize_dst_init_short in max_pool kernel.

This doesn't affect correctness of max_pool op
and resolves the issue on subsequent reduce op.

Perf looks good on model device perf tests.

Ticket

Link to Github Issue

This issue should still be properly solved on LLK level, but given it's P0 in metal this should serve as a workaround until it does.

Checklist

@pavlejosipovic pavlejosipovic requested a review from a team as a code owner January 17, 2025 14:03
Copy link
Contributor

@ncvetkovicTT ncvetkovicTT left a comment

Choose a reason for hiding this comment

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

It's a valid workaround until we find out why block_ct_dim=1 doesn't work in llk_pack_untilize. Thanks @pavlejosipovic

pack_untilize_dst is causing issues on GS and WH
in case where reduce op is called after max_pool.

Workaround is to use default value for block_ct_dim
in pack_untilize_dst_init_short in max_pool kernel.

This doesn't affect correctness of max_pool op
and resolves the issue on subsequent reduce op.

Perf looks good on model device perf tests.
@pavlejosipovic pavlejosipovic force-pushed the pjosipovic/workaround_for_16847 branch from 70c5c8d to 63e574d Compare January 17, 2025 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants