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/18 08:46:42 UTC
[GitHub] [tvm] masahi opened a new pull request #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
masahi opened a new pull request #7468:
URL: https://github.com/apache/tvm/pull/7468
----------------------------------------------------------------
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] mbrookhart commented on pull request #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#issuecomment-782544265
I have no complaints with this but also very little experience with this part of the code, so I'm happy with it if no one objects.
----------------------------------------------------------------
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 #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#issuecomment-782512215
If there is no comment, I assume everyone is cool with this change, and I'll ask someone to merge this.
----------------------------------------------------------------
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 #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#discussion_r580024093
##########
File path: python/tvm/topi/cuda/nms.py
##########
@@ -610,8 +610,10 @@ def _get_sorted_indices(data, data_buf, score_index, score_shape):
)
target = tvm.target.Target.current()
- # TODO(masahi): Check -libs=thrust option
- if target and target.kind.name in ["cuda", "rocm"] and is_thrust_available():
+ if target and (
+ can_use_thrust(target, "tvm.contrib.thrust.sort")
+ or can_use_rocthrust(target, "tvm.contrib.thrust.sort")
+ ):
sort_tensor = argsort_thrust(score_tensor, axis=1, is_ascend=False, dtype="int32")
else:
sort_tensor = argsort(score_tensor, axis=1, is_ascend=False, dtype="int32")
Review comment:
Warning added in https://github.com/apache/tvm/pull/7468/commits/607f2d591a36f14f5013090bb843da44554cb8c6
Please have a 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] Laurawly commented on pull request #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
Laurawly commented on pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#issuecomment-782517490
Could you also add a comment in the deploy ssd tutorial to let people know this change if they want to use cuda + thrust?
----------------------------------------------------------------
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 #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#discussion_r580024093
##########
File path: python/tvm/topi/cuda/nms.py
##########
@@ -610,8 +610,10 @@ def _get_sorted_indices(data, data_buf, score_index, score_shape):
)
target = tvm.target.Target.current()
- # TODO(masahi): Check -libs=thrust option
- if target and target.kind.name in ["cuda", "rocm"] and is_thrust_available():
+ if target and (
+ can_use_thrust(target, "tvm.contrib.thrust.sort")
+ or can_use_rocthrust(target, "tvm.contrib.thrust.sort")
+ ):
sort_tensor = argsort_thrust(score_tensor, axis=1, is_ascend=False, dtype="int32")
else:
sort_tensor = argsort(score_tensor, axis=1, is_ascend=False, dtype="int32")
Review comment:
Warning added in https://github.com/apache/tvm/pull/7468/commits/f688f64739375931574f40c0618920cff5f3b492
Please have a 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] masahi merged pull request #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #7468:
URL: https://github.com/apache/tvm/pull/7468
----------------------------------------------------------------
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 #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#issuecomment-783338519
thanks @comaniac @Laurawly @mbrookhart
----------------------------------------------------------------
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] Laurawly edited a comment on pull request #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
Laurawly edited a comment on pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#issuecomment-782517490
Could you also add a comment in the [deploy ssd tutorial](https://tvm.apache.org/docs/tutorials/frontend/deploy_ssd_gluoncv.html) to let people know this change if they want to use cuda + thrust?
----------------------------------------------------------------
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 a change in pull request #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#discussion_r578641105
##########
File path: python/tvm/topi/cuda/nms.py
##########
@@ -610,8 +610,10 @@ def _get_sorted_indices(data, data_buf, score_index, score_shape):
)
target = tvm.target.Target.current()
- # TODO(masahi): Check -libs=thrust option
- if target and target.kind.name in ["cuda", "rocm"] and is_thrust_available():
+ if target and (
+ can_use_thrust(target, "tvm.contrib.thrust.sort")
+ or can_use_rocthrust(target, "tvm.contrib.thrust.sort")
+ ):
sort_tensor = argsort_thrust(score_tensor, axis=1, is_ascend=False, dtype="int32")
else:
sort_tensor = argsort(score_tensor, axis=1, is_ascend=False, dtype="int32")
Review comment:
Just a thought and feel free to ignore: Since THRUST won't be used without `-libs=thrust` after this PR, would that be better to add a warning message here if `tvm.contrib.thrust.sort` is available (i.e., users built TVM with THRUST enabled) and target doesn't have `-libs=thrust`? We can then remove the warning in the next release or so.
----------------------------------------------------------------
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 #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#discussion_r578718336
##########
File path: python/tvm/topi/cuda/nms.py
##########
@@ -610,8 +610,10 @@ def _get_sorted_indices(data, data_buf, score_index, score_shape):
)
target = tvm.target.Target.current()
- # TODO(masahi): Check -libs=thrust option
- if target and target.kind.name in ["cuda", "rocm"] and is_thrust_available():
+ if target and (
+ can_use_thrust(target, "tvm.contrib.thrust.sort")
+ or can_use_rocthrust(target, "tvm.contrib.thrust.sort")
+ ):
sort_tensor = argsort_thrust(score_tensor, axis=1, is_ascend=False, dtype="int32")
else:
sort_tensor = argsort(score_tensor, axis=1, is_ascend=False, dtype="int32")
Review comment:
Sure I'll do that after I get more feedback
----------------------------------------------------------------
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] Laurawly edited a comment on pull request #7468: [CUDA][THRUST] Enforce -libs=thrust to allow thrust offload
Posted by GitBox <gi...@apache.org>.
Laurawly edited a comment on pull request #7468:
URL: https://github.com/apache/tvm/pull/7468#issuecomment-782517490
LGTM, just a small comment: could you also add a comment in the [deploy ssd tutorial](https://tvm.apache.org/docs/tutorials/frontend/deploy_ssd_gluoncv.html) to let people know this change if they want to use cuda + thrust?
----------------------------------------------------------------
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