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 2022/09/20 16:46:53 UTC

[GitHub] [tvm] tkonolige opened a new pull request, #12849: [FIX,PROFILING] Fix gpu timer name and lookup

tkonolige opened a new pull request, #12849:
URL: https://github.com/apache/tvm/pull/12849

   In the switch from gpu to cuda naming, the cuda timer was passed over. Renaming it to "profiling.timer.cuda" so it is correctly picked up by the timing mechanisms.
   
   @AndrewZhaoLuo 


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] AndrewZhaoLuo merged pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo merged PR #12849:
URL: https://github.com/apache/tvm/pull/12849


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] tkonolige commented on a diff in pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
tkonolige commented on code in PR #12849:
URL: https://github.com/apache/tvm/pull/12849#discussion_r975624976


##########
src/runtime/cuda/cuda_device_api.cc:
##########
@@ -283,7 +285,7 @@ class GPUTimerNode : public TimerNode {
 
 TVM_REGISTER_OBJECT_TYPE(GPUTimerNode);
 
-TVM_REGISTER_GLOBAL("profiling.timer.gpu").set_body_typed([](Device dev) {
+TVM_REGISTER_GLOBAL("profiling.timer.cuda").set_body_typed([](Device dev) {

Review Comment:
   done



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] AndrewZhaoLuo commented on pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #12849:
URL: https://github.com/apache/tvm/pull/12849#issuecomment-1252641388

   Can you post what sort of improvement you see?


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] AndrewZhaoLuo commented on a diff in pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #12849:
URL: https://github.com/apache/tvm/pull/12849#discussion_r975614266


##########
src/runtime/cuda/cuda_device_api.cc:
##########
@@ -283,7 +285,7 @@ class GPUTimerNode : public TimerNode {
 
 TVM_REGISTER_OBJECT_TYPE(GPUTimerNode);
 
-TVM_REGISTER_GLOBAL("profiling.timer.gpu").set_body_typed([](Device dev) {
+TVM_REGISTER_GLOBAL("profiling.timer.cuda").set_body_typed([](Device dev) {

Review Comment:
   I think this would prevent future errors like this from occuring. If it is easy to add in this PR



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] AndrewZhaoLuo commented on a diff in pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #12849:
URL: https://github.com/apache/tvm/pull/12849#discussion_r975612603


##########
src/runtime/cuda/cuda_device_api.cc:
##########
@@ -283,7 +285,7 @@ class GPUTimerNode : public TimerNode {
 
 TVM_REGISTER_OBJECT_TYPE(GPUTimerNode);
 
-TVM_REGISTER_GLOBAL("profiling.timer.gpu").set_body_typed([](Device dev) {
+TVM_REGISTER_GLOBAL("profiling.timer.cuda").set_body_typed([](Device dev) {

Review Comment:
   So it was looking for `profiling.timer.cuda` this entire time? Why wouldn't profiling crash when it could not find it? 



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] tkonolige commented on pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
tkonolige commented on PR #12849:
URL: https://github.com/apache/tvm/pull/12849#issuecomment-1252644884

   @AndrewZhaoLuo its hard to quantify what sort of improvement you would see.


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] tkonolige commented on a diff in pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
tkonolige commented on code in PR #12849:
URL: https://github.com/apache/tvm/pull/12849#discussion_r975613547


##########
src/runtime/cuda/cuda_device_api.cc:
##########
@@ -283,7 +285,7 @@ class GPUTimerNode : public TimerNode {
 
 TVM_REGISTER_OBJECT_TYPE(GPUTimerNode);
 
-TVM_REGISTER_GLOBAL("profiling.timer.gpu").set_body_typed([](Device dev) {
+TVM_REGISTER_GLOBAL("profiling.timer.cuda").set_body_typed([](Device dev) {

Review Comment:
   There's a default timer that is used. Maybe we should warn if the timer can not be found?



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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] AndrewZhaoLuo commented on pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #12849:
URL: https://github.com/apache/tvm/pull/12849#issuecomment-1252646663

   Wait, so what change will this actually have a on numbers from profiling?
   
   Will there be less overhead, will kernels be more accurately timed, etc?


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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


[GitHub] [tvm] tkonolige commented on pull request #12849: [FIX,PROFILING] Fix gpu timer name and lookup

Posted by GitBox <gi...@apache.org>.
tkonolige commented on PR #12849:
URL: https://github.com/apache/tvm/pull/12849#issuecomment-1252653914

   Yes, this will improve accuracy and decrease overhead.


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

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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