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

[SPIRV] Minor update to TIR sort to make it work on VK/SPIR-V #7607

Merged
merged 4 commits into from Mar 9, 2021

Conversation

masahi
Copy link
Member

@masahi masahi commented Mar 8, 2021

Didn't look into why but this change seems to make it possible to run TIR sort with VK/SPIR-V on static shape. Dynamic shapes on SPIR-V seem to have some issues in general.

Enabled TIR sort tests on vulkan.

@mbrookhart @tmoreau89 @jwfromm

@mbrookhart
Copy link
Contributor

When I was developing this, I noticed that implicitly allocating the start/middle/end variables, i.e. removing this line: start = ib.allocate("int64", (1,), name="start", scope="local") broke things on nvptx, that's why I included them. NVPTX doesn't seem to be enabled for this test, could we turn that backend on and make sure we didn't break something?

@masahi
Copy link
Member Author

masahi commented Mar 9, 2021

seems everything is ok, ready to merge? @mbrookhart

@@ -75,7 +75,7 @@ def check_device(device):
f(tvm_data, tvm_out)
tvm.testing.assert_allclose(tvm_out.asnumpy(), np_sort, rtol=1e0)

for device in ["llvm", "cuda", "opencl"]:
for device in ["llvm", "cuda", "opencl", "vulkan", "nvptx"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @masahi - in the case of vulkan here the tests won't be run until we add vulkan to our CI docker image, right? These are just tested locally?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes vulkan was tested locally, but not on CI.

@masahi
Copy link
Member Author

masahi commented Mar 9, 2021

@mbrookhart I found the reason why dynamic shape is not working #7620. We can merge this PR first and I can add a dynamic sort test in #7620.

@mbrookhart mbrookhart merged commit a0656f5 into apache:main Mar 9, 2021
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
…#7607)

* sort started to working

* static size sort seems to be working

* test sort on vulkan

* add nvptx to sort test too
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
…#7607)

* sort started to working

* static size sort seems to be working

* test sort on vulkan

* add nvptx to sort test too
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.

None yet

3 participants