[GitHub] [incubator-tvm] ANSHUMAN87 edited a comment on pull request #5510: [FRONTEND][TFLite] Fully connected op conversion made in sync with TFLite
ANSHUMAN87 edited a comment on pull request #5510: URL: https://github.com/apache/incubator-tvm/pull/5510#issuecomment-623859989 > Minor nits, A better commit message would be to refer to handling batch matmul in fully connected op rather than what it stands. > > Isn't there a need for Quantized inputs testing here ? @u99127 : Thanks for your comments! Just for your info, TFLite converter has multiple limitations in terms of various input types supported, so it is not possible always to add all the cases. I had already tried for it, but converter has errors. Also it does not support N-dim. About the commit, actually the changes are quite unrelated to batch matmul, it is just that open up the issue in implementation. So IMO the commit message looks ok to me. Please let me know if you think otherwise. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #5510: [FRONTEND][TFLite] Fully connected op conversion made in sync with TFLite
ANSHUMAN87 commented on a change in pull request #5510: URL: https://github.com/apache/incubator-tvm/pull/5510#discussion_r419869014 ## File path: python/tvm/relay/frontend/tflite.py ## @@ -1329,16 +1329,20 @@ def convert_fully_connected(self, op): input_tensor_shape = input_tensor.tensor.ShapeAsNumpy() weight_tensor_shape = weight_tensor.tensor.ShapeAsNumpy() -# reshape input tensor from N H W C to N H*W*C -input_size_per_batch = 1 -for s in range(1, len(input_tensor_shape)): -input_size_per_batch *= input_tensor_shape[s] -assert input_size_per_batch == weight_tensor_shape[1], \ -"input size and weight size are mismatched" -target_shape = tuple((input_tensor_shape[0], input_size_per_batch)) Review comment: @u99127 : This assert is wrong assumption, as it always assumes first dimension is the batch size, while it has utterly failed to link between the input and weights, which is highly dependent when you operate in Fully_Connected or Dense operator. As the frontend is just a transformation of an successfully deployed model, so we can safely assume the valid relationship exists! However this point will be evident when i put some comment to describe batch size computation as per your last comment. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] ANSHUMAN87 commented on pull request #5510: [FRONTEND][TFLite] Fully connected op conversion made in sync with TFLite
ANSHUMAN87 commented on pull request #5510: URL: https://github.com/apache/incubator-tvm/pull/5510#issuecomment-623859989 > Minor nits, A better commit message would be to refer to handling batch matmul in fully connected op rather than what it stands. > > Isn't there a need for Quantized inputs testing here ? @u99127 : Thanks for your comments! Just for your info, TFLite converter has multiple limitations in terms of various input types supported, so it is not possible always to add all the cases. I had already tried for it, but converter has errors. Also it does not support N-dim. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] siju-samuel commented on a change in pull request #5486: [TFLITE]Select op support for tflite frontend
siju-samuel commented on a change in pull request #5486: URL: https://github.com/apache/incubator-tvm/pull/5486#discussion_r419859922 ## File path: python/tvm/relay/frontend/tflite.py ## @@ -109,16 +109,17 @@ def __init__(self, model, subgraph, exp_tab): 'PAD': self.convert_pad, 'POW': self.convert_pow, 'PRELU': self.convert_prelu, -'REDUCE_ANY': self._convert_reduce_any, -'REDUCE_MAX': self._convert_reduce_max, -'REDUCE_MIN': self._convert_reduce_min, -'REDUCE_PROD': self._convert_reduce_prod, +'REDUCE_ANY': self.convert_reduce_any, +'REDUCE_MAX': self.convert_reduce_max, +'REDUCE_MIN': self.convert_reduce_min, +'REDUCE_PROD': self.convert_reduce_prod, 'RELU':self.convert_relu, Review comment: @u99127 for code consistency i changed, its not related to this PR. This 5 methods were starting with `_` and i removed those. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] siju-samuel commented on pull request #5488: [TFLITE] SELECT
siju-samuel commented on pull request #5488: URL: https://github.com/apache/incubator-tvm/pull/5488#issuecomment-623853156 @dhruvaray Thanks for the PR. This part is covered in PR #5486 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #5506: [WEB][RUNTIME] TVM WebAssembly JS Runtime
tqchen commented on pull request #5506: URL: https://github.com/apache/incubator-tvm/pull/5506#issuecomment-623799974 @kazum Updated to include package.json 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: us...@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request #5459: [Relay][Op]Support symbolic TopK, Ones, Zeros and Full
kevinthesun commented on a change in pull request #5459:
URL: https://github.com/apache/incubator-tvm/pull/5459#discussion_r419829632
##
File path: include/tvm/relay/attrs/algorithm.h
##
@@ -50,14 +50,14 @@ struct ArgsortAttrs : public tvm::AttrsNode {
};
struct TopKAttrs : public tvm::AttrsNode {
- int k;
+ Expr k;
Review comment:
Agree this is just a temp solution before we can unify symbolic args
with attrs. Conceptually we don't need attrs to include these inputs any more,
no matter they are actually constant or not.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request #4312: [TOPI][Relay][OP] Dynamic NMS and strided_slice
kevinthesun commented on a change in pull request #4312: URL: https://github.com/apache/incubator-tvm/pull/4312#discussion_r419826528 ## File path: python/tvm/relay/op/_transform.py ## @@ -101,6 +101,28 @@ def _arange_shape_func(start, stop, step): def arange_shape_func(attrs, inputs, _): return [_arange_shape_func(*inputs)] +@script +def _strided_slice_shape_func(data, begin, end, strides): +ndim = len(data.shape) +out = output_tensor((ndim,), "int64") +for i in const_range(ndim): +cbegin = 0 +cend = data.shape[i] +cstride = 1 +if len(begin) > i: +cbegin = begin[i] +if len(end) > i: +cend = end[i] +if len(strides) > i: Review comment: ```suggestion if strides.shape[0] > i: ``` 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: us...@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request #4312: [TOPI][Relay][OP] Dynamic NMS and strided_slice
kevinthesun commented on a change in pull request #4312: URL: https://github.com/apache/incubator-tvm/pull/4312#discussion_r419826341 ## File path: python/tvm/relay/op/_transform.py ## @@ -101,6 +101,28 @@ def _arange_shape_func(start, stop, step): def arange_shape_func(attrs, inputs, _): return [_arange_shape_func(*inputs)] +@script +def _strided_slice_shape_func(data, begin, end, strides): +ndim = len(data.shape) +out = output_tensor((ndim,), "int64") +for i in const_range(ndim): +cbegin = 0 +cend = data.shape[i] +cstride = 1 +if len(begin) > i: Review comment: ```suggestion if begin.shape[0] > i: ``` 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: us...@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request #4312: [TOPI][Relay][OP] Dynamic NMS and strided_slice
kevinthesun commented on a change in pull request #4312: URL: https://github.com/apache/incubator-tvm/pull/4312#discussion_r419826426 ## File path: python/tvm/relay/op/_transform.py ## @@ -101,6 +101,28 @@ def _arange_shape_func(start, stop, step): def arange_shape_func(attrs, inputs, _): return [_arange_shape_func(*inputs)] +@script +def _strided_slice_shape_func(data, begin, end, strides): +ndim = len(data.shape) +out = output_tensor((ndim,), "int64") +for i in const_range(ndim): +cbegin = 0 +cend = data.shape[i] +cstride = 1 +if len(begin) > i: +cbegin = begin[i] +if len(end) > i: Review comment: ```suggestion if end.shape[0] > i: ``` 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: us...@infra.apache.org
[GitHub] [incubator-tvm] wpan11nv commented on a change in pull request #5498: [Optimization] Warp level reduction support for CUDA
wpan11nv commented on a change in pull request #5498:
URL: https://github.com/apache/incubator-tvm/pull/5498#discussion_r419821795
##
File path: src/tir/transforms/lower_warp_memory.cc
##
@@ -265,10 +265,11 @@ class WarpAccessRewriter : protected StmtExprMutator {
<< op->index << " local_index=" << local_index;
PrimExpr load_value = LoadNode::make(
op->dtype, op->buffer_var, local_index, op->predicate);
+ PrimExpr mask = IntImm(DataType::UInt(32), 0x);
Review comment:
I suspect this is not in use. Let see if this is the case, A proper fix
is to use __shfl_sync in lower_warp_memory pass.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] anijain2305 commented on issue #5455: [CI] [TEST] test_conv2d_int8_intrinsics
anijain2305 commented on issue #5455: URL: https://github.com/apache/incubator-tvm/issues/5455#issuecomment-623763751 Dug little bit deeper, but still not able to root cause. I think the const int bounds are good. https://github.com/apache/incubator-tvm/blob/7e88030a804357421b766d7309f9085ca2b83378/src/arith/const_int_bound.cc#L149-L161 The problem here is that two different PrimExpr have same PrimExprNode* at L150 ~~~ tvm/src/arith/const_int_bound.cc:166: Expr --> 63, op = 0x3b73800 tvm/src/arith/const_int_bound.cc:154: Expr --> 15, op = 0x3b73800 ~~~ And therefore, Expr(63) whose ConstIntBound is set correctly to [63, 63] is used to compare the bounds for Expr(15), causing failure. Will pursue this direction, please let me know of any suggestions @tqchen 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419742381 ## File path: .gitignore ## @@ -2,9 +2,10 @@ __pycache__/ *.py[cod] *$py.class - +*.S Review comment: ah ok. I guess this means we can't check in assembly without removing it, but we can do that later 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#issuecomment-623717030 @tmoreau89 @areusch Thanks, please https://tvm.apache.org/docs/contribute/code_review.html?highlight=approve#approve-and-request-changes-explicitly 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419739620 ## File path: .gitignore ## @@ -2,9 +2,10 @@ __pycache__/ *.py[cod] *$py.class - +*.S Review comment: try to ignore assembly files generated in commands. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#issuecomment-623713671 I think this looks good to me now! 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419732922 ## File path: .gitignore ## @@ -2,9 +2,10 @@ __pycache__/ *.py[cod] *$py.class - +*.S Review comment: why this change? 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: us...@infra.apache.org
[GitHub] [incubator-tvm] u99127 commented on a change in pull request #5486: [TFLITE]Select op support for tflite frontend
u99127 commented on a change in pull request #5486: URL: https://github.com/apache/incubator-tvm/pull/5486#discussion_r419721738 ## File path: python/tvm/relay/frontend/tflite.py ## @@ -109,16 +109,17 @@ def __init__(self, model, subgraph, exp_tab): 'PAD': self.convert_pad, 'POW': self.convert_pow, 'PRELU': self.convert_prelu, -'REDUCE_ANY': self._convert_reduce_any, -'REDUCE_MAX': self._convert_reduce_max, -'REDUCE_MIN': self._convert_reduce_min, -'REDUCE_PROD': self._convert_reduce_prod, +'REDUCE_ANY': self.convert_reduce_any, +'REDUCE_MAX': self.convert_reduce_max, +'REDUCE_MIN': self.convert_reduce_min, +'REDUCE_PROD': self.convert_reduce_prod, 'RELU':self.convert_relu, Review comment: Why are these changes relevant to this Pull request ? 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: us...@infra.apache.org
[incubator-tvm] branch master updated: [RUST][RUNTIME] Fix workspace (#5503)
This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git
The following commit(s) were added to refs/heads/master by this push:
new 7e88030 [RUST][RUNTIME] Fix workspace (#5503)
7e88030 is described below
commit 7e88030a804357421b766d7309f9085ca2b83378
Author: MORITA Kazutaka
AuthorDate: Tue May 5 05:51:36 2020 +0900
[RUST][RUNTIME] Fix workspace (#5503)
* [RUST][RUNTIME] Fix workspace
* use ok_or_else instead of ok_or
---
rust/runtime/src/workspace.rs | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/rust/runtime/src/workspace.rs b/rust/runtime/src/workspace.rs
index 8344dfb..65ad253 100644
--- a/rust/runtime/src/workspace.rs
+++ b/rust/runtime/src/workspace.rs
@@ -64,7 +64,7 @@ impl WorkspacePool {
.iter()
.fold(None, |cur_ws_idx: Option, | {
let ws_size = self.workspaces[idx].size();
-if !ws_size >= size {
+if ws_size < size {
return cur_ws_idx;
}
cur_ws_idx.or(Some(idx)).and_then(|cur_idx| {
@@ -92,9 +92,8 @@ impl WorkspacePool {
break;
}
}
-if let Some(ws_idx) = ws_idx {
-self.free.push(ws_idx);
-}
+let ws_idx = ws_idx.ok_or_else(|| format_err!("Invalid pointer"))?;
+self.free.push(ws_idx);
Ok(())
}
}
@@ -135,6 +134,5 @@ pub extern "C" fn TVMBackendFreeWorkspace(
Ok(()) => 0,
Err(_) => -1,
}) as c_int
-});
-0
+})
}
[GitHub] [incubator-tvm] u99127 commented on a change in pull request #5510: [FRONTEND][TFLite] Fully connected op conversion made in sync with TFLite
u99127 commented on a change in pull request #5510:
URL: https://github.com/apache/incubator-tvm/pull/5510#discussion_r419715441
##
File path: tests/python/frontend/tflite/test_forward.py
##
@@ -419,6 +419,31 @@ def test_forward_cast():
_test_cast(np.arange(6.0, dtype=np.float32).reshape((1, 6)),
cast_dtype=tf.uint8)
_test_cast(np.arange(6.0, dtype=np.int32).reshape((1, 6)),
cast_dtype=tf.int64)
+###
+# Batch Mat Mul
+#
+def _test_batch_matmul(A_shape, B_shape, dtype, adjoint_a=False,
adjoint_b=False):
+with tf.Graph().as_default():
+A = array_ops.placeholder(shape=A_shape, dtype=dtype, name='A')
+B = array_ops.placeholder(shape=B_shape, dtype=dtype, name='B')
+result = math_ops.matmul(A, B, adjoint_a=adjoint_a,
+ adjoint_b=adjoint_b, name='batchmatmul')
+
+A_np = np.random.uniform(high=5.0, size=A_shape).astype(dtype)
+B_np = np.random.uniform(high=5.0, size=B_shape).astype(dtype)
+compare_tflite_with_tvm([A_np, B_np], [A.name, B.name], [A, B],
[result])
+
+
+def test_forward_batch_matmul():
+""" BATCH_MAT_MUL """
+print("Jai hanuman!!!")
+_test_batch_matmul((3, 5, 4), (3, 4, 5), 'float32')
+_test_batch_matmul((3, 5, 4), (3, 4, 5), 'float32', True, True)
+_test_batch_matmul((3, 5, 4), (3, 5, 4), 'float32', True, False)
+_test_batch_matmul((3, 5, 4), (3, 5, 4), 'float32', False, True)
+_test_batch_matmul((2, 3, 4, 5, 6), (2, 3, 4, 6, 5), 'float32')
+print("Jai hanuman!!!")
Review comment:
Unneeded print.
##
File path: python/tvm/relay/frontend/tflite.py
##
@@ -1329,16 +1329,20 @@ def convert_fully_connected(self, op):
input_tensor_shape = input_tensor.tensor.ShapeAsNumpy()
weight_tensor_shape = weight_tensor.tensor.ShapeAsNumpy()
-# reshape input tensor from N H W C to N H*W*C
-input_size_per_batch = 1
-for s in range(1, len(input_tensor_shape)):
-input_size_per_batch *= input_tensor_shape[s]
-assert input_size_per_batch == weight_tensor_shape[1], \
-"input size and weight size are mismatched"
-target_shape = tuple((input_tensor_shape[0], input_size_per_batch))
+# Weight should have only 2 dimensions(TFLite convention)
+assert len(weight_tensor_shape) == 2, "Weight should be only 2-dim"
+
+input_size = 1
+for s in range(0, len(input_tensor_shape)):
+input_size *= input_tensor_shape[s]
+
Review comment:
A comment here would be appropriate or indeed a reference to TFlite
documentation.
##
File path: python/tvm/relay/frontend/tflite.py
##
@@ -1329,16 +1329,20 @@ def convert_fully_connected(self, op):
input_tensor_shape = input_tensor.tensor.ShapeAsNumpy()
weight_tensor_shape = weight_tensor.tensor.ShapeAsNumpy()
-# reshape input tensor from N H W C to N H*W*C
-input_size_per_batch = 1
-for s in range(1, len(input_tensor_shape)):
-input_size_per_batch *= input_tensor_shape[s]
-assert input_size_per_batch == weight_tensor_shape[1], \
-"input size and weight size are mismatched"
-target_shape = tuple((input_tensor_shape[0], input_size_per_batch))
Review comment:
Can you explain why this assert is wrong or needs to be removed in your
covering note ?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419718629
##
File path: src/runtime/rpc/rpc_pipe_impl.cc
##
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file rpc_pipe_impl.cc
+ * \brief Pipe-based RPC channel.
+ */
+// Linux only for now, as linux is the most common usecase.
+#if defined(__linux__) || defined(__ANDROID__)
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include "rpc_endpoint.h"
+#include "rpc_local_session.h"
+#include "../../support/pipe.h"
+
+namespace tvm {
+namespace runtime {
+
+class PipeChannel final : public RPCChannel {
+ public:
+ explicit PipeChannel(int readfd, int writefd, pid_t child_pid)
+ : readfd_(readfd), writefd_(writefd), child_pid_(child_pid) {
+ }
+
+ ~PipeChannel() {
+Close();
+ }
+
+ size_t Send(const void* data, size_t size) final {
+ssize_t n = write(writefd_, data, size);
+if (n == -1) {
+ LOG(FATAL) << "Pipe write error";
+}
+return static_cast(n);
+ }
+
+ size_t Recv(void* data, size_t size) final {
+ssize_t n = read(readfd_, data, size);
+if (n == -1) {
+ LOG(FATAL) << "Pipe read error";
+}
+return static_cast(n);
+ }
+
+ void Close() {
+close(readfd_);
+close(writefd_);
+kill(child_pid_, SIGKILL);
+ }
+
+ private:
+ int readfd_;
+ int writefd_;
+ pid_t child_pid_;
+};
+
+
+Module CreatePipeClient(std::vector cmd) {
+ int parent2child[2];
+ int child2parent[2];
+ CHECK_EQ(pipe(parent2child), 0);
+ CHECK_EQ(pipe(child2parent), 0);
+
+ int parent_read = child2parent[0];
+ int parent_write = parent2child[1];
+ int child_read = parent2child[0];
+ int child_write = child2parent[1];
+
+ pid_t pid = fork();
+ if (pid == 0) {
+// child process
+close(parent_read);
+close(parent_write);
+std::string sread_pipe = std::to_string(child_read);
Review comment:
oh okay, that makes sense to me. I think this is fine then.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419717711 ## File path: python/tvm/_ffi/base.py ## @@ -48,8 +48,11 @@ def _load_lib(): """Load libary by searching possible path.""" lib_path = libinfo.find_lib_path() lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_GLOBAL) -# DMatrix functions lib.TVMGetLastError.restype = ctypes.c_char_p +# Put the libpath to LD_LIBRARY_PATH +# will be useful for pipe session to find libtvm +os.environ["LD_LIBRARY_PATH"] = "%s:%s" % ( Review comment: correct me if i'm wrong, doesn't this mean that any python script that imports tvm and then launches a subprocess will launch a subprocess with LD_LIBRARY_PATH set? we really don't want that. if the subprocess depends on any library that happens to be in the same dir as libtvm.so, it will wind up loading that instead of the system-installed copy. this is a really subtle bug that is very difficult to track down. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419704899 ## File path: python/tvm/_ffi/base.py ## @@ -48,8 +48,11 @@ def _load_lib(): """Load libary by searching possible path.""" lib_path = libinfo.find_lib_path() lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_GLOBAL) -# DMatrix functions lib.TVMGetLastError.restype = ctypes.c_char_p +# Put the libpath to LD_LIBRARY_PATH +# will be useful for pipe session to find libtvm +os.environ["LD_LIBRARY_PATH"] = "%s:%s" % ( Review comment: Note that the change to LD_LIBRARY_PATH won't affert the execution after the python script exits. I still think it makes sense to make the libary path in the installation to be available for sub-processes 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419702687 ## File path: python/tvm/contrib/cc.py ## @@ -90,7 +90,8 @@ def get_target_triple(): def cross_compiler(compile_func, options=None, output_format=None, - get_target_triple=None): + get_target_triple=None, + add_files=None): Review comment: I think your understanding is correct 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419702240
##
File path: python/tvm/rpc/client.py
##
@@ -183,28 +198,37 @@ class LocalSession(RPCSession):
need to be ran both locally and remotely.
"""
def __init__(self):
-# pylint: disable=super-init-not-called
-self.context = nd.context
-self.get_function = tvm._ffi.get_global_func
-self._temp = util.tempdir()
+self._temp = server._server_env([])
+RPCSession.__init__(self, _ffi_api.LocalSession())
-def upload(self, data, target=None):
-if isinstance(data, bytearray):
-if not target:
-raise ValueError("target must present when file is a
bytearray")
-blob = data
-else:
-blob = bytearray(open(data, "rb").read())
-if not target:
-target = os.path.basename(data)
-with open(self._temp.relpath(target), "wb") as f:
-f.write(blob)
-def download(self, path):
-return bytearray(open(self._temp.relpath(path), "rb").read())
+@tvm._ffi.register_func("rpc.PopenSession")
+def _popen_session(binary):
+temp = util.tempdir()
-def load_module(self, path):
-return _load_module(self._temp.relpath(path))
+if isinstance(binary, (bytes, bytearray)):
+path_exec = temp.relpath("server.minrpc")
+with open(path_exec, "wb") as outfile:
+outfile.write(binary)
+os.chmod(path_exec, stat.S_IXUSR)
+path_exec = os.path.abspath(path_exec)
+else:
+path_exec = os.path.abspath(binary)
Review comment:
You are right that there is none atm, we could certainly skip this part.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419701703
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419699242
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419699242
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[incubator-tvm] branch v0.6 updated: Fix Canonical Simplifier for v0.6 (#5509)
This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch v0.6
in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git
The following commit(s) were added to refs/heads/v0.6 by this push:
new 60c5e48 Fix Canonical Simplifier for v0.6 (#5509)
60c5e48 is described below
commit 60c5e485f873803b9bcf5811fd212968506391fd
Author: Yao Wang
AuthorDate: Mon May 4 13:03:47 2020 -0700
Fix Canonical Simplifier for v0.6 (#5509)
---
src/arithmetic/canonical_simplify.cc | 1 +
tests/python/unittest/test_arith_canonical_simplify.py | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/arithmetic/canonical_simplify.cc
b/src/arithmetic/canonical_simplify.cc
index 1b576a6..2a05834 100644
--- a/src/arithmetic/canonical_simplify.cc
+++ b/src/arithmetic/canonical_simplify.cc
@@ -860,6 +860,7 @@ SplitModConst(SplitExpr lhs, int64_t cval, DivMode
div_mode) {
lhs->upper_factor != SplitExprNode::kPosInf) {
auto updated = ToSplitExpr(Mutate(ModImpl(
lhs->index, make_const(lhs.type(), new_upper_factor), div_mode)));
+updated.CopyOnWrite()->scale = lhs->scale;
// re-apply the lower_factor
if (lhs->lower_factor != 1) {
return SplitDivConst(updated, lhs->lower_factor, div_mode);
diff --git a/tests/python/unittest/test_arith_canonical_simplify.py
b/tests/python/unittest/test_arith_canonical_simplify.py
index 28a4388..08038ff 100644
--- a/tests/python/unittest/test_arith_canonical_simplify.py
+++ b/tests/python/unittest/test_arith_canonical_simplify.py
@@ -123,7 +123,7 @@ def test_floormod_simplify():
x, y = tvm.var("x"), tvm.var("y")
ck.verify(flm(flm((x*4) + y - 466036, 24528) - 24512, 16),
flm((x*4) + y + 12, 16))
-
+ck.verify(flm(flm((x*4), 16), 8), flm(x, 2) * 4)
def test_canonical_mixed():
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419691019
##
File path: src/runtime/rpc/rpc_endpoint.cc
##
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file rpc_session.cc
+ * \brief RPC session for remote function call.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "rpc_endpoint.h"
+#include "rpc_local_session.h"
+#include "../object_internal.h"
+#include "../../support/ring_buffer.h"
+#include "../../support/arena.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * Event-driven state-machine based handlers for RPCEndpoint.
+ *
+ * Key functions:
+ *
+ * - SendPackedSeq: send the arguments over to the peer
+ * - HandleNextEvent: handle the next request from the peer(RPCCode followed
by per code protocol).
+ */
+class RPCEndpoint::EventHandler : public dmlc::Stream {
+ public:
+ EventHandler(support::RingBuffer* reader,
+ support::RingBuffer* writer,
+ std::string name,
+ std::string* remote_key)
+ : reader_(reader),
+writer_(writer),
+name_(name),
+remote_key_(remote_key) {
+this->Clear();
+
+if (*remote_key == "%toinit") {
+ state_ = kInitHeader;
+ remote_key_->resize(0);
+ pending_request_bytes_ = sizeof(int32_t);
+}
+ }
+
+ /*!
+ * \brief Bytes needed to fulfill current request
+ */
+ size_t BytesNeeded() const {
+if (reader_->bytes_available() < pending_request_bytes_) {
+ return pending_request_bytes_ - reader_->bytes_available();
+} else {
+ return 0;
+}
+ }
+
+ /*!
+ * \brief Request number of bytes from the reader.
+ * \param nbytes The number of bytes
+ */
+ void RequestBytes(size_t nbytes) {
+pending_request_bytes_ += nbytes;
+reader_->Reserve(pending_request_bytes_);
+ }
+
+ /*! \return Whether we are ready to handle next request. */
+ bool Ready() const {
+return reader_->bytes_available() >= pending_request_bytes_;
+ }
+
+ /*! \return Whether we can perform a clean shutdown */
+ bool CanCleanShutdown() const {
+return state_ == kRecvPacketNumBytes;
+ }
+
+ /*! \brief Finish the copy ack stage. */
+ void FinishCopyAck() {
+this->SwitchToState(kRecvPacketNumBytes);
+ }
+
+ /*!
+ * \brief Enter the io loop until the next event.
+ * \param client_mode Whether we are in the client.
+ * \param setreturn The function to set the return value encoding.
+ * \return The function to set return values when there is a return event.
+ */
+ RPCCode HandleNextEvent(bool client_mode, RPCSession::FEncodeReturn
setreturn) {
+std::swap(client_mode_, client_mode);
+
+while (this->Ready()) {
+ switch (state_) {
+case kInitHeader: HandleInitHeader(); break;
+case kRecvPacketNumBytes: {
+ uint64_t packet_nbytes;
+ CHECK(this->Read(_nbytes));
+ if (packet_nbytes != 0) {
Review comment:
there is no max limit for now.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419688468
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] areusch commented on pull request #5500: Show max flops in autotvm logs.
areusch commented on pull request #5500: URL: https://github.com/apache/incubator-tvm/pull/5500#issuecomment-623670799 ok, let's close this then, because it's more user preference and it's already in a callback. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419687967
##
File path: src/runtime/rpc/minrpc/posix_popen_server.cc
##
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Disable constructor to bring minimum dep on c++ABI.
+#define TVM_ARENA_HAS_DESTRUCTOR 0
+
+#include
+#include
+#include "minrpc_server.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief IOHandler based on posix API.
+ */
+class PosixIOHandler {
Review comment:
Good point, perhaps we can add that as a followup.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419687452
##
File path: src/runtime/rpc/rpc_pipe_impl.cc
##
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file rpc_pipe_impl.cc
+ * \brief Pipe-based RPC channel.
+ */
+// Linux only for now, as linux is the most common usecase.
+#if defined(__linux__) || defined(__ANDROID__)
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include "rpc_endpoint.h"
+#include "rpc_local_session.h"
+#include "../../support/pipe.h"
+
+namespace tvm {
+namespace runtime {
+
+class PipeChannel final : public RPCChannel {
+ public:
+ explicit PipeChannel(int readfd, int writefd, pid_t child_pid)
+ : readfd_(readfd), writefd_(writefd), child_pid_(child_pid) {
+ }
+
+ ~PipeChannel() {
+Close();
+ }
+
+ size_t Send(const void* data, size_t size) final {
+ssize_t n = write(writefd_, data, size);
+if (n == -1) {
+ LOG(FATAL) << "Pipe write error";
+}
+return static_cast(n);
+ }
+
+ size_t Recv(void* data, size_t size) final {
+ssize_t n = read(readfd_, data, size);
+if (n == -1) {
+ LOG(FATAL) << "Pipe read error";
+}
+return static_cast(n);
+ }
+
+ void Close() {
+close(readfd_);
+close(writefd_);
+kill(child_pid_, SIGKILL);
+ }
+
+ private:
+ int readfd_;
+ int writefd_;
+ pid_t child_pid_;
+};
+
+
+Module CreatePipeClient(std::vector cmd) {
+ int parent2child[2];
+ int child2parent[2];
+ CHECK_EQ(pipe(parent2child), 0);
+ CHECK_EQ(pipe(child2parent), 0);
+
+ int parent_read = child2parent[0];
+ int parent_write = parent2child[1];
+ int child_read = parent2child[0];
+ int child_write = child2parent[1];
+
+ pid_t pid = fork();
+ if (pid == 0) {
+// child process
+close(parent_read);
+close(parent_write);
+std::string sread_pipe = std::to_string(child_read);
Review comment:
do you mean dup2 to stdio? My only worry is that what if the runtime do
something like print to stdout. For now most of the runtime only logs to stderr
so we might be fine. But passing in pipe number might be more robust in this
case
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419685840
##
File path: python/tvm/rpc/client.py
##
@@ -183,28 +198,37 @@ class LocalSession(RPCSession):
need to be ran both locally and remotely.
"""
def __init__(self):
-# pylint: disable=super-init-not-called
-self.context = nd.context
-self.get_function = tvm._ffi.get_global_func
-self._temp = util.tempdir()
+self._temp = server._server_env([])
+RPCSession.__init__(self, _ffi_api.LocalSession())
-def upload(self, data, target=None):
-if isinstance(data, bytearray):
-if not target:
-raise ValueError("target must present when file is a
bytearray")
-blob = data
-else:
-blob = bytearray(open(data, "rb").read())
-if not target:
-target = os.path.basename(data)
-with open(self._temp.relpath(target), "wb") as f:
-f.write(blob)
-def download(self, path):
-return bytearray(open(self._temp.relpath(path), "rb").read())
+@tvm._ffi.register_func("rpc.PopenSession")
+def _popen_session(binary):
+temp = util.tempdir()
-def load_module(self, path):
-return _load_module(self._temp.relpath(path))
+if isinstance(binary, (bytes, bytearray)):
+path_exec = temp.relpath("server.minrpc")
+with open(path_exec, "wb") as outfile:
+outfile.write(binary)
+os.chmod(path_exec, stat.S_IXUSR)
+path_exec = os.path.abspath(path_exec)
+else:
+path_exec = os.path.abspath(binary)
Review comment:
I guess i'm wondering why we would need to do that, and wondering if
it's more motivated by your rpc server test?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419685914
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419685397 ## File path: python/tvm/contrib/cc.py ## @@ -90,7 +90,8 @@ def get_target_triple(): def cross_compiler(compile_func, options=None, output_format=None, - get_target_triple=None): + get_target_triple=None, + add_files=None): Review comment: okay, I think de-coupling the APIs in A1 makes sense. if we do that though, then only autotvm (and other user scripts and any µTVM veneers) need to care about linker flags and linker scripts, correct? we can remove that complexity from `export_library`. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419685186
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
Review comment:
right now there is no maximum len 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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419684714
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419684443 ## File path: python/tvm/rpc/minrpc.py ## @@ -0,0 +1,79 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Utils to path.""" +import os +from tvm._ffi import libinfo +from tvm.contrib import cc + + +def find_minrpc_server_libpath(server="posix_popen_server"): +"""Get the path of minrpc server libary. + +Parameters +-- +server : str +The kind of built in minrpc server. + +Returns +--- +path : str +The path to the min server library. +""" +curr_dir = os.path.dirname(os.path.realpath(os.path.expanduser(__file__))) +source_dir = os.path.abspath(os.path.join(curr_dir, "..", "..", "..")) Review comment: This is temporary hack. Ideally we should ship min_server to a location (e.g. build) that get installed when we do `setup.py install` instead of relying on the availablity of the source. Or we just ship the source with the installation. When we figure that out(possibly in micro tvm link), we can have a consolidated library as in the libinfo.find_library() 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419681322
##
File path: python/tvm/rpc/server.py
##
@@ -56,7 +57,7 @@ def _server_env(load_library, work_path=None):
temp = util.tempdir()
# pylint: disable=unused-variable
-@tvm._ffi.register_func("tvm.rpc.server.workpath")
+@tvm._ffi.register_func("tvm.rpc.server.workpath", override=True)
Review comment:
okay, just documenting that I found this kwarg confusing when first
learning the code :)
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419681078
##
File path: python/tvm/rpc/client.py
##
@@ -183,28 +198,37 @@ class LocalSession(RPCSession):
need to be ran both locally and remotely.
"""
def __init__(self):
-# pylint: disable=super-init-not-called
-self.context = nd.context
-self.get_function = tvm._ffi.get_global_func
-self._temp = util.tempdir()
+self._temp = server._server_env([])
+RPCSession.__init__(self, _ffi_api.LocalSession())
-def upload(self, data, target=None):
-if isinstance(data, bytearray):
-if not target:
-raise ValueError("target must present when file is a
bytearray")
-blob = data
-else:
-blob = bytearray(open(data, "rb").read())
-if not target:
-target = os.path.basename(data)
-with open(self._temp.relpath(target), "wb") as f:
-f.write(blob)
-def download(self, path):
-return bytearray(open(self._temp.relpath(path), "rb").read())
+@tvm._ffi.register_func("rpc.PopenSession")
+def _popen_session(binary):
+temp = util.tempdir()
-def load_module(self, path):
-return _load_module(self._temp.relpath(path))
+if isinstance(binary, (bytes, bytearray)):
+path_exec = temp.relpath("server.minrpc")
+with open(path_exec, "wb") as outfile:
+outfile.write(binary)
+os.chmod(path_exec, stat.S_IXUSR)
+path_exec = os.path.abspath(path_exec)
+else:
+path_exec = os.path.abspath(binary)
Review comment:
good pt about existence check. abspath makes the execution more
consistent, e.g. if we need to chpwd in the subsequent functions
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419676108 ## File path: python/tvm/contrib/cc.py ## @@ -90,7 +90,8 @@ def get_target_triple(): def cross_compiler(compile_func, options=None, output_format=None, - get_target_triple=None): + get_target_triple=None, + add_files=None): Review comment: I think it is mainly a difference in API choices. - A0 pushing more flags to export_library - A1 using functional style programming decorator, to decorate a existing compiler by adding ldflags implicitly. Pass in fcompile variants, or use fcompile->fcompile functors to decorate new compilers. My take is that A1 is as powerful as A0, and the complexity of compiler customization goes to the construction process of fcompile. It also have the advantage of moving customization to a single place (fcompile) so that we don't have to worry about customizing the export_library step in autotvm and downstream toolchains that uses export_library. We can certainly de-couple the APIs in A1. Right now compilation and linking are coupled, and a more powerful way to construct A1 might be as follows, and we can work toward that direction. ```python fcompile = attach_linker_opts(get_compiler()) ``` 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419676108 ## File path: python/tvm/contrib/cc.py ## @@ -90,7 +90,8 @@ def get_target_triple(): def cross_compiler(compile_func, options=None, output_format=None, - get_target_triple=None): + get_target_triple=None, + add_files=None): Review comment: I think it is mainly a difference in API choices. - A0 pushing more flags to export_library - A1 using functional style programming decorator, to decorate a existing compiler by adding ldflags implicitly. Pass in fcompile variants, or use fcompile->fcompile functors to decorate new compilers. My take is that A1 is as powerful as A0, and the complexity of compiler customization goes to the construction process of fcompile. It also have the advantage of moving customization to a single point (fcompile) so that we don't have to worry about customizing the export_library step in autotvm and downstream toolchains that uses export_library. We can certainly de-couple the APIs in A1. Right now compilation and linking are coupled, and a more powerful way to construct A1 might be as follows, and we can work toward that direction. ```python fcompile = attach_linker_opts(get_compiler()) ``` 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419676682
##
File path: src/runtime/rpc/minrpc/posix_popen_server.cc
##
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Disable constructor to bring minimum dep on c++ABI.
+#define TVM_ARENA_HAS_DESTRUCTOR 0
+
+#include
+#include
+#include "minrpc_server.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief IOHandler based on posix API.
+ */
+class PosixIOHandler {
Review comment:
will there also be a flush? it would be helpful in implementing simpler
e.g. ethernet, usb, etc transports.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419676108 ## File path: python/tvm/contrib/cc.py ## @@ -90,7 +90,8 @@ def get_target_triple(): def cross_compiler(compile_func, options=None, output_format=None, - get_target_triple=None): + get_target_triple=None, + add_files=None): Review comment: I think it is mainly a difference in API: - A0 pushing more flags to export_library - A1 using functional style programming decorator, to decorate a existing compiler by adding ldflags implicitly. Pass in fcompile variants, or use fcompile->fcompile functors to decorate new compilers. My take is that A1 is as powerful as A0, and the complexity of compiler customization goes to the construction process of fcompile We can certainly de-couple the APIs in A1. Right now compilation and linking are coupled, and a more powerful way to construct A1 might be as follows, and we can work toward that direction. ```python fcompile = attach_linker_opts(get_compiler()) ``` 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419676108 ## File path: python/tvm/contrib/cc.py ## @@ -90,7 +90,8 @@ def get_target_triple(): def cross_compiler(compile_func, options=None, output_format=None, - get_target_triple=None): + get_target_triple=None, + add_files=None): Review comment: I think it is mainly a difference in API: - A0 pushing more flags to export_library - A1 using functional style programming decorator, to decorate a existing compiler by adding ldflags implicitly. Pass in fcompile variants, or use fcompile->fcompile functors to decorate new compilers. My take is that A1 is as powerful as A0, and the complexity of compiler customization goes to the decoration process. We can certainly de-couple the APIs in A1. Right now compilation and linking are coupled, and a more powerful way to construct A1 might be as follows, and we can work toward that direction. ```python fcompile = attach_linker_opts(get_compiler()) ``` 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419673923
##
File path: src/runtime/rpc/rpc_pipe_impl.cc
##
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file rpc_pipe_impl.cc
+ * \brief Pipe-based RPC channel.
+ */
+// Linux only for now, as linux is the most common usecase.
+#if defined(__linux__) || defined(__ANDROID__)
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include "rpc_endpoint.h"
+#include "rpc_local_session.h"
+#include "../../support/pipe.h"
+
+namespace tvm {
+namespace runtime {
+
+class PipeChannel final : public RPCChannel {
+ public:
+ explicit PipeChannel(int readfd, int writefd, pid_t child_pid)
+ : readfd_(readfd), writefd_(writefd), child_pid_(child_pid) {
+ }
+
+ ~PipeChannel() {
+Close();
+ }
+
+ size_t Send(const void* data, size_t size) final {
+ssize_t n = write(writefd_, data, size);
+if (n == -1) {
+ LOG(FATAL) << "Pipe write error";
+}
+return static_cast(n);
+ }
+
+ size_t Recv(void* data, size_t size) final {
+ssize_t n = read(readfd_, data, size);
+if (n == -1) {
+ LOG(FATAL) << "Pipe read error";
+}
+return static_cast(n);
+ }
+
+ void Close() {
+close(readfd_);
+close(writefd_);
+kill(child_pid_, SIGKILL);
+ }
+
+ private:
+ int readfd_;
+ int writefd_;
+ pid_t child_pid_;
+};
+
+
+Module CreatePipeClient(std::vector cmd) {
+ int parent2child[2];
+ int child2parent[2];
+ CHECK_EQ(pipe(parent2child), 0);
+ CHECK_EQ(pipe(child2parent), 0);
+
+ int parent_read = child2parent[0];
+ int parent_write = parent2child[1];
+ int child_read = parent2child[0];
+ int child_write = child2parent[1];
+
+ pid_t pid = fork();
+ if (pid == 0) {
+// child process
+close(parent_read);
+close(parent_write);
+std::string sread_pipe = std::to_string(child_read);
Review comment:
can we use dup2 here instead, or is that not portable?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419671449
##
File path: jvm/core/src/main/java/org/apache/tvm/contrib/GraphRuntime.java
##
@@ -38,53 +38,14 @@
* @return Runtime graph module that can be used to execute the graph.
*/
public static GraphModule create(String graphJson, Module libmod, TVMContext
ctx) {
-Module graphModule = null;
-if (ctx.deviceType >= RPC.RPC_SESS_MASK) {
- if (!(ctx instanceof TVMRemoteContext)) {
-throw new IllegalArgumentException(
-"Looks like you are using remote context with no RPCSession bind."
-+ "Use session.context instead.");
- }
- RPCSession rpcSession = ((TVMRemoteContext) ctx).rpcSession;
- // check arguments
- if (!"rpc".equals(libmod.typeKey())) {
-throw new IllegalArgumentException("libmod.typeKey != rpc");
- }
- final int sessIndex = (int) ((Function) reflectionStaticCall(
- RPC.class, "getApi", "_SessTableIndex"))
- .pushArg(libmod).invoke().asLong();
- if (sessIndex != (Integer) reflectionGetField(rpcSession, "tblIndex")) {
-throw new IllegalArgumentException(String.format(
-"libmod SessTableIndex=%d mismatch rpcSession.tblIndex=%d",
-sessIndex, reflectionGetField(rpcSession, "tblIndex")));
- }
-
- Function rpcModuleHandle = (Function) reflectionStaticCall(
- RPC.class, "getApi","_ModuleHandle");
- if (rpcModuleHandle == null) {
-throw new RuntimeException("Cannot find global function
tvm.rpc._ModuleHandle."
-+ "Did you compile tvm_runtime with the correct version?");
- }
-
- Function fcreate =
Function.getFunction("tvm.graph_runtime.remote_create");
- if (fcreate == null) {
-throw new RuntimeException("Cannot find global function
tvm.graph_runtime.remote_create."
-+ "Did you compile tvm_runtime with correct version?");
- }
-
- TVMValue hmod = rpcModuleHandle.pushArg(libmod).invoke();
- graphModule = fcreate.call(graphJson, hmod,
- ctx.deviceType % RPC.RPC_SESS_MASK, ctx.deviceId).asModule();
-} else {
- Function fcreate = Function.getFunction("tvm.graph_runtime.create");
- if (fcreate == null) {
-throw new RuntimeException("Cannot find global function
tvm.graph_runtime.create."
-+ "Did you compile tvm_runtime with correct version?");
- }
- graphModule = fcreate.pushArg(graphJson)
- .pushArg(libmod).pushArg(ctx.deviceType).pushArg(ctx.deviceId)
- .invoke().asModule();
+Function fcreate = Function.getFunction("tvm.graph_runtime.create");
+if (fcreate == null) {
Review comment:
give it is in the java runtime and not in the scope of this PR, will
leave as it is. There is also a debatable tradeoff (common usage vs less
abstraction)
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419660040
##
File path: src/runtime/rpc/rpc_endpoint.cc
##
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file rpc_session.cc
+ * \brief RPC session for remote function call.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "rpc_endpoint.h"
+#include "rpc_local_session.h"
+#include "../object_internal.h"
+#include "../../support/ring_buffer.h"
+#include "../../support/arena.h"
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * Event-driven state-machine based handlers for RPCEndpoint.
+ *
+ * Key functions:
+ *
+ * - SendPackedSeq: send the arguments over to the peer
+ * - HandleNextEvent: handle the next request from the peer(RPCCode followed
by per code protocol).
+ */
+class RPCEndpoint::EventHandler : public dmlc::Stream {
+ public:
+ EventHandler(support::RingBuffer* reader,
+ support::RingBuffer* writer,
+ std::string name,
+ std::string* remote_key)
+ : reader_(reader),
+writer_(writer),
+name_(name),
+remote_key_(remote_key) {
+this->Clear();
+
+if (*remote_key == "%toinit") {
+ state_ = kInitHeader;
+ remote_key_->resize(0);
+ pending_request_bytes_ = sizeof(int32_t);
+}
+ }
+
+ /*!
+ * \brief Bytes needed to fulfill current request
+ */
+ size_t BytesNeeded() const {
+if (reader_->bytes_available() < pending_request_bytes_) {
+ return pending_request_bytes_ - reader_->bytes_available();
+} else {
+ return 0;
+}
+ }
+
+ /*!
+ * \brief Request number of bytes from the reader.
+ * \param nbytes The number of bytes
+ */
+ void RequestBytes(size_t nbytes) {
+pending_request_bytes_ += nbytes;
+reader_->Reserve(pending_request_bytes_);
+ }
+
+ /*! \return Whether we are ready to handle next request. */
+ bool Ready() const {
+return reader_->bytes_available() >= pending_request_bytes_;
+ }
+
+ /*! \return Whether we can perform a clean shutdown */
+ bool CanCleanShutdown() const {
+return state_ == kRecvPacketNumBytes;
+ }
+
+ /*! \brief Finish the copy ack stage. */
+ void FinishCopyAck() {
+this->SwitchToState(kRecvPacketNumBytes);
+ }
+
+ /*!
+ * \brief Enter the io loop until the next event.
+ * \param client_mode Whether we are in the client.
+ * \param setreturn The function to set the return value encoding.
+ * \return The function to set return values when there is a return event.
+ */
+ RPCCode HandleNextEvent(bool client_mode, RPCSession::FEncodeReturn
setreturn) {
+std::swap(client_mode_, client_mode);
+
+while (this->Ready()) {
+ switch (state_) {
+case kInitHeader: HandleInitHeader(); break;
+case kRecvPacketNumBytes: {
+ uint64_t packet_nbytes;
+ CHECK(this->Read(_nbytes));
+ if (packet_nbytes != 0) {
Review comment:
check against a max?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] junrushao1994 commented on a change in pull request #5459: [Relay][Op]Support symbolic TopK, Ones, Zeros and Full
junrushao1994 commented on a change in pull request #5459:
URL: https://github.com/apache/incubator-tvm/pull/5459#discussion_r419659665
##
File path: include/tvm/relay/attrs/algorithm.h
##
@@ -50,14 +50,14 @@ struct ArgsortAttrs : public tvm::AttrsNode {
};
struct TopKAttrs : public tvm::AttrsNode {
- int k;
+ Expr k;
Review comment:
I think finally it is worthdoing to unify args and attrs, because
constant attrs are only useful when dealing with simplest CNNs.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419657264
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419655184
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419654945
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419654748
##
File path: src/runtime/rpc/minrpc/minrpc_server.h
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file minrpc_server.h
+ * \brief Minimum RPC server implementation,
+ *redirects all the calls to C runtime API.
+ *
+ * \note This file do not depend on c++ std or c std,
+ * and only depends on TVM's C runtime API.
+ */
+#ifndef TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+#define TVM_RUNTIME_RPC_MINRPC_MINRPC_SERVER_H_
+
+#include
+#include
+#include "../rpc_protocol.h"
+#include "../../../support/arena.h"
+
+/*! \brief Whether or not to enable glog style DLOG */
+#ifndef TVM_MINRPC_ENABLE_LOGGING
+#define TVM_MINRPC_ENABLE_LOGGING 0
+#endif
+
+#ifndef MINRPC_CHECK
+#define MINRPC_CHECK(cond) \
+ if (!(cond)) this->ThrowError(RPCServerStatus::kCheckError);
+#endif
+
+#if TVM_MINRPC_ENABLE_LOGGING
+#include
+#endif
+
+
+namespace tvm {
+namespace runtime {
+
+/*!
+ * \brief A minimum RPC server that only depends on the tvm C runtime..
+ *
+ * All the dependencies are provided by the io arguments.
+ *
+ * \tparam TIOHandler IO provider to provide io handling.
+ * An IOHandler needs to provide the following functions:
+ * - PosixWrite, PosixRead, Close: posix style, read, write, close API.
+ * - Exit: exit with status code.
+ */
+template
+class MinRPCServer {
+ public:
+ /*!
+ * \brief Constructor.
+ * \param io The IO handler.
+ */
+ explicit MinRPCServer(TIOHandler io)
+ : io_(io), arena_(PageAllocator(io)) {}
+
+ /*! \brief Run the server loop until shutdown signal is received. */
+ void ServerLoop() {
+RPCCode code;
+uint64_t packet_len;
+
+while (true) {
+ arena_.RecycleAll();
+ allow_clean_shutdown_ = true;
+
+ this->Read(_len);
+ if (packet_len == 0) continue;
+ this->Read();
+
+ allow_clean_shutdown_ = false;
+
+ if (code >= RPCCode::kSyscallCodeStart) {
+this->HandleSyscallFunc(code);
+ } else {
+switch (code) {
+ case RPCCode::kCallFunc: {
+HandleNormalCallFunc();
+break;
+ }
+ case RPCCode::kInitServer: {
+HandleInitServer();
+break;
+ }
+ case RPCCode::kCopyFromRemote: {
+HandleCopyFromRemote();
+break;
+ }
+ case RPCCode::kCopyToRemote: {
+HandleCopyToRemote();
+break;
+ }
+ case RPCCode::kShutdown: {
+this->Shutdown();
+return;
+ }
+ default: {
+this->ThrowError(RPCServerStatus::kUnknownRPCCode);
+break;
+ }
+}
+ }
+}
+ }
+
+ void Shutdown() {
+arena_.FreeAll();
+io_.Close();
+ }
+
+ void HandleNormalCallFunc() {
+uint64_t call_handle;
+TVMValue* values;
+int* tcodes;
+int num_args;
+TVMValue ret_value[3];
+int ret_tcode[3];
+
+this->Read(_handle);
+RecvPackedSeq(, , _args);
+
+int call_ecode = TVMFuncCall(
+reinterpret_cast(call_handle),
+values, tcodes, num_args,
+&(ret_value[1]), &(ret_tcode[1]));
+
+if (call_ecode == 0) {
+ // Return value encoding as in LocalSession
+ int rv_tcode = ret_tcode[1];
+ ret_tcode[0] = kDLInt;
+ ret_value[0].v_int64 = rv_tcode;
+ if (rv_tcode == kTVMNDArrayHandle) {
+ret_tcode[1] = kTVMDLTensorHandle;
+ret_value[2].v_handle = ret_value[1].v_handle;
+ret_tcode[2] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 3);
+ } else if (rv_tcode == kTVMPackedFuncHandle ||
+ rv_tcode == kTVMModuleHandle) {
+ret_tcode[1] = kTVMOpaqueHandle;
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ } else {
+this->ReturnPackedSeq(ret_value, ret_tcode, 2);
+ }
+} else {
+ this->ReturnLastTVMError();
+}
+ }
+
+ void HandleCopyFromRemote() {
+uint64_t handle, offset, num_bytes;
+TVMContext ctx;
+
[GitHub] [incubator-tvm] ANSHUMAN87 opened a new pull request #5510: [FRONTEND][TFLite] Fully connected op conversion made in sync with TFLite
ANSHUMAN87 opened a new pull request #5510: URL: https://github.com/apache/incubator-tvm/pull/5510 @FrozenGene : Please help review! Thank you! 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419626688 ## File path: python/tvm/contrib/cc.py ## @@ -90,7 +90,8 @@ def get_target_triple(): def cross_compiler(compile_func, options=None, output_format=None, - get_target_triple=None): + get_target_triple=None, + add_files=None): Review comment: I am not very attached to the argument though. Pehaps changig to something like `default_libs` or `add_libs` would alleviate the concern? 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419626178
##
File path: python/tvm/contrib/cc.py
##
@@ -90,7 +90,8 @@ def get_target_triple():
def cross_compiler(compile_func,
options=None,
output_format=None,
- get_target_triple=None):
+ get_target_triple=None,
+ add_files=None):
Review comment:
Indeed it is an API tradeoff. The main advantage of functor decorator
style is that it is composable.
For example, we could pass use the following to attach minserver to the ndk
compiler.
```c++
mod.export_library("xyz", tvm.rpc.with_minrpc(cc.ndk))
```
Another reason is that in the case of minrpc, we need to attach a special
property(need_system_lib) to tell the export_library to check for the
`--system-lib` option.
We might need a bit more thoughts to make the alternative API(as follows)
work.
```c++
mod.export_library("xyz", cc.ndk, libs=[tvm.rpc.minrpc()])
```
Finally, such functor decorator is easier to work with in the AutoTVM(we
just have to pass the decorated version as build_func, instead of passing an
additional argument)
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
tqchen commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419623838 ## File path: python/tvm/_ffi/base.py ## @@ -48,8 +48,11 @@ def _load_lib(): """Load libary by searching possible path.""" lib_path = libinfo.find_lib_path() lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_GLOBAL) -# DMatrix functions lib.TVMGetLastError.restype = ctypes.c_char_p +# Put the libpath to LD_LIBRARY_PATH +# will be useful for pipe session to find libtvm +os.environ["LD_LIBRARY_PATH"] = "%s:%s" % ( Review comment: Given that it makes sense to make libtvm available to subprocesses, I think we can leave it as it is. While setting rpath for the binary would work if the binary is on the same machine, it may not work after we send it through an RPC, so it might be helpful to keep the env variable. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] kazum commented on pull request #5506: [WEB][RUNTIME] TVM WebAssembly JS Runtime
kazum commented on pull request #5506: URL: https://github.com/apache/incubator-tvm/pull/5506#issuecomment-623614061 `npm run bundle` fails. Did you forget to add web/package.json? 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419619940 ## File path: python/tvm/rpc/minrpc.py ## @@ -0,0 +1,79 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Utils to path.""" +import os +from tvm._ffi import libinfo +from tvm.contrib import cc + + +def find_minrpc_server_libpath(server="posix_popen_server"): +"""Get the path of minrpc server libary. + +Parameters +-- +server : str +The kind of built in minrpc server. + +Returns +--- +path : str +The path to the min server library. +""" +curr_dir = os.path.dirname(os.path.realpath(os.path.expanduser(__file__))) +source_dir = os.path.abspath(os.path.join(curr_dir, "..", "..", "..")) Review comment: is there a util function we can use for 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419619522 ## File path: python/tvm/rpc/client.py ## @@ -397,15 +421,41 @@ def connect(url, port, key="", session_timeout=0): the connection when duration is longer than this value. When duration is zero, it means the request must always be kept alive. +session_constructor: List +List of additional arguments to passed as the remote session constructor. Review comment: well it's more like a list, the first arg is a string specifying the name of the constructor, and following args are the positional args to that function. update docstring? 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419619088 ## File path: python/tvm/rpc/client.py ## @@ -378,7 +402,7 @@ def request_and_run(self, key, max_retry, str(last_err))) -def connect(url, port, key="", session_timeout=0): +def connect(url, port, key="", session_timeout=0, session_constructor=None): Review comment: maybe we can call it session_constructor_args so it's more clear it should be a list rather than a callable? 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419618169
##
File path: python/tvm/rpc/client.py
##
@@ -183,28 +198,37 @@ class LocalSession(RPCSession):
need to be ran both locally and remotely.
"""
def __init__(self):
-# pylint: disable=super-init-not-called
-self.context = nd.context
-self.get_function = tvm._ffi.get_global_func
-self._temp = util.tempdir()
+self._temp = server._server_env([])
+RPCSession.__init__(self, _ffi_api.LocalSession())
-def upload(self, data, target=None):
-if isinstance(data, bytearray):
-if not target:
-raise ValueError("target must present when file is a
bytearray")
-blob = data
-else:
-blob = bytearray(open(data, "rb").read())
-if not target:
-target = os.path.basename(data)
-with open(self._temp.relpath(target), "wb") as f:
-f.write(blob)
-def download(self, path):
-return bytearray(open(self._temp.relpath(path), "rb").read())
+@tvm._ffi.register_func("rpc.PopenSession")
+def _popen_session(binary):
+temp = util.tempdir()
-def load_module(self, path):
-return _load_module(self._temp.relpath(path))
+if isinstance(binary, (bytes, bytearray)):
+path_exec = temp.relpath("server.minrpc")
+with open(path_exec, "wb") as outfile:
+outfile.write(binary)
+os.chmod(path_exec, stat.S_IXUSR)
+path_exec = os.path.abspath(path_exec)
+else:
+path_exec = os.path.abspath(binary)
Review comment:
maybe we could at least do a existence check and check it is an
executable file here, since otherwise i'm not sure what error the user might
see from C? also, is abspath necessary?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419617098
##
File path: python/tvm/rpc/client.py
##
@@ -183,28 +198,37 @@ class LocalSession(RPCSession):
need to be ran both locally and remotely.
"""
def __init__(self):
-# pylint: disable=super-init-not-called
-self.context = nd.context
-self.get_function = tvm._ffi.get_global_func
-self._temp = util.tempdir()
+self._temp = server._server_env([])
+RPCSession.__init__(self, _ffi_api.LocalSession())
-def upload(self, data, target=None):
-if isinstance(data, bytearray):
-if not target:
-raise ValueError("target must present when file is a
bytearray")
-blob = data
-else:
-blob = bytearray(open(data, "rb").read())
-if not target:
-target = os.path.basename(data)
-with open(self._temp.relpath(target), "wb") as f:
-f.write(blob)
-def download(self, path):
-return bytearray(open(self._temp.relpath(path), "rb").read())
+@tvm._ffi.register_func("rpc.PopenSession")
+def _popen_session(binary):
+temp = util.tempdir()
-def load_module(self, path):
-return _load_module(self._temp.relpath(path))
+if isinstance(binary, (bytes, bytearray)):
+path_exec = temp.relpath("server.minrpc")
+with open(path_exec, "wb") as outfile:
Review comment:
maybe we can log the temporary file name being created here at debug
level?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419616440
##
File path: python/tvm/rpc/client.py
##
@@ -183,28 +198,37 @@ class LocalSession(RPCSession):
need to be ran both locally and remotely.
"""
def __init__(self):
-# pylint: disable=super-init-not-called
-self.context = nd.context
-self.get_function = tvm._ffi.get_global_func
-self._temp = util.tempdir()
+self._temp = server._server_env([])
+RPCSession.__init__(self, _ffi_api.LocalSession())
-def upload(self, data, target=None):
-if isinstance(data, bytearray):
-if not target:
-raise ValueError("target must present when file is a
bytearray")
-blob = data
-else:
-blob = bytearray(open(data, "rb").read())
-if not target:
-target = os.path.basename(data)
-with open(self._temp.relpath(target), "wb") as f:
-f.write(blob)
-def download(self, path):
-return bytearray(open(self._temp.relpath(path), "rb").read())
+@tvm._ffi.register_func("rpc.PopenSession")
+def _popen_session(binary):
+temp = util.tempdir()
-def load_module(self, path):
-return _load_module(self._temp.relpath(path))
+if isinstance(binary, (bytes, bytearray)):
+path_exec = temp.relpath("server.minrpc")
+with open(path_exec, "wb") as outfile:
+outfile.write(binary)
+os.chmod(path_exec, stat.S_IXUSR)
Review comment:
TIL you can run a file you can't read. but, maybe we should make it
readable by the user too in case of e.g. segfault or whatnot?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419613421
##
File path: python/tvm/error.py
##
@@ -57,6 +57,11 @@ def __init__(self, msg):
register_error("KeyError", KeyError)
+@register_error
+class RPCError(RuntimeError):
+"""Error thrown by the RPC call."""
Review comment:
can you specify more (I.e. by the transport, by the
serializer/deserializer, by the remote server, or by the remote function)?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419613026 ## File path: python/tvm/contrib/cc.py ## @@ -90,7 +90,8 @@ def get_target_triple(): def cross_compiler(compile_func, options=None, output_format=None, - get_target_triple=None): + get_target_triple=None, + add_files=None): Review comment: this is a little bit of a weird place (compiler configuration time) to add extra files (I.e. invocation configuration time). why can't this be pushed to the invocation of fcompile? 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419611904 ## File path: python/tvm/_ffi/base.py ## @@ -48,8 +48,11 @@ def _load_lib(): """Load libary by searching possible path.""" lib_path = libinfo.find_lib_path() lib = ctypes.CDLL(lib_path[0], ctypes.RTLD_GLOBAL) -# DMatrix functions lib.TVMGetLastError.restype = ctypes.c_char_p +# Put the libpath to LD_LIBRARY_PATH +# will be useful for pipe session to find libtvm +os.environ["LD_LIBRARY_PATH"] = "%s:%s" % ( Review comment: is this just to make minrpcserver work? if so, could we just add in the search path in that binary rather than changing LD_LIBRARY_PATH for all library loads here? or at least, can you revert the change when you're done? i think this means that all hooks that invoke a subprocess inherit the modified LD_LIBRARY_PATH 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: us...@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484: URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419610442 ## File path: python/tvm/_ffi/_ctypes/packed_func.py ## @@ -141,6 +141,17 @@ def _make_tvm_args(args, temp_args): elif isinstance(arg, TVMContext): values[i].v_int64 = _ctx_to_int64(arg) type_codes[i] = TypeCode.TVM_CONTEXT +elif isinstance(arg, bytes): Review comment: do you need the separate elif block here? seems like you could just make the following block `elif isinstance(arg, (bytearray, bytes))` and collapse the logic there. ## File path: python/tvm/_ffi/_cython/packed_func.pxi ## @@ -142,6 +142,18 @@ cdef inline int make_arg(object arg, value[0].v_ctx = (( ctypes.addressof(arg)))[0] tcode[0] = kTVMContext +elif isinstance(arg, bytes): Review comment: same question here 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: us...@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #5422: [RELAY][Convert Layout] Specify additional layouts in convert layout pass
comaniac commented on a change in pull request #5422:
URL: https://github.com/apache/incubator-tvm/pull/5422#discussion_r419595355
##
File path: docs/dev/convert_layout.rst
##
@@ -116,6 +117,7 @@ These steps happen for each operator in sequence, where
ConvertLayout pass keeps
data_layout = attrs['data_layout']
kernel_layout = attrs['kernel_layout']
data, weight = inputs
+desired_layout = str(desired_layouts[0])
assert desired_layout == 'NCHW', \
"Currently only transformation to NCHW layout is supported."
if desired_layout == 'NCHW':
Review comment:
This checking seems not necessary because Line 121 already has an
assertion. Either remove this branch or move the assertion to the else branch.
##
File path: docs/dev/convert_layout.rst
##
@@ -218,24 +220,40 @@ Second example is for a lightly-layout sensitive operator
- batch normalization.
ConvertLayout pass is extremely easy to use. The pass is not a part of default
relay.build pipeline. The intended usage is to call it between the
framework-to-relay parser and relay.build module call.
+In order to specify the layouts to convert to, we create a mapping of
heavily-layout sensitive operators to a list of the desired layouts for that
operator.
+
.. code-block:: python
# TFlite framework to Relay parser - Default layout is NHWC
mod, params = relay.frontend.from_tflite(tflite_model,
shape_dict=shape_dict,
dtype_dict=dtype_dict)
+# We assume our model's heavily-layout sensitive operators only consist of
nn.conv2d
+desired_layouts = {'nn.conv2d': ['NCHW']}
+
# Convert the layout to NCHW
# RemoveUnunsedFunctions is used to clean up the graph.
seq = tvm.transform.Sequential([relay.transform.RemoveUnusedFunctions(),
- relay.transform.ConvertLayout('NCHW')])
+
relay.transform.ConvertLayout(desired_layouts)])
with relay.transform.PassContext(opt_level=3):
mod = seq(mod)
# Call relay compilation
with relay.build_config(opt_level=3):
graph, lib, params = relay.build(mod, target, params=params)
+
+The example above only considers data layout, the kernel layout is
automatically converted to one that is supported by TVM. If we wish to also
convert to a specific kernel layout this can be done like so:
Review comment:
s/, the kernel/, because the kernel.
##
File path: docs/dev/convert_layout.rst
##
@@ -116,6 +117,7 @@ These steps happen for each operator in sequence, where
ConvertLayout pass keeps
data_layout = attrs['data_layout']
kernel_layout = attrs['kernel_layout']
data, weight = inputs
+desired_layout = str(desired_layouts[0])
Review comment:
It's confusing that you only use the first layout in this list. Although
I later found the explanation in this doc, it'd be better to add 1-2 sentences
here.
##
File path: docs/dev/convert_layout.rst
##
@@ -218,24 +220,40 @@ Second example is for a lightly-layout sensitive operator
- batch normalization.
ConvertLayout pass is extremely easy to use. The pass is not a part of default
relay.build pipeline. The intended usage is to call it between the
framework-to-relay parser and relay.build module call.
+In order to specify the layouts to convert to, we create a mapping of
heavily-layout sensitive operators to a list of the desired layouts for that
operator.
+
.. code-block:: python
# TFlite framework to Relay parser - Default layout is NHWC
mod, params = relay.frontend.from_tflite(tflite_model,
shape_dict=shape_dict,
dtype_dict=dtype_dict)
+# We assume our model's heavily-layout sensitive operators only consist of
nn.conv2d
+desired_layouts = {'nn.conv2d': ['NCHW']}
+
# Convert the layout to NCHW
# RemoveUnunsedFunctions is used to clean up the graph.
seq = tvm.transform.Sequential([relay.transform.RemoveUnusedFunctions(),
- relay.transform.ConvertLayout('NCHW')])
+
relay.transform.ConvertLayout(desired_layouts)])
with relay.transform.PassContext(opt_level=3):
mod = seq(mod)
# Call relay compilation
with relay.build_config(opt_level=3):
graph, lib, params = relay.build(mod, target, params=params)
+
+The example above only considers data layout, the kernel layout is
automatically converted to one that is supported by TVM. If we wish to also
convert to a specific kernel layout this can be done like so:
+
+.. code-block:: python
+
+desired_layouts = {'nn.conv2d': ['NCHW', 'HWIO']}
+pass =
[GitHub] [incubator-tvm] areusch commented on a change in pull request #5484: [REFACTOR][RPC][PROCOTOL-CHANGE] Modularize the RPC infra
areusch commented on a change in pull request #5484:
URL: https://github.com/apache/incubator-tvm/pull/5484#discussion_r419606101
##
File path: jvm/core/src/main/java/org/apache/tvm/contrib/GraphRuntime.java
##
@@ -38,53 +38,14 @@
* @return Runtime graph module that can be used to execute the graph.
*/
public static GraphModule create(String graphJson, Module libmod, TVMContext
ctx) {
-Module graphModule = null;
-if (ctx.deviceType >= RPC.RPC_SESS_MASK) {
- if (!(ctx instanceof TVMRemoteContext)) {
-throw new IllegalArgumentException(
-"Looks like you are using remote context with no RPCSession bind."
-+ "Use session.context instead.");
- }
- RPCSession rpcSession = ((TVMRemoteContext) ctx).rpcSession;
- // check arguments
- if (!"rpc".equals(libmod.typeKey())) {
-throw new IllegalArgumentException("libmod.typeKey != rpc");
- }
- final int sessIndex = (int) ((Function) reflectionStaticCall(
- RPC.class, "getApi", "_SessTableIndex"))
- .pushArg(libmod).invoke().asLong();
- if (sessIndex != (Integer) reflectionGetField(rpcSession, "tblIndex")) {
-throw new IllegalArgumentException(String.format(
-"libmod SessTableIndex=%d mismatch rpcSession.tblIndex=%d",
-sessIndex, reflectionGetField(rpcSession, "tblIndex")));
- }
-
- Function rpcModuleHandle = (Function) reflectionStaticCall(
- RPC.class, "getApi","_ModuleHandle");
- if (rpcModuleHandle == null) {
-throw new RuntimeException("Cannot find global function
tvm.rpc._ModuleHandle."
-+ "Did you compile tvm_runtime with the correct version?");
- }
-
- Function fcreate =
Function.getFunction("tvm.graph_runtime.remote_create");
- if (fcreate == null) {
-throw new RuntimeException("Cannot find global function
tvm.graph_runtime.remote_create."
-+ "Did you compile tvm_runtime with correct version?");
- }
-
- TVMValue hmod = rpcModuleHandle.pushArg(libmod).invoke();
- graphModule = fcreate.call(graphJson, hmod,
- ctx.deviceType % RPC.RPC_SESS_MASK, ctx.deviceId).asModule();
-} else {
- Function fcreate = Function.getFunction("tvm.graph_runtime.create");
- if (fcreate == null) {
-throw new RuntimeException("Cannot find global function
tvm.graph_runtime.create."
-+ "Did you compile tvm_runtime with correct version?");
- }
- graphModule = fcreate.pushArg(graphJson)
- .pushArg(libmod).pushArg(ctx.deviceType).pushArg(ctx.deviceId)
- .invoke().asModule();
+Function fcreate = Function.getFunction("tvm.graph_runtime.create");
+if (fcreate == null) {
Review comment:
would be better to add this to a new helper function
`Function.getFunctionOrThrow()`. doesn't need to be in this PR though
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] zhiics commented on a change in pull request #5231: [POC] Pattern Language, Matcher, Rewriter, and Function Paritioner
zhiics commented on a change in pull request #5231:
URL: https://github.com/apache/incubator-tvm/pull/5231#discussion_r419596978
##
File path: include/tvm/relay/dataflow_matcher.h
##
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/relay/dataflow_matcher.h
+ * \brief A pattern matcher for matching dataflow properties.
+ */
+#ifndef TVM_RELAY_DATAFLOW_MATCHER_H_
+#define TVM_RELAY_DATAFLOW_MATCHER_H_
+
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace relay {
+
+class DFPatternCallback;
+/*!
+ * \brief Base type of all dataflow pattern callbacks.
+ * \sa DFPatternCallback
+ */
+class DFPatternCallbackNode : public Object {
+ public:
+ /*! \brief Pattern this callback matches */
+ DFPattern pattern_;
+ /*! \brief Function to call when finding a matched expression */
+ PackedFunc function_;
+
+ void VisitAttrs(tvm::AttrVisitor* v) {}
+
+ TVM_DLL static DFPatternCallback make(DFPattern pattern, PackedFunc
callback);
Review comment:
we now actually prefer constructors in ObjectRef instead of using
"make". Same to all other new nodes.
##
File path: include/tvm/relay/dataflow_pattern.h
##
@@ -0,0 +1,378 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/relay/dataflow_pattern.h
+ * \brief A pattern language for matching dataflow properties.
+ */
+#ifndef TVM_RELAY_DATAFLOW_PATTERN_H_
+#define TVM_RELAY_DATAFLOW_PATTERN_H_
+
+#include
+#include
+#include
+
+namespace tvm {
+namespace relay {
+
+/*!
+ * \brief Base type of all dataflow patterns.
+ * \sa DFPattern
+ */
+class DFPatternNode : public Object {
+ public:
+ static constexpr const char* _type_key = "DFPatternNode";
+ TVM_DECLARE_BASE_OBJECT_INFO(DFPatternNode, Object);
+};
+
+/*!
+ * \brief Managed reference to dataflow patterns.
+ * \sa DFPatternNode
+ */
+class DFPattern : public ObjectRef {
+ public:
+ TVM_DEFINE_OBJECT_REF_METHODS(DFPattern, ObjectRef, DFPatternNode);
+};
+
+/*!
+ * \brief Pattern for Relay Expression.
+ */
+class ExprPatternNode : public DFPatternNode {
+ public:
+ /*! \brief The expression to match. */
+ Expr expr;
+
+ void VisitAttrs(tvm::AttrVisitor* v) {
+v->Visit("expr", );
+ }
+
+ static constexpr const char* _type_key =
"relay.dataflow_pattern.ExprPattern";
+ TVM_DECLARE_FINAL_OBJECT_INFO(ExprPatternNode, DFPatternNode);
+};
+
+/*!
+ * \brief A pattern which matches a literal expression.
+ *
+ * \note Uses structural equality on expressions to check equality.
+ *
+ */
+class ExprPattern : public DFPattern {
+ public:
+ TVM_DLL ExprPattern(Expr expr);
+ TVM_DEFINE_OBJECT_REF_METHODS(ExprPattern, DFPattern, ExprPatternNode);
+};
+
+
+/*!
+ * \brief A Pattern to Match a Relay Variable
+ */
+class VarPattern;
+/*! \brief Container for Var */
+class VarPatternNode : public DFPatternNode {
+ public:
+ /*!
+ * \brief The name of the Var (optional).
+ */
+ std::string name;
Review comment:
Maybe we can use tvm::String instead of std::string as we are porting
std::string in node system to the former.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun opened a new pull request #5509: [BUGFIX][BACKPORT-0.6][ARITH] Fix FloorMod Simplifier
kevinthesun opened a new pull request #5509: URL: https://github.com/apache/incubator-tvm/pull/5509 @tqchen 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: us...@infra.apache.org
[GitHub] [incubator-tvm] anijain2305 commented on a change in pull request #5479: [Relay-TFLite] FP32 and Quantized Object Detection Model
anijain2305 commented on a change in pull request #5479: URL: https://github.com/apache/incubator-tvm/pull/5479#discussion_r419587225 ## File path: python/tvm/relay/frontend/tflite.py ## @@ -320,6 +321,45 @@ def dequantize(self, expr, tensor): input_zero_point=tensor.qnn_params['zero_point']) return dequantized + +def convert_qnn_fused_activation_function(self, expr, fused_activation_fn, + scale, zero_point, dtype): +"""Convert TFLite fused activation function. The expr is an input quantized tensor with +scale and zero point """ Review comment: This is needed for quantized object detection model 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: us...@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #5493: [REFACTOR][BOYC] Non recursive partitioning
comaniac commented on a change in pull request #5493:
URL: https://github.com/apache/incubator-tvm/pull/5493#discussion_r419586113
##
File path: src/relay/transforms/partition_graph.cc
##
@@ -124,37 +112,40 @@ class AnnotationChecker : public ExprVisitor {
* the compiler name.
*/
-class Partitioner : public ExprMutator {
+class Partitioner : public MixedModeMutator {
public:
explicit Partitioner(const IRModule& module) : module_(module) {
for (auto f : module->functions) {
GlobalVar f_var = f.first;
BaseFunc f_func = f.second;
- // Creating regionset per function in the module
+ // Creating regionset per function in the module.
auto region_set = AnnotatedRegionSet::Create(f_func,
partitioning::compiler_begin_op,
partitioning::compiler_end_op);
regions_sets_[region_set] = f_func;
+
+ // Initial region function metadata.
+ for (auto region : region_set) {
+region_func_meta_[region];
+ }
}
}
- Expr VisitExpr_(const CallNode* call) final {
+ Expr Rewrite_(const CallNode* call, const Expr& post) final {
auto op_node = call->op.as();
if (op_node == nullptr || call->attrs.as() == nullptr) {
- return ExprMutator::VisitExpr_(call);
+ return post;
} else if (call->op == compiler_begin_op) {
- // The annotation node is inserted on edge so it must have only one
- // argument.
+ // The annotation node is inserted on edge so it must have only one
argument.
CHECK_EQ(call->args.size(), 1U);
// Traverse the rest graph.
Expr parent = call->args[0];
- auto input_expr = VisitExpr(parent);
+ auto input_expr = Downcast(post)->args[0];
// Backtrace the parent to find the first ancestor node that is not a
begin or end op
while (const auto* parent_call = parent.as()) {
-if (parent_call->op == compiler_begin_op ||
-parent_call->op == compiler_end_op) {
+if (parent_call->op == compiler_begin_op || parent_call->op ==
compiler_end_op) {
Review comment:
It's possible that this will work as you said, but we just didn't try
it. The reasons are 1) it seems no benefit of using `post` instead of `pre`,
and 2) we just interpreted the original logic to refactor the pass to
non-recursive manner, so we can make sure it will work.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] kevinthesun commented on pull request #5467: [Relay]Improve Shape Func handling for Tuple inputs
kevinthesun commented on pull request #5467: URL: https://github.com/apache/incubator-tvm/pull/5467#issuecomment-623583616 @jroesch PTAL 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: us...@infra.apache.org
[GitHub] [incubator-tvm] mbrookhart commented on a change in pull request #5231: [POC] Pattern Language, Matcher, Rewriter, and Function Paritioner
mbrookhart commented on a change in pull request #5231:
URL: https://github.com/apache/incubator-tvm/pull/5231#discussion_r419582381
##
File path: include/tvm/relay/dataflow_functor.h
##
@@ -0,0 +1,248 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/relay/dataflow_matcher.h
+ * \brief A pattern matcher for matching dataflow properties.
+ */
+#ifndef TVM_RELAY_DATAFLOW_FUNCTOR_H_
+#define TVM_RELAY_DATAFLOW_FUNCTOR_H_
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+namespace tvm {
+namespace relay {
+
+/*!
+ * \brief A dynamical functor that dispatches on in the first DFPattern
argument.
+ *
+ * \tparam FType function signiture
+ * This type is only defined for FType with function signature R(const
DFPattern&,
+ * Args...)
+ */
+template
+class DFPatternFunctor;
+
+// functions to be overriden.
+#define DFPATTERN_FUNCTOR_DEFAULT \
+ { return VisitDFPatternDefault_(op, std::forward(args)...); }
+
+#define RELAY_DFPATTERN_FUNCTOR_DISPATCH(OP)
\
+ vtable.template set_dispatch([](const ObjectRef& n, TSelf* self, Args...
args) { \
+return self->VisitDFPattern_(static_cast(n.get()),
std::forward(args)...); \
+ });
+
+template
+class DFPatternFunctor {
+ private:
+ using TSelf = DFPatternFunctor;
+ using FType = tvm::NodeFunctor;
+
+ public:
+ /*! \brief the result type of this functor */
+ using result_type = R;
+ /*! \brief virtual destructor */
+ virtual ~DFPatternFunctor() {}
+ /*!
+ * \brief Same as call.
+ * \param n The expression node.
+ * \param args Additional arguments.
+ * \return The result of the call
+ */
+ R operator()(const DFPattern& n, Args... args) {
+return VisitDFPattern(n, std::forward(args)...);
+ }
+ /*!
+ * \brief The functor call.
+ * \param n The expression node.
+ * \param args Additional arguments.
+ * \return The result of the call
+ */
+ virtual R VisitDFPattern(const DFPattern& n, Args... args) {
+CHECK(n.defined());
+static FType vtable = InitVTable();
+return vtable(n, this, std::forward(args)...);
+ }
+ // Functions that can be overriden by subclass
+ virtual R VisitDFPattern_(const AltPatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const AttrPatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const CallPatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const DominatorPatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const ExprPatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const TupleGetItemPatternNode* op,
+Args... args) DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const TuplePatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const TypePatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const VarPatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPattern_(const WildcardPatternNode* op, Args... args)
DFPATTERN_FUNCTOR_DEFAULT;
+ virtual R VisitDFPatternDefault_(const Object* op, Args...) {
+LOG(FATAL) << "Do not have a default for " << op->GetTypeKey();
+throw;
+ }
+
+ private:
+ // initialize the vtable.
+ static FType InitVTable() {
+FType vtable;
+// Set dispatch
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(AltPatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(AttrPatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(CallPatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(DominatorPatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(ExprPatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(TupleGetItemPatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(TuplePatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(TypePatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(VarPatternNode);
+RELAY_DFPATTERN_FUNCTOR_DISPATCH(WildcardPatternNode);
+return vtable;
+ }
+};
+
+/*!
+ * \brief A simple visitor wrapper around DFPatternFunctor.
+ * Recursively visit the content.
+ *
+ *
[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
icemelon9 commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r419567958
##
File path: python/tvm/relay/op/strategy/x86.py
##
@@ -18,13 +18,17 @@
# pylint:
disable=invalid-name,unused-argument,wildcard-import,unused-wildcard-import
import logging
+import re
import topi
from tvm.te import SpecializedCondition
from .generic import *
from .. import op as _op
logger = logging.getLogger('strategy')
+_NCHWc_matcher = re.compile("^NCHW[0-9]+c$")
+_OIHWio_matcher = re.compile("^OIHW[0-9]+i[-+]?[0-9]+o$")
Review comment:
```suggestion
_OIHWio_matcher = re.compile("^OIHW[0-9]+i[0-9]+o$")
```
##
File path: python/tvm/relay/op/strategy/x86.py
##
@@ -96,6 +99,12 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
wrap_compute_conv2d(topi.x86.conv2d_nchw),
wrap_topi_schedule(topi.x86.schedule_conv2d_nchw),
name="conv2d_nchw.x86")
+if layout == "NCHW":
+assert kernel_layout == "OIHW"
+add_implementation_nchw()
+elif _NCHWc_matcher.match(layout): # check if layout is NCHWxc
+assert _OIHWio_matcher.match(kernel_layout) # check if kernel is
OIHWio
+add_implementation_nchw()
Review comment:
Here you should directly add `topi.x86.conv2d_NCHWc` since the op
already has NCHWc layout.
##
File path: python/tvm/relay/op/strategy/x86.py
##
@@ -128,6 +135,15 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
wrap_compute_conv2d(topi.nn.depthwise_conv2d_nchw),
wrap_topi_schedule(topi.generic.schedule_depthwise_conv2d_nchw),
name="depthwise_conv2d_nchw.generic")
+if layout == "NCHW":
+assert kernel_layout == "OIHW"
+channel_multiplier = get_const_tuple(inputs[1].shape)[1]
+add_implementation_depthwise_nchw(channel_multiplier)
+elif _NCHWc_matcher.match(layout): # check if layout is NCHWxc
+assert _OIHWio_matcher.match(kernel_layout) # check if kernel is
OIHWio
+kernel_shape = get_const_tuple(inputs[1].shape)
+channel_multiplier = kernel_shape[1] * kernel_shape[4]
+add_implementation_depthwise_nchw(channel_multiplier)
Review comment:
same here
##
File path: topi/python/topi/x86/conv2d_alter_op.py
##
@@ -31,6 +32,9 @@
logger = logging.getLogger('topi')
+_NCHWc_matcher = re.compile("^NCHW[0-9]+c$")
+_OIHWio_matcher = re.compile("^OIHW[0-9]+i[-+]?[0-9]+o$")
Review comment:
```suggestion
_OIHWio_matcher = re.compile("^OIHW[0-9]+i[0-9]+o$")
```
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
icemelon9 commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r419566833
##
File path: src/relay/transforms/pattern_util.h
##
@@ -270,7 +270,7 @@ inline Constant MakeConstantScalar(DataType dtype, T value)
{
*/
template
static inline Constant MakeConstantTensor(DataType dtype, std::vector
shape,
- std::vector value) {
+ T value) {
Review comment:
Could you update 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:
us...@infra.apache.org
[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5429: [RELAY][TF] Support symbolic newshape for Reshape
icemelon9 commented on a change in pull request #5429:
URL: https://github.com/apache/incubator-tvm/pull/5429#discussion_r419566554
##
File path: tests/python/relay/test_any.py
##
@@ -138,23 +138,35 @@ def test_any_concat():
result = ex.evaluate()(x_np, y_np)
tvm.testing.assert_allclose(result.asnumpy(), ref)
-def verify_any_reshape(x_shape, newshape, x_np_shape, out_shape):
+def verify_any_reshape(x_shape, newshape, x_np_shape, out_shape,
variable_newshape=False):
x = relay.var('x', shape=x_shape, dtype="float32")
+data = np.random.uniform(size=x_np_shape).astype('float32')
+params = [x]
+args = [data]
+
+if variable_newshape:
+newshape_var = relay.var('newshape', shape=(len(newshape),),
dtype='int64')
+params.append(newshape_var)
+args.append(np.array(newshape, dtype='int64'))
+newshape = newshape_var
+
y = relay.reshape(x, newshape=newshape)
mod = tvm.IRModule()
-mod["main"] = relay.Function([x], y)
-data = np.random.uniform(size=x_np_shape).astype('float32')
+mod["main"] = relay.Function(params, y)
+
for kind in ["debug", "vm"]:
ex = relay.create_executor(kind, mod=mod, ctx=tvm.cpu(), target="llvm")
-result = ex.evaluate()(data).asnumpy()
+result = ex.evaluate()(*args).asnumpy()
assert result.shape == out_shape
tvm.testing.assert_allclose(result.flatten(), data.flatten())
def test_any_reshape():
-verify_any_reshape(any_dims(3), (1, -1), (2, 3, 4), (1, 24))
-verify_any_reshape(any_dims(3), (0, -1), (2, 3, 4), (2, 12))
+for variable_newshape in [False, True]:
+# Variable newshape only supports that output rank is the same as
newshape
Review comment:
Could you also add some test to `test_op_level3`?
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] mbaret commented on a change in pull request #5479: [Relay-TFLite] FP32 and Quantized Object Detection Model
mbaret commented on a change in pull request #5479:
URL: https://github.com/apache/incubator-tvm/pull/5479#discussion_r419538743
##
File path: python/tvm/relay/frontend/tflite_flexbuffer.py
##
@@ -0,0 +1,152 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name, unused-argument, too-many-lines,
import-outside-toplevel
+"""Tensorflow lite frontend helper to parse custom options in Flexbuffer
format."""
+
+import struct
+from enum import IntEnum
+
+class BitWidth(IntEnum):
+"""Flexbuffer bit width schema from flexbuffers.h"""
+BIT_WIDTH_8 = 0
+BIT_WIDTH_16 = 1
+BIT_WIDTH_32 = 2
+BIT_WIDTH_64 = 3
+
+class FlexBufferType(IntEnum):
+"""Flexbuffer type schema from flexbuffers.h"""
+FBT_NULL = 0
+FBT_INT = 1
+FBT_UINT = 2
+FBT_FLOAT = 3
+# Types above stored inline, types below store an offset.
+FBT_KEY = 4
+FBT_STRING = 5
+FBT_INDIRECT_INT = 6
+FBT_INDIRECT_UINT = 7
+FBT_INDIRECT_FLOAT = 8
+FBT_MAP = 9
+FBT_VECTOR = 10 # Untyped.
+FBT_VECTOR_INT = 11 # Typed any size (stores no type table).
+FBT_VECTOR_UINT = 12
+FBT_VECTOR_FLOAT = 13
+FBT_VECTOR_KEY = 14
+FBT_VECTOR_STRING = 15
+FBT_VECTOR_INT2 = 16 # Typed tuple (no type table, no size field).
+FBT_VECTOR_UINT2 = 17
+FBT_VECTOR_FLOAT2 = 18
+FBT_VECTOR_INT3 = 19 # Typed triple (no type table, no size field).
+FBT_VECTOR_UINT3 = 20
+FBT_VECTOR_FLOAT3 = 21
+FBT_VECTOR_INT4 = 22 # Typed quad (no type table, no size field).
+FBT_VECTOR_UINT4 = 23
+FBT_VECTOR_FLOAT4 = 24
+FBT_BLOB = 25
+FBT_BOOL = 26
+FBT_VECTOR_BOOL = 36 # To Allow the same type of conversion of type to
vector type
+
+
+class FlexBufferDecoder(object):
+"""
+This implements partial flexbuffer deserialization to be able
+to read custom options. It is not intended to be a general
+purpose flexbuffer deserializer and as such only supports a
+limited number of types and assumes the data is a flat map.
+"""
+
+def __init__(self, buffer):
+self.buffer = buffer
+
+def indirect_jump(self, offset, byte_width):
+""" Helper function to read the offset value and jump """
+unpack_str = ""
+if byte_width == 1:
+unpack_str = "> 2)
+value_bytes = self.buffer[end + i * byte_width: end + (i + 1) *
byte_width]
+if value_type == FlexBufferType.FBT_BOOL:
+value = bool(value_bytes[0])
+elif value_type == FlexBufferType.FBT_INT:
+value = struct.unpack(" assert_equal for classes
##
File path: tests/python/frontend/tflite/test_forward.py
##
@@ -1942,6 +1957,100 @@ def test_forward_qnn_mobilenet_v3_net():
tvm.testing.assert_allclose(tvm_sorted_labels, tflite_sorted_labels)
+###
+# SSD Mobilenet
Review comment:
Either 'SSD Mobilenet Quantized' or remove the other 'SSD Mobilenet'
header.
##
File path: python/tvm/relay/frontend/tflite.py
##
@@ -320,6 +321,45 @@ def dequantize(self, expr, tensor):
input_zero_point=tensor.qnn_params['zero_point'])
return dequantized
+
+def convert_qnn_fused_activation_function(self, expr, fused_activation_fn,
+ scale, zero_point, dtype):
+"""Convert TFLite fused activation function. The expr is an input
quantized tensor with
+scale and zero point """
Review comment:
I think this could be a separate PR as it's not specific to object
detection.
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] nhynes commented on a change in pull request #5503: [RUST][RUNTIME] Fix workspace
nhynes commented on a change in pull request #5503:
URL: https://github.com/apache/incubator-tvm/pull/5503#discussion_r419536942
##
File path: rust/runtime/src/workspace.rs
##
@@ -92,9 +92,8 @@ impl WorkspacePool {
break;
}
}
-if let Some(ws_idx) = ws_idx {
-self.free.push(ws_idx);
-}
+let ws_idx = ws_idx.ok_or(format_err!("Invalid pointer"))?;
Review comment:
```suggestion
let ws_idx = ws_idx.ok_or_else(|_| format_err!("Invalid pointer"))?;
```
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:
us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #5505: [BUGFIX][BACKPORT-0.6][ARITH] Fix FloorMod Simplifier
tqchen commented on pull request #5505: URL: https://github.com/apache/incubator-tvm/pull/5505#issuecomment-623533282 @kevinthesun can you also send a PR to the 0.6 branch? Thank you! 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: us...@infra.apache.org
[GitHub] [incubator-tvm] tqchen edited a comment on pull request #5505: [ARITH][Canonical Simplifier]Fix FloorMod
tqchen edited a comment on pull request #5505: URL: https://github.com/apache/incubator-tvm/pull/5505#issuecomment-623531864 Thanks @kevinthesun I checked and it is indeed the correct fix. Thanks @junrushao1994 @Hzfengsy 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: us...@infra.apache.org
[incubator-tvm] branch master updated (0abf581 -> 6bbab4c)
This is an automated email from the ASF dual-hosted git repository. tqchen pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/incubator-tvm.git. from 0abf581 bump tophub rocm version (#5504) add 6bbab4c Fix Canonical Simplifier (#5505) No new revisions were added by this update. Summary of changes: src/arith/canonical_simplify.cc| 1 + tests/python/unittest/test_arith_canonical_simplify.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
[incubator-tvm-site] branch asf-site updated: Build at Mon May 4 07:34:27 PDT 2020
This is an automated email from the ASF dual-hosted git repository. tqchen pushed a commit to branch asf-site in repository https://gitbox.apache.org/repos/asf/incubator-tvm-site.git The following commit(s) were added to refs/heads/asf-site by this push: new 8501961 Build at Mon May 4 07:34:27 PDT 2020 8501961 is described below commit 85019610998c7297eb8b72d617306b92a0eb5511 Author: tqchen AuthorDate: Mon May 4 07:34:27 2020 -0700 Build at Mon May 4 07:34:27 PDT 2020 --- atom.xml| 2 +- community.html | 1 + images/community/edgecortix.png | Bin 0 -> 94207 bytes rss.xml | 4 ++-- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/atom.xml b/atom.xml index 775f322..6284eab 100644 --- a/atom.xml +++ b/atom.xml @@ -4,7 +4,7 @@ TVM https://tvm.apache.org; rel="self"/> https://tvm.apache.org"/> - 2020-03-30T15:47:25-07:00 + 2020-05-04T07:34:25-07:00 https://tvm.apache.org diff --git a/community.html b/community.html index 3cc31b9..768f585 100644 --- a/community.html +++ b/community.html @@ -216,6 +216,7 @@ in alphabetical order. + diff --git a/images/community/edgecortix.png b/images/community/edgecortix.png new file mode 100644 index 000..4f69d21 Binary files /dev/null and b/images/community/edgecortix.png differ diff --git a/rss.xml b/rss.xml index dc52cef..507c076 100644 --- a/rss.xml +++ b/rss.xml @@ -5,8 +5,8 @@ TVM - https://tvm.apache.org https://tvm.apache.org; rel="self" type="application/rss+xml" /> -Mon, 30 Mar 2020 15:47:25 -0700 -Mon, 30 Mar 2020 15:47:25 -0700 +Mon, 04 May 2020 07:34:25 -0700 +Mon, 04 May 2020 07:34:25 -0700 60
[incubator-tvm-site] branch master updated: add edgecortix inc to community (#6)
This is an automated email from the ASF dual-hosted git repository. tqchen pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-tvm-site.git The following commit(s) were added to refs/heads/master by this push: new c333db8 add edgecortix inc to community (#6) c333db8 is described below commit c333db891ca9d70f49b89523ac8c8c48caa6e01c Author: masahi AuthorDate: Mon May 4 23:33:52 2020 +0900 add edgecortix inc to community (#6) --- community.md| 1 + images/community/edgecortix.png | Bin 0 -> 94207 bytes 2 files changed, 1 insertion(+) diff --git a/community.md b/community.md index 7013cd4..e3ef7e1 100644 --- a/community.md +++ b/community.md @@ -76,6 +76,7 @@ in alphabetical order. + diff --git a/images/community/edgecortix.png b/images/community/edgecortix.png new file mode 100644 index 000..4f69d21 Binary files /dev/null and b/images/community/edgecortix.png differ
[GitHub] [incubator-tvm] dhruvaray opened a new pull request #5508: [TFLITE]GATHER_ND
dhruvaray opened a new pull request #5508: URL: https://github.com/apache/incubator-tvm/pull/5508 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: us...@infra.apache.org
[GitHub] [incubator-tvm] siju-samuel opened a new pull request #5507: [CRT]fix to reduce RAM size during loading model
siju-samuel opened a new pull request #5507: URL: https://github.com/apache/incubator-tvm/pull/5507 This fix will help to reduce RAM size during setup storage. I was facing issue with arduino which has only 256kb ram. For my model Setupstorage was consuming 80kb of ram and with this fix i could reduce it to 40kb. In the current code always float32 datatype is used and 4bytes is used to allocate storage pool. If runnng 8/16bit models also it will allocate 32bits. @liangfu @tmoreau89 Please help to review this code. Note: This fix can be applied to tvm runtime as well, but not very significant there. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] kparzysz-quic commented on a change in pull request #5492: [RUNTIME] Hexagon driver for offloading kernels to simulator
kparzysz-quic commented on a change in pull request #5492: URL: https://github.com/apache/incubator-tvm/pull/5492#discussion_r419427426 ## File path: src/runtime/hexagon/sim/driver/CMakeLists.txt ## @@ -0,0 +1,62 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review comment: 1. It's not just for a standalone lib/program, it's a standalone CMakeLists.txt. All other cmake files in the cmake directory are included in the main CMakeLists.txt. This one isn't. 2. The driver has to be compiled with hexagon-clang. That doesn't correlate with any other compilation that uses hexagon-clang (libtvm_runtime.so needs to be compiled for Hexagon in both cases, when using a simulator and a hardware device), so it shouldn't really be included in the main CMakeFile.txt. I was planning to add it as a dependency to the main CMakeLists.txt after it's checked in. This would remove the need for manual running. 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: us...@infra.apache.org
[GitHub] [incubator-tvm] lhutton1 commented on pull request #5422: [RELAY][Convert Layout] Specify additional layouts in convert layout pass
lhutton1 commented on pull request #5422: URL: https://github.com/apache/incubator-tvm/pull/5422#issuecomment-623353852 @anijain2305 could you take another look when you have time? 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: us...@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm commented on a change in pull request #5493: [REFACTOR][BOYC] Non recursive partitioning
manupa-arm commented on a change in pull request #5493:
URL: https://github.com/apache/incubator-tvm/pull/5493#discussion_r419281724
##
File path: src/relay/transforms/partition_graph.cc
##
@@ -124,37 +112,40 @@ class AnnotationChecker : public ExprVisitor {
* the compiler name.
*/
-class Partitioner : public ExprMutator {
+class Partitioner : public MixedModeMutator {
public:
explicit Partitioner(const IRModule& module) : module_(module) {
for (auto f : module->functions) {
GlobalVar f_var = f.first;
BaseFunc f_func = f.second;
- // Creating regionset per function in the module
+ // Creating regionset per function in the module.
auto region_set = AnnotatedRegionSet::Create(f_func,
partitioning::compiler_begin_op,
partitioning::compiler_end_op);
regions_sets_[region_set] = f_func;
+
+ // Initial region function metadata.
+ for (auto region : region_set) {
+region_func_meta_[region];
+ }
}
}
- Expr VisitExpr_(const CallNode* call) final {
+ Expr Rewrite_(const CallNode* call, const Expr& post) final {
auto op_node = call->op.as();
if (op_node == nullptr || call->attrs.as() == nullptr) {
- return ExprMutator::VisitExpr_(call);
+ return post;
} else if (call->op == compiler_begin_op) {
- // The annotation node is inserted on edge so it must have only one
- // argument.
+ // The annotation node is inserted on edge so it must have only one
argument.
CHECK_EQ(call->args.size(), 1U);
// Traverse the rest graph.
Expr parent = call->args[0];
- auto input_expr = VisitExpr(parent);
+ auto input_expr = Downcast(post)->args[0];
// Backtrace the parent to find the first ancestor node that is not a
begin or end op
while (const auto* parent_call = parent.as()) {
-if (parent_call->op == compiler_begin_op ||
-parent_call->op == compiler_end_op) {
+if (parent_call->op == compiler_begin_op || parent_call->op ==
compiler_end_op) {
Review comment:
True, but it does not violate topological ordering. I think only
irregularity is that when the first output of a region is encountered --> all
the inputs of the region would be traversed, as opposed to a typical pass where
only the sensitive inputs to that particular output is traversed. Just curious,
have you seen any failures if you have just used post ?
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:
us...@infra.apache.org
