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/02/14 19:57:57 UTC

[GitHub] [tvm] masahi opened a new pull request #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

masahi opened a new pull request #7457:
URL: https://github.com/apache/tvm/pull/7457


   


----------------------------------------------------------------
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] tqchen commented on pull request #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   I see, we can use this workaround for now. But can you please add ICHECKS so we error out when attempting to push a constant that goes beyond the limit?


----------------------------------------------------------------
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 #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   ok I'll try that


----------------------------------------------------------------
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 #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   https://github.com/apache/tvm/pull/7572


----------------------------------------------------------------
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 #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   Maybe a more straightforward approach is to send a scalar as a buffer of size 1, just like CUDA/OpenCL backend does.


----------------------------------------------------------------
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] tqchen commented on pull request #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   Thanks masa, perhaps it is a good time to revisit whether Vulkan metal could work with i64. 


----------------------------------------------------------------
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 #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   Does it make sense to use `vkCmdPushConstants` twice, one for 32 bit and another for 64 bit? If that is not possible, I think we need this patch for a workaround.


----------------------------------------------------------------
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] tqchen removed a comment on pull request #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

Posted by GitBox <gi...@apache.org>.
tqchen removed a comment on pull request #7457:
URL: https://github.com/apache/tvm/pull/7457#issuecomment-779883695


   I see, we can use this workaround for now. But can you please add ICHECKS so we error out when attempting to push a constant that goes beyond the limit?


----------------------------------------------------------------
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] tqchen commented on pull request #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   The way device side works is through creating a struct.
   
   say the original function is ```fn(int32 arg0, int64 arg1)```
   
   We generate the following device code:
   
   ```
   struct ArgBuffer {
      // always generate two int values to pad things to the same alignment
      int32 arg0[2];
      int64 arg1
   };
   
   fn (ArgBuffer* arg_buffer) {
      int32 arg0 = arg_buffer->arg0[0];
      int64 arg1 = arg_buffer->arg1;
   }
   ```


----------------------------------------------------------------
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] tqchen commented on pull request #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   I take another look and it seems that both vulkan and metal(after metal 2.2) now support i64.
   
   Perhaps we could update the ArgUnion solution of these APIs to allow pass 64 bit integer. For example
   
   ```c++
    union ArgUnion64 { 
      int32_t v_int32; 
      uint32_t v_uint32; 
      float v_float32;
      int64_t v_int64;
   }'
   ```
   On the device end, always create array of size two `int32_t v_int32[2]`, and read the value from the first element. This would require all arguments to be aligned to 64 bit, given not a lot of arguments are being passed in this way, we should be good.
   
   @masahi do you mind to make that change and test out instead?
   
   


----------------------------------------------------------------
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 closed pull request #7457: [TOPI][Vulkan, Metal] Avoid passing int64 scalar arg to VK/Metal runtime

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


   


----------------------------------------------------------------
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