comaniac commented on a change in pull request #9248:
URL: https://github.com/apache/tvm/pull/9248#discussion_r726611923
##########
File path: python/tvm/autotvm/graph_tuner/utils/traverse_graph.py
##########
@@ -50,6 +51,8 @@ def expr2graph(expr, target_ops, node_dict, node_list):
Each node will be stored as a dictionary in the format of
{"op": str, "node": tvm.relay.expr, "inputs": [int], "types":
[tvm.relay.Type],
"name": str, "workloads": [tuple], "topi_op": [function]}
+
+ tvm_target : The TVM Target
Review comment:
```suggestion
tvm_target : tvm.target
The TVM target object.
```
Also please double check the target type. Based on
`base_graph_tuner.py:L134`, it seems to me that this is always a target object
instead of a target string.
##########
File path: tests/python/unittest/test_autotvm_graph_tuner_utils.py
##########
@@ -57,7 +60,7 @@ def test_has_multiple_inputs():
target_ops = [relay.op.get("nn.conv2d")]
node_list = []
node_dict = {}
- expr2graph(net, target_ops, node_dict, node_list)
+ expr2graph(net, target_ops, node_dict, node_list, "llvm")
Review comment:
Be careful about the target type.
##########
File path: python/tvm/autotvm/graph_tuner/utils/traverse_graph.py
##########
@@ -77,7 +80,16 @@ def _infer_type(node):
return entry if isinstance(node, relay.Function) else entry.body
-def _expr2graph_impl(expr, target_ops, node_dict, node_list):
+def _replace_device_with_tracing(target):
+ """This is to replace -device=XXX with -device=tracing in the tvm_target
string.
+ It is a stand-along function for testability"""
Review comment:
1. Write down more explanations in our discussion, including why we need
to override and why it is safe.
2. Add parameter and return type.
##########
File path: tests/python/unittest/test_autotvm_graph_tuner_utils.py
##########
@@ -155,6 +158,19 @@ def test_get_out_nodes():
)
[email protected](
+ "target,expected",
+ [
+ pytest.param("llvm", "llvm -device=tracing"),
+ pytest.param("llvm -device=arm_cpu -arg=xxx", "llvm -device=tracing
-arg=xxx"),
+ pytest.param("llvm -device=arm_cpu", "llvm -device=tracing"),
+ pytest.param("llvm -device=abc, def", "llvm -device=tracing"),
Review comment:
I don't think we need these 2. Instead, could we add a case for cuda?
##########
File path: python/tvm/autotvm/graph_tuner/utils/traverse_graph.py
##########
@@ -77,7 +80,16 @@ def _infer_type(node):
return entry if isinstance(node, relay.Function) else entry.body
-def _expr2graph_impl(expr, target_ops, node_dict, node_list):
+def _replace_device_with_tracing(target):
+ """This is to replace -device=XXX with -device=tracing in the tvm_target
string.
+ It is a stand-along function for testability"""
+ if "-device" in target:
+ return re.sub("-device=[^\\-$]+", "-device=tracing ", target).strip("
")
+ else:
+ return target + " -device=tracing"
Review comment:
Note that the input target is an object, not a string.
```python
if "device" in target.attrs:
return re.sub("-device=[^\\-$]+", "-device=tracing ", str(target)).strip("
")
else:
return ("%s -device=tracing" % str(target)).strip(" ")
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
