You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/05/03 17:20:09 UTC

[GitHub] [tvm] Lunderberg opened a new pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Lunderberg opened a new pull request #7966:
URL: https://github.com/apache/tvm/pull/7966


   - Bugfix, missing decoration on uniform buffer arguments.  
     Caused segfault when running on NVidia GPUs, with models that required uniform buffer arguments for constants (>128 bytes of constants).
   
   - Updated test_target_codegen_spirv.py to use @tvm.testing.requires_vulkan
     Previously, these tests would show success if USE_VULKAN=OFF.  Now, they correctly show that they are skipped instead.
   
   - Explicitly require int64 support at device creation time, since the TVM-generated shaders require it.
       
   - Allocate an appropriate descriptor set pool size for the buffer inputs, including both uniform and storage buffers, when not using the push_descriptor extension.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7966:
URL: https://github.com/apache/tvm/pull/7966#discussion_r625379351



##########
File path: tests/python/unittest/test_target_codegen_vulkan.py
##########
@@ -158,8 +158,54 @@ def build_f(f_ref):
     run_stress()
 
 
+@tvm.testing.requires_vulkan

Review comment:
       oops I didn't know that `test_target_codegen_vulkan.py` existed. We can merge `test_target_codegen_vulkan.py` and `test_target_codegen_spirv.py` if you like.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #7966:
URL: https://github.com/apache/tvm/pull/7966#issuecomment-831565158


   I confirmed that this patch works on AMD too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi merged pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Posted by GitBox <gi...@apache.org>.
masahi merged pull request #7966:
URL: https://github.com/apache/tvm/pull/7966


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] tmoreau89 commented on a change in pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Posted by GitBox <gi...@apache.org>.
tmoreau89 commented on a change in pull request #7966:
URL: https://github.com/apache/tvm/pull/7966#discussion_r625280303



##########
File path: src/target/spirv/ir_builder.h
##########
@@ -499,7 +499,8 @@ class IRBuilder {
    * \param binding The binding locaiton in descriptor set

Review comment:
       Please add the description of the new param `descriptor_set`, thank you!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] Lunderberg commented on a change in pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Posted by GitBox <gi...@apache.org>.
Lunderberg commented on a change in pull request #7966:
URL: https://github.com/apache/tvm/pull/7966#discussion_r625285702



##########
File path: src/target/spirv/ir_builder.h
##########
@@ -499,7 +499,8 @@ class IRBuilder {
    * \param binding The binding locaiton in descriptor set

Review comment:
       Thank you, and added.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] Lunderberg commented on a change in pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Posted by GitBox <gi...@apache.org>.
Lunderberg commented on a change in pull request #7966:
URL: https://github.com/apache/tvm/pull/7966#discussion_r625401305



##########
File path: tests/python/unittest/test_target_codegen_vulkan.py
##########
@@ -158,8 +158,54 @@ def build_f(f_ref):
     run_stress()
 
 
+@tvm.testing.requires_vulkan

Review comment:
       Sound good, and I'd agree that it would be better to have them merged.  Commit incoming to merge both files into `test_target_codegen_vulkan.py`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #7966:
URL: https://github.com/apache/tvm/pull/7966#issuecomment-831884758


   Thanks @Lunderberg @tmoreau89 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] Lunderberg commented on pull request #7966: [Vulkan][Runtime] Uniform buffer bugfix, minor cleanup

Posted by GitBox <gi...@apache.org>.
Lunderberg commented on pull request #7966:
URL: https://github.com/apache/tvm/pull/7966#issuecomment-831407047


   Potential reviewers: @masahi @tmoreau89 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org