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

[Target] Add a default warp size 1 for vulkan and opencl #8109

Merged
merged 1 commit into from
May 22, 2021

Conversation

masahi
Copy link
Member

@masahi masahi commented May 21, 2021

I've made this change too many times during my development, to workaround topi scatter op that requires thread_warp_size for no good reason. Since #8030 transpose op also requires thread_warp_size and all models that use transpose op are broken on vk.

@tmoreau89 @Lunderberg @junrushao1994 @zxybazh

@zxybazh
Copy link
Member

zxybazh commented May 21, 2021

The code looks good to me. May I know why the default thread_warp_size is 1 here?

@masahi
Copy link
Member Author

masahi commented May 21, 2021

Because it need to support ALL vulkan or opencl devices. 1 is the only value that could work, but it is useless in terms of performance. It's only there so that compiling models on vulkan wouldn't break.

@zxybazh
Copy link
Member

zxybazh commented May 21, 2021

Thanks for the quick reply. In that case, thread_warp_size of 1 should be a good default value.

@Lunderberg
Copy link
Contributor

Looks good to me, and I agree on using 1 as the default thread_warp_size. Both the VkPhysicalDeviceSubgroupProperties and the VkPhysicalDeviceSubgroupSizeControlPropertiesEXT documentation give a minimum subgroup size of 1, so the spec doesn't give us any further guarantees.

@tqchen tqchen merged commit 8934d6b into apache:main May 22, 2021
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 17, 2021
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 17, 2021
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.

4 participants