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/04 22:51:11 UTC

[GitHub] [tvm] zxybazh opened a new pull request #7410: Add cuda tags and unit test

zxybazh opened a new pull request #7410:
URL: https://github.com/apache/tvm/pull/7410


   This commit added cuda tags of most cuda devices for target specification and completed cuda target tag attributes.


----------------------------------------------------------------
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] junrushao1994 commented on a change in pull request #7410: Add cuda tags and unit test

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



##########
File path: src/target/tag.cc
##########
@@ -21,9 +21,22 @@
  * \file src/target/target_tag.cc
  * \brief Target tag registry
  */
+
+#define TVM_REGISTER_CUDA_TAG(Name, Arch, SharedMem, RegPerBlock)\
+TVM_REGISTER_TARGET_TAG(Name)             \
+    .set_config({                                       \
+        {"kind", String("cuda")},                       \
+        {"arch", String(Arch)},                         \
+        {"shared_memory_per_block", Integer(SharedMem)},\
+        {"registers_per_block", Integer(RegPerBlock)},  \
+        {"max_threads_per_block", Integer(1024)},       \
+        {"thread_warp_size", Integer(32)},              \
+    });
+

Review comment:
       Please move the macro definition to immediately before `TVM_REGISTER_CUDA_TAG `, and undef it once it is used

##########
File path: src/target/tag.cc
##########
@@ -21,9 +21,22 @@
  * \file src/target/target_tag.cc
  * \brief Target tag registry
  */
+
+#define TVM_REGISTER_CUDA_TAG(Name, Arch, SharedMem, RegPerBlock)\
+TVM_REGISTER_TARGET_TAG(Name)             \
+    .set_config({                                       \
+        {"kind", String("cuda")},                       \
+        {"arch", String(Arch)},                         \
+        {"shared_memory_per_block", Integer(SharedMem)},\
+        {"registers_per_block", Integer(RegPerBlock)},  \
+        {"max_threads_per_block", Integer(1024)},       \
+        {"thread_warp_size", Integer(32)},              \
+    });
+

Review comment:
       Please move the macro definition to immediately before `TVM_REGISTER_CUDA_TAG `, and undef it once used

##########
File path: src/target/tag.cc
##########
@@ -66,12 +79,252 @@ Target TargetTag::AddTag(String name, Map<String, ObjectRef> config, bool overri
   return Target(config);
 }
 
-/**********  Register Target tags  **********/
-
-TVM_REGISTER_TARGET_TAG("nvidia/rtx2080ti")
-    .set_config({
-        {"kind", String("cuda")},
-        {"arch", String("sm_75")},
-    });
+/**********  Register Target tags  ***********/
 
+  TVM_REGISTER_CUDA_TAG("nvidia/tesla-k80", "sm_37", 49152, 65536);

Review comment:
       Wait, shall we remove the indent here 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] comaniac merged pull request #7410: Add cuda tags and unit test

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


   


----------------------------------------------------------------
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] zxybazh commented on a change in pull request #7410: Add cuda tags and unit test

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



##########
File path: src/target/tag.cc
##########
@@ -66,12 +79,252 @@ Target TargetTag::AddTag(String name, Map<String, ObjectRef> config, bool overri
   return Target(config);
 }
 
-/**********  Register Target tags  **********/
-
-TVM_REGISTER_TARGET_TAG("nvidia/rtx2080ti")
-    .set_config({
-        {"kind", String("cuda")},
-        {"arch", String("sm_75")},
-    });
+/**********  Register Target tags  ***********/
 
+  TVM_REGISTER_CUDA_TAG("nvidia/tesla-k80", "sm_37", 49152, 65536);

Review comment:
       Fixed.




----------------------------------------------------------------
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] junrushao1994 commented on pull request #7410: Add cuda tags and unit test

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


   @comaniac Would you mind taking a second look?


----------------------------------------------------------------
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] zxybazh commented on a change in pull request #7410: Add cuda tags and unit test

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



##########
File path: src/target/tag.cc
##########
@@ -21,9 +21,22 @@
  * \file src/target/target_tag.cc
  * \brief Target tag registry
  */
+
+#define TVM_REGISTER_CUDA_TAG(Name, Arch, SharedMem, RegPerBlock)\
+TVM_REGISTER_TARGET_TAG(Name)             \
+    .set_config({                                       \
+        {"kind", String("cuda")},                       \
+        {"arch", String(Arch)},                         \
+        {"shared_memory_per_block", Integer(SharedMem)},\
+        {"registers_per_block", Integer(RegPerBlock)},  \
+        {"max_threads_per_block", Integer(1024)},       \
+        {"thread_warp_size", Integer(32)},              \
+    });
+

Review comment:
       Fixed.
   




----------------------------------------------------------------
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] junrushao1994 commented on a change in pull request #7410: Add cuda tags and unit test

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



##########
File path: src/target/tag.cc
##########
@@ -21,9 +21,22 @@
  * \file src/target/target_tag.cc
  * \brief Target tag registry
  */
+
+#define TVM_REGISTER_CUDA_TAG(Name, Arch, SharedMem, RegPerBlock)\
+TVM_REGISTER_TARGET_TAG(Name)             \
+    .set_config({                                       \
+        {"kind", String("cuda")},                       \
+        {"arch", String(Arch)},                         \
+        {"shared_memory_per_block", Integer(SharedMem)},\
+        {"registers_per_block", Integer(RegPerBlock)},  \
+        {"max_threads_per_block", Integer(1024)},       \
+        {"thread_warp_size", Integer(32)},              \
+    });
+

Review comment:
       Please move the macro definition to immediately before `TVM_REGISTER_CUDA_TAG `, and undef it once used




----------------------------------------------------------------
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] junrushao1994 commented on a change in pull request #7410: Add cuda tags and unit test

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



##########
File path: src/target/tag.cc
##########
@@ -66,12 +79,252 @@ Target TargetTag::AddTag(String name, Map<String, ObjectRef> config, bool overri
   return Target(config);
 }
 
-/**********  Register Target tags  **********/
-
-TVM_REGISTER_TARGET_TAG("nvidia/rtx2080ti")
-    .set_config({
-        {"kind", String("cuda")},
-        {"arch", String("sm_75")},
-    });
+/**********  Register Target tags  ***********/
 
+  TVM_REGISTER_CUDA_TAG("nvidia/tesla-k80", "sm_37", 49152, 65536);

Review comment:
       Wait, shall we remove the indent here 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] junrushao1994 commented on pull request #7410: Add cuda tags and unit test

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


   Thanks @comaniac! For reference, we crawled the information from:
   
   [1] https://developer.nvidia.com/cuda-gpus
   [2] https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#features-and-technical-specifications


----------------------------------------------------------------
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] comaniac commented on pull request #7410: Add cuda tags and unit test

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


   Thanks @zxybazh @junrushao1994 
   It seems to me that we should be consistent to let all fields have a default value (or not), but we could do this later.


----------------------------------------------------------------
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] zxybazh commented on pull request #7410: Add cuda tags and unit test

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


   @junrushao1994  Please review, thanks!


----------------------------------------------------------------
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] junrushao1994 commented on a change in pull request #7410: Add cuda tags and unit test

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



##########
File path: src/target/tag.cc
##########
@@ -21,9 +21,22 @@
  * \file src/target/target_tag.cc
  * \brief Target tag registry
  */
+
+#define TVM_REGISTER_CUDA_TAG(Name, Arch, SharedMem, RegPerBlock)\
+TVM_REGISTER_TARGET_TAG(Name)             \
+    .set_config({                                       \
+        {"kind", String("cuda")},                       \
+        {"arch", String(Arch)},                         \
+        {"shared_memory_per_block", Integer(SharedMem)},\
+        {"registers_per_block", Integer(RegPerBlock)},  \
+        {"max_threads_per_block", Integer(1024)},       \
+        {"thread_warp_size", Integer(32)},              \
+    });
+

Review comment:
       Please move the macro definition to immediately before `TVM_REGISTER_CUDA_TAG `, and undef it once it is used




----------------------------------------------------------------
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] zxybazh commented on a change in pull request #7410: Add cuda tags and unit test

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



##########
File path: src/target/tag.cc
##########
@@ -66,12 +79,252 @@ Target TargetTag::AddTag(String name, Map<String, ObjectRef> config, bool overri
   return Target(config);
 }
 
-/**********  Register Target tags  **********/
-
-TVM_REGISTER_TARGET_TAG("nvidia/rtx2080ti")
-    .set_config({
-        {"kind", String("cuda")},
-        {"arch", String("sm_75")},
-    });
+/**********  Register Target tags  ***********/
 
+  TVM_REGISTER_CUDA_TAG("nvidia/tesla-k80", "sm_37", 49152, 65536);

Review comment:
       Fixed.

##########
File path: src/target/tag.cc
##########
@@ -21,9 +21,22 @@
  * \file src/target/target_tag.cc
  * \brief Target tag registry
  */
+
+#define TVM_REGISTER_CUDA_TAG(Name, Arch, SharedMem, RegPerBlock)\
+TVM_REGISTER_TARGET_TAG(Name)             \
+    .set_config({                                       \
+        {"kind", String("cuda")},                       \
+        {"arch", String(Arch)},                         \
+        {"shared_memory_per_block", Integer(SharedMem)},\
+        {"registers_per_block", Integer(RegPerBlock)},  \
+        {"max_threads_per_block", Integer(1024)},       \
+        {"thread_warp_size", Integer(32)},              \
+    });
+

Review comment:
       Fixed.
   




----------------------------------------------------------------
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] zxybazh commented on pull request #7410: Add cuda tags and unit test

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


   @junrushao1994  Please review, thanks!


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