You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/02/22 05:55:35 UTC
[GitHub] [tvm] ZihengJiang opened a new pull request #7488: [Runtime] Special Memory Scope Support
ZihengJiang opened a new pull request #7488:
URL: https://github.com/apache/tvm/pull/7488
In current TVM system, we categorize the machine's memory into serval types: `global`, `shared`, etc. Based on this, we implicitly allocate and use those memory with the scheduling primitives `cache_read` / `cache_write` . However, caching still requires explicit movement during launch of every kernel, it can be beneficial to allocate these special memory explicitly during runtime so they can be reused across operators. Apart from this, TVM assumes that all the memory objects can be manipulated with a plain pointer (`void*`). The main limitations of the current runtime are:
- L0: Only support one kind of memory scope(global on-chip DRAM).
- L1: Assumes memory as flat 1D access, cannot support special multi-dimensional layout memory(e.g. OpenCL's texture memory).
This PR bings changes in the TVM runtime to support special memory.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583149281
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -90,6 +90,17 @@ class TVM_DLL DeviceAPI {
*/
virtual void* AllocDataSpace(TVMContext ctx, size_t nbytes, size_t alignment,
DLDataType type_hint) = 0;
+ /*!
+ * \brief Allocate a data space on device with memory scope support.
+ * \param ctx The device context to perform operation.
+ * \param ndim The number of dimension of allocated tensor.
+ * \param shape The shape of allocated tensor.
+ * \param dtype The type of elements.
+ * \param mem_scope The memory scope of allocated tensor.
+ * \return The allocated device pointer.
+ */
+ virtual void* AllocDataSpace(TVMContext ctx, int ndim, const int64_t* shape, DLDataType dtype,
+ Optional<String> mem_scope = NullOpt);
Review comment:
The present as string from the interface level because different device comes with different scoping convention. Internally they are mapped to enum at each DeviceAPI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r582861877
##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -253,42 +254,41 @@ def numpyasarray(np_data):
return arr, shape
-def empty(shape, dtype="float32", ctx=context(1, 0)):
+def empty(shape, dtype="float32", ctx=context(1, 0), mem_scope=None):
"""Create an empty array given shape and device
Parameters
----------
shape : tuple of int
- The shape of the array
+ The shape of the array.
dtype : type or str
The data type of the array.
ctx : TVMContext
- The context of the array
+ The context of the array.
+
+ mem_scope : str
Review comment:
Optional[str]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r584166158
##########
File path: src/runtime/crt/common/crt_runtime_api.c
##########
@@ -106,8 +106,19 @@ int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, const int64_t* sha
int TVMDeviceFreeDataSpace(TVMContext ctx, void* ptr) { return TVMPlatformMemoryFree(ptr, ctx); }
+static bool IsContiguous(const DLTensor* arr) {
Review comment:
bool may not be a standard C type, please try to stay within the c language
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583158464
##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -253,42 +254,35 @@ def numpyasarray(np_data):
return arr, shape
-def empty(shape, dtype="float32", ctx=context(1, 0)):
+def empty(shape, dtype="float32", ctx=context(1, 0), mem_scope=None):
"""Create an empty array given shape and device
Parameters
----------
shape : tuple of int
- The shape of the array
+ The shape of the array.
dtype : type or str
The data type of the array.
ctx : TVMContext
- The context of the array
+ The context of the array.
+
+ mem_scope : str
+ The memory scope of the array.
Returns
-------
arr : tvm.nd.NDArray
The array tvm supported.
"""
- shape = c_array(tvm_shape_index_t, shape)
- ndim = ctypes.c_int(len(shape))
- handle = TVMArrayHandle()
+ arr = np.array(shape, 'int64')
+ ptr = arr.ctypes.data_as(ctypes.POINTER(ctypes.c_int64))
+ shape_ptr = ctypes.cast(ptr, ctypes.c_void_p)
+ ndim = len(shape)
dtype = DataType(dtype)
- check_call(
- _LIB.TVMArrayAlloc(
- shape,
- ndim,
- ctypes.c_int(dtype.type_code),
- ctypes.c_int(dtype.bits),
- ctypes.c_int(dtype.lanes),
- ctx.device_type,
- ctx.device_id,
- ctypes.byref(handle),
- )
- )
- return _make_array(handle, False, False)
+ arr = _ffi_api.TVMArrayAllocWithScope(shape_ptr, ndim, dtype, ctx, mem_scope)
Review comment:
We felt that a C API change is a bigger one that would need more thoughts(e.g. delete the API or upgrade). using a FFI function defers that to a future point without breaking backward compact
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583992985
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -144,6 +144,48 @@ void* DeviceAPI::AllocWorkspace(TVMContext ctx, size_t size, DLDataType type_hin
return AllocDataSpace(ctx, size, kTempAllocaAlignment, type_hint);
}
+static size_t GetDataAlignment(const DLDataType dtype) {
+ size_t align = (dtype.bits / 8) * dtype.lanes;
+ if (align < kAllocAlignment) return kAllocAlignment;
+ return align;
+}
+
+void* DeviceAPI::AllocDataSpace(TVMContext ctx, int ndim, const int64_t* shape, DLDataType dtype,
+ Optional<String> mem_scope) {
+ if (!mem_scope.defined() || mem_scope.value() == "global") {
+ // by default, we can always redirect to the flat memory allocations
+ DLTensor temp;
+ temp.data = nullptr;
+ temp.ctx = ctx;
+ temp.ndim = ndim;
+ temp.dtype = dtype;
+ temp.shape = const_cast<int64_t*>(shape);
+ temp.strides = nullptr;
+ temp.byte_offset = 0;
+ size_t size = GetDataSize(temp);
+ size_t alignment = GetDataAlignment(temp.dtype);
+ return AllocDataSpace(ctx, size, alignment, dtype);
+ }
+ LOG(FATAL) << "Device does not support allocate data space with "
+ << "specified memory scope: " << mem_scope.value();
+ return nullptr;
+}
+
+void DeviceAPI::CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream) {
+ // by default, we can always redirect to the flat memory copy operation.
+ size_t nbytes = GetDataSize(*from);
+ ICHECK_EQ(nbytes, GetDataSize(*to));
+ ICHECK(from->strides == nullptr && to->strides == nullptr);
Review comment:
Not sure...Do we want to CHECK or ICHECK here?
##########
File path: src/runtime/crt/common/crt_runtime_api.c
##########
@@ -87,16 +87,33 @@ int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment, DLDa
if (alignment != 1) {
nbytes = (nbytes + alignment - 1) / alignment * alignment;
}
-
return TVMPlatformMemoryAllocate(nbytes, ctx, out_data);
}
+int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, const int64_t* shape,
+ DLDataType dtype, const char* mem_scope, void** out_data) {
+ size_t nbytes = 1;
+ for (int i = 0; i < ndim; ++i) {
+ nbytes *= shape[i];
+ }
+ nbytes *= (dtype.bits * dtype.lanes + 7) / 8;
+
+ int kAllocAlignment = 128;
Review comment:
nitpick: let's mark it as constexpr
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583167628
##########
File path: src/runtime/minrpc/minrpc_server.h
##########
@@ -209,30 +220,39 @@ class MinRPCServer {
}
void HandleCopyToRemote() {
- uint64_t handle, offset, num_bytes;
- TVMContext ctx;
- DLDataType type_hint;
-
- this->Read(&handle);
- this->Read(&offset);
+ DLTensor* arr = this->ArenaAlloc<DLTensor>(1);
+ uint64_t data_handle;
+ this->Read(&data_handle);
+ arr->data = reinterpret_cast<void*>(data_handle);
+ this->Read(&(arr->ctx));
+ this->Read(&(arr->ndim));
+ this->Read(&(arr->dtype));
+ arr->shape = this->ArenaAlloc<int64_t>(arr->ndim);
+ this->ReadArray(arr->shape, arr->ndim);
+ arr->strides = nullptr;
+ this->Read(&(arr->byte_offset));
+ uint64_t num_bytes;
this->Read(&num_bytes);
- this->Read(&ctx);
- this->Read(&type_hint);
- int call_ecode = 0;
- if (ctx.device_type == kDLCPU) {
- uint8_t* dptr = reinterpret_cast<uint8_t*>(handle) + offset;
+ int call_ecode = 0;
+ if (arr->ctx.device_type == kDLCPU) {
+ uint8_t* dptr = reinterpret_cast<uint8_t*>(data_handle) + arr->byte_offset;
this->ReadArray(dptr, num_bytes);
} else {
uint8_t* temp_data = this->ArenaAlloc<uint8_t>(num_bytes);
this->ReadArray(temp_data, num_bytes);
-
- call_ecode =
- TVMDeviceCopyDataFromTo(temp_data, 0, reinterpret_cast<void*>(handle), offset, num_bytes,
- DLContext{kDLCPU, 0}, ctx, type_hint, nullptr);
+ DLTensor temp;
+ temp.data = temp_data;
+ temp.ctx = DLContext{kDLCPU, 0};
+ temp.ndim = arr->ndim;
+ temp.dtype = arr->dtype;
+ temp.shape = arr->shape;
+ temp.strides = nullptr;
+ temp.byte_offset = 0;
Review comment:
Because most cases we have to pass in most positional arguments. If there are a lot of defaults then i agree a helper would be good, directly constructing also has the benefit of being explicit about field name like kw arguments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#issuecomment-786655409
Thanks @csullivan , please take another look and https://tvm.apache.org/docs/contribute/code_review.html?highlight=approve%20changes#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:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r581027219
##########
File path: include/tvm/runtime/c_runtime_api.h
##########
@@ -559,6 +559,20 @@ TVM_DLL int TVMByteArrayFree(TVMByteArray* arr);
TVM_DLL int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment,
DLDataType type_hint, void** out_data);
+/*!
+ * \brief Allocate a data space on device.
Review comment:
```
Allocate a data space on device with special memory scope.
\note The memory could use a special multi-dimensional memory layout. That is why we pass shape,
and dtype instead of raw number of bytes.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583998009
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -144,6 +144,48 @@ void* DeviceAPI::AllocWorkspace(TVMContext ctx, size_t size, DLDataType type_hin
return AllocDataSpace(ctx, size, kTempAllocaAlignment, type_hint);
}
+static size_t GetDataAlignment(const DLDataType dtype) {
+ size_t align = (dtype.bits / 8) * dtype.lanes;
+ if (align < kAllocAlignment) return kAllocAlignment;
+ return align;
+}
+
+void* DeviceAPI::AllocDataSpace(TVMContext ctx, int ndim, const int64_t* shape, DLDataType dtype,
+ Optional<String> mem_scope) {
+ if (!mem_scope.defined() || mem_scope.value() == "global") {
+ // by default, we can always redirect to the flat memory allocations
+ DLTensor temp;
+ temp.data = nullptr;
+ temp.ctx = ctx;
+ temp.ndim = ndim;
+ temp.dtype = dtype;
+ temp.shape = const_cast<int64_t*>(shape);
+ temp.strides = nullptr;
+ temp.byte_offset = 0;
+ size_t size = GetDataSize(temp);
+ size_t alignment = GetDataAlignment(temp.dtype);
+ return AllocDataSpace(ctx, size, alignment, dtype);
+ }
+ LOG(FATAL) << "Device does not support allocate data space with "
+ << "specified memory scope: " << mem_scope.value();
+ return nullptr;
+}
+
+void DeviceAPI::CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream) {
+ // by default, we can always redirect to the flat memory copy operation.
+ size_t nbytes = GetDataSize(*from);
+ ICHECK_EQ(nbytes, GetDataSize(*to));
+ ICHECK(from->strides == nullptr && to->strides == nullptr);
Review comment:
Good catch, this is a ICHECK case. @ZihengJiang actually we need to do IsContiguous check, since we might want to copy data that contains strides but is contiguous.
Please also double check other cases when the DLTensor can be constructed from NDArray
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r581183745
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -150,15 +150,16 @@ static size_t GetDataAlignment(const DLDataType dtype) {
return align;
}
-void* DeviceAPI::AllocDataSpace(TVMContext ctx, std::vector<int64_t> shape, DLDataType dtype,
- Optional<String> mem_scope) {
+void* DeviceAPI::AllocDataSpace(TVMContext ctx, int ndim, int64_t* shape,
Review comment:
use const int64_t*
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583999764
##########
File path: src/runtime/crt/common/crt_runtime_api.c
##########
@@ -87,16 +87,33 @@ int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment, DLDa
if (alignment != 1) {
nbytes = (nbytes + alignment - 1) / alignment * alignment;
}
-
return TVMPlatformMemoryAllocate(nbytes, ctx, out_data);
}
+int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, const int64_t* shape,
+ DLDataType dtype, const char* mem_scope, void** out_data) {
+ size_t nbytes = 1;
+ for (int i = 0; i < ndim; ++i) {
+ nbytes *= shape[i];
+ }
+ nbytes *= (dtype.bits * dtype.lanes + 7) / 8;
+
+ int kAllocAlignment = 128;
Review comment:
I see. Didn't notice that it is in C. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen merged pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #7488:
URL: https://github.com/apache/tvm/pull/7488
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r581183745
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -150,15 +150,16 @@ static size_t GetDataAlignment(const DLDataType dtype) {
return align;
}
-void* DeviceAPI::AllocDataSpace(TVMContext ctx, std::vector<int64_t> shape, DLDataType dtype,
- Optional<String> mem_scope) {
+void* DeviceAPI::AllocDataSpace(TVMContext ctx, int ndim, int64_t* shape,
Review comment:
use const int64_t* (can const cast to DLTensor->shape internally)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ZihengJiang commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
ZihengJiang commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583282138
##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -253,42 +254,35 @@ def numpyasarray(np_data):
return arr, shape
-def empty(shape, dtype="float32", ctx=context(1, 0)):
+def empty(shape, dtype="float32", ctx=context(1, 0), mem_scope=None):
"""Create an empty array given shape and device
Parameters
----------
shape : tuple of int
- The shape of the array
+ The shape of the array.
dtype : type or str
The data type of the array.
ctx : TVMContext
- The context of the array
+ The context of the array.
+
+ mem_scope : str
+ The memory scope of the array.
Returns
-------
arr : tvm.nd.NDArray
The array tvm supported.
"""
- shape = c_array(tvm_shape_index_t, shape)
- ndim = ctypes.c_int(len(shape))
- handle = TVMArrayHandle()
+ arr = np.array(shape, 'int64')
+ ptr = arr.ctypes.data_as(ctypes.POINTER(ctypes.c_int64))
+ shape_ptr = ctypes.cast(ptr, ctypes.c_void_p)
+ ndim = len(shape)
dtype = DataType(dtype)
- check_call(
- _LIB.TVMArrayAlloc(
- shape,
- ndim,
- ctypes.c_int(dtype.type_code),
- ctypes.c_int(dtype.bits),
- ctypes.c_int(dtype.lanes),
- ctx.device_type,
- ctx.device_id,
- ctypes.byref(handle),
- )
- )
- return _make_array(handle, False, False)
+ arr = _ffi_api.TVMArrayAllocWithScope(shape_ptr, ndim, dtype, ctx, mem_scope)
Review comment:
We can upgrade this to CAPI after it is stable
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r584127204
##########
File path: src/runtime/crt/common/crt_runtime_api.c
##########
@@ -87,16 +87,33 @@ int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment, DLDa
if (alignment != 1) {
nbytes = (nbytes + alignment - 1) / alignment * alignment;
}
-
return TVMPlatformMemoryAllocate(nbytes, ctx, out_data);
}
+int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, const int64_t* shape,
+ DLDataType dtype, const char* mem_scope, void** out_data) {
+ size_t nbytes = 1;
+ for (int i = 0; i < ndim; ++i) {
+ nbytes *= shape[i];
+ }
+ nbytes *= (dtype.bits * dtype.lanes + 7) / 8;
+
+ int kAllocAlignment = 128;
+ size_t align = (dtype.bits / 8) * dtype.lanes;
+ if (align < kAllocAlignment) align = kAllocAlignment;
+ return TVMDeviceAllocDataSpace(ctx, nbytes, align, dtype, out_data);
+}
+
int TVMDeviceFreeDataSpace(TVMContext ctx, void* ptr) { return TVMPlatformMemoryFree(ptr, ctx); }
-int TVMDeviceCopyDataFromTo(const void* from, size_t from_offset, void* to, size_t to_offset,
- size_t num_bytes, TVMContext ctx_from, TVMContext ctx_to,
- DLDataType type_hint, TVMStreamHandle stream) {
- memcpy(((uint8_t*)to) + to_offset, ((uint8_t*)from) + from_offset, num_bytes);
+int TVMDeviceCopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream) {
+ assert(from->strides == NULL && to->strides == NULL);
Review comment:
update this as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583998009
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -144,6 +144,48 @@ void* DeviceAPI::AllocWorkspace(TVMContext ctx, size_t size, DLDataType type_hin
return AllocDataSpace(ctx, size, kTempAllocaAlignment, type_hint);
}
+static size_t GetDataAlignment(const DLDataType dtype) {
+ size_t align = (dtype.bits / 8) * dtype.lanes;
+ if (align < kAllocAlignment) return kAllocAlignment;
+ return align;
+}
+
+void* DeviceAPI::AllocDataSpace(TVMContext ctx, int ndim, const int64_t* shape, DLDataType dtype,
+ Optional<String> mem_scope) {
+ if (!mem_scope.defined() || mem_scope.value() == "global") {
+ // by default, we can always redirect to the flat memory allocations
+ DLTensor temp;
+ temp.data = nullptr;
+ temp.ctx = ctx;
+ temp.ndim = ndim;
+ temp.dtype = dtype;
+ temp.shape = const_cast<int64_t*>(shape);
+ temp.strides = nullptr;
+ temp.byte_offset = 0;
+ size_t size = GetDataSize(temp);
+ size_t alignment = GetDataAlignment(temp.dtype);
+ return AllocDataSpace(ctx, size, alignment, dtype);
+ }
+ LOG(FATAL) << "Device does not support allocate data space with "
+ << "specified memory scope: " << mem_scope.value();
+ return nullptr;
+}
+
+void DeviceAPI::CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream) {
+ // by default, we can always redirect to the flat memory copy operation.
+ size_t nbytes = GetDataSize(*from);
+ ICHECK_EQ(nbytes, GetDataSize(*to));
+ ICHECK(from->strides == nullptr && to->strides == nullptr);
Review comment:
Good catch, @ZihengJiang actually we need to do IsContiguous check, since we might want to copy data that contains strides but is contiguous.
Please also double check other cases when the DLTensor can be constructed from NDArray
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r584166167
##########
File path: src/runtime/crt/common/crt_runtime_api.c
##########
@@ -106,8 +106,19 @@ int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, const int64_t* sha
int TVMDeviceFreeDataSpace(TVMContext ctx, void* ptr) { return TVMPlatformMemoryFree(ptr, ctx); }
+static bool IsContiguous(const DLTensor* arr) {
+ if (arr->strides == nullptr) return true;
Review comment:
nullptr is not recognized by c
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583996857
##########
File path: src/runtime/crt/common/crt_runtime_api.c
##########
@@ -87,16 +87,33 @@ int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment, DLDa
if (alignment != 1) {
nbytes = (nbytes + alignment - 1) / alignment * alignment;
}
-
return TVMPlatformMemoryAllocate(nbytes, ctx, out_data);
}
+int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, const int64_t* shape,
+ DLDataType dtype, const char* mem_scope, void** out_data) {
+ size_t nbytes = 1;
+ for (int i = 0; i < ndim; ++i) {
+ nbytes *= shape[i];
+ }
+ nbytes *= (dtype.bits * dtype.lanes + 7) / 8;
+
+ int kAllocAlignment = 128;
Review comment:
constexpr is not a C keyword, so we might need to keep it as it is
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r581040168
##########
File path: src/runtime/rpc/rpc_device_api.cc
##########
@@ -65,30 +77,41 @@ class RPCDeviceAPI final : public DeviceAPI {
}
delete space;
}
- void CopyDataFromTo(const void* from, size_t from_offset, void* to, size_t to_offset, size_t size,
- TVMContext ctx_from, TVMContext ctx_to, DLDataType type_hint,
- TVMStreamHandle stream) final {
+
+ void CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream) final {
+ DLContext ctx_from = from->ctx;
+ DLContext ctx_to = to->ctx;
if (IsRPCSessionContext(ctx_from) && IsRPCSessionContext(ctx_to)) {
ICHECK(ctx_from.device_type == ctx_to.device_type)
<< "Cannot copy across two different remote session";
auto remote_ctx_from = RemoveRPCSessionMask(ctx_from);
auto remote_ctx_to = RemoveRPCSessionMask(ctx_to);
+ void* from_data = static_cast<const RemoteSpace*>(from->data)->data;
+ void* to_data = static_cast<const RemoteSpace*>(to->data)->data;
+ DLTensor from_tensor{from_data, remote_ctx_from, from->ndim, from->dtype,
Review comment:
DLTensor from_tensor = *from;
from_tensor.ctx = RemoveRPCSessionMask(ctx_from);
from_tensor.data = static_cast<const RemoteSpace*>(from->data)->data;
##########
File path: src/runtime/rpc/rpc_endpoint.cc
##########
@@ -904,32 +883,41 @@ void RPCDevAllocData(RPCSession* handler, TVMArgs args, TVMRetValue* rv) {
*rv = data;
}
+void RPCDevAllocDataWithScope(RPCSession* handler, TVMArgs args, TVMRetValue* rv) {
+ DLTensor* arr = args[0];
+ TVMContext ctx = arr->ctx;
+ std::vector<int64_t> shape(arr->shape, arr->shape + arr->ndim);
+ DLDataType dtype = arr->dtype;
+ int tcode = args[1].type_code();
+ Optional<String> mem_scope = NullOpt;
+ if (tcode == kTVMStr) {
+ mem_scope = args[1].operator std::string();
Review comment:
consider `operator String()`(although std::string also works)
##########
File path: src/runtime/rpc/rpc_endpoint.cc
##########
@@ -815,51 +798,47 @@ void RPCEndpoint::CallFunc(RPCSession::PackedFuncHandle h, const TVMValue* arg_v
ICHECK(code == RPCCode::kReturn) << "code=" << static_cast<int>(code);
}
-void RPCEndpoint::CopyToRemote(void* from, size_t from_offset, void* to, size_t to_offset,
- size_t data_size, TVMContext ctx_to, DLDataType type_hint) {
+void RPCEndpoint::CopyToRemote(void* from_bytes, DLTensor* to, uint64_t nbytes) {
std::lock_guard<std::mutex> lock(mutex_);
RPCCode code = RPCCode::kCopyToRemote;
- uint64_t handle = reinterpret_cast<uint64_t>(to);
- uint64_t offset = static_cast<uint64_t>(to_offset);
- uint64_t size = static_cast<uint64_t>(data_size);
- uint64_t packet_nbytes = sizeof(code) + sizeof(handle) + sizeof(offset) + sizeof(size) +
- sizeof(ctx_to) + sizeof(type_hint) + data_size;
+ uint64_t num_data_bytes = static_cast<uint64_t>(GetDataSize(*to));
+ ICHECK_EQ(nbytes, num_data_bytes);
+
+ uint64_t to_data = reinterpret_cast<uint64_t>(to->data);
+ uint64_t num_shape_bytes = to->ndim * sizeof(int64_t);
Review comment:
shape_bytes
##########
File path: src/runtime/rpc/rpc_endpoint.cc
##########
@@ -815,51 +798,47 @@ void RPCEndpoint::CallFunc(RPCSession::PackedFuncHandle h, const TVMValue* arg_v
ICHECK(code == RPCCode::kReturn) << "code=" << static_cast<int>(code);
}
-void RPCEndpoint::CopyToRemote(void* from, size_t from_offset, void* to, size_t to_offset,
- size_t data_size, TVMContext ctx_to, DLDataType type_hint) {
+void RPCEndpoint::CopyToRemote(void* from_bytes, DLTensor* to, uint64_t nbytes) {
std::lock_guard<std::mutex> lock(mutex_);
RPCCode code = RPCCode::kCopyToRemote;
- uint64_t handle = reinterpret_cast<uint64_t>(to);
- uint64_t offset = static_cast<uint64_t>(to_offset);
- uint64_t size = static_cast<uint64_t>(data_size);
- uint64_t packet_nbytes = sizeof(code) + sizeof(handle) + sizeof(offset) + sizeof(size) +
- sizeof(ctx_to) + sizeof(type_hint) + data_size;
+ uint64_t num_data_bytes = static_cast<uint64_t>(GetDataSize(*to));
+ ICHECK_EQ(nbytes, num_data_bytes);
+
+ uint64_t to_data = reinterpret_cast<uint64_t>(to->data);
+ uint64_t num_shape_bytes = to->ndim * sizeof(int64_t);
+ uint64_t packet_nbytes = sizeof(code) + sizeof(to_data) + sizeof(to->ctx) + sizeof(to->ndim) +
+ sizeof(to->dtype) + sizeof(to->byte_offset) + num_shape_bytes +
+ sizeof(nbytes) + num_data_bytes;
handler_->Write(packet_nbytes);
handler_->Write(code);
- handler_->Write(handle);
- handler_->Write(offset);
- handler_->Write(size);
- handler_->Write(ctx_to);
- handler_->Write(type_hint);
- handler_->WriteArray(reinterpret_cast<char*>(from) + from_offset, data_size);
-
+ RPCReference::SendDLTensor(handler_, to);
+ handler_->Write(nbytes);
+ handler_->WriteArray(reinterpret_cast<char*>(from_bytes), nbytes);
ICHECK(HandleUntilReturnEvent(true, [](TVMArgs) {}) == RPCCode::kReturn);
}
-void RPCEndpoint::CopyFromRemote(void* from, size_t from_offset, void* to, size_t to_offset,
- size_t data_size, TVMContext ctx_from, DLDataType type_hint) {
+void RPCEndpoint::CopyFromRemote(DLTensor* from, void* to_bytes, uint64_t nbytes) {
std::lock_guard<std::mutex> lock(mutex_);
RPCCode code = RPCCode::kCopyFromRemote;
- uint64_t handle = reinterpret_cast<uint64_t>(from);
- uint64_t offset = static_cast<uint64_t>(from_offset);
- uint64_t size = static_cast<uint64_t>(data_size);
- uint64_t packet_nbytes = sizeof(code) + sizeof(handle) + sizeof(offset) + sizeof(size) +
- sizeof(ctx_from) + sizeof(type_hint);
+ uint64_t num_data_bytes = static_cast<uint64_t>(GetDataSize(*from));
+ CHECK_EQ(nbytes, num_data_bytes);
+
+ uint64_t from_data = reinterpret_cast<uint64_t>(from->data);
+ uint64_t num_shape_bytes = from->ndim * sizeof(int64_t);
Review comment:
shape_bytes
##########
File path: src/runtime/rpc/rpc_endpoint.h
##########
@@ -135,8 +135,7 @@ class RPCEndpoint {
* \param ctx_to The target context.
* \param type_hint Hint of content data type.
*/
- void CopyToRemote(void* from, size_t from_offset, void* to, size_t to_offset, size_t nbytes,
- TVMContext ctx_to, DLDataType type_hint);
+ void CopyToRemote(void* from_bytes, DLTensor* to, uint64_t nbytes);
Review comment:
consistency: either use size_t nbytes or uint64_t nbytes everywhere (RPCSession and RPCEndpoint)
##########
File path: src/runtime/rpc/rpc_local_session.cc
##########
@@ -87,26 +88,25 @@ void LocalSession::CallFunc(RPCSession::PackedFuncHandle func, const TVMValue* a
this->EncodeReturn(std::move(rv), encode_return);
}
-void LocalSession::CopyToRemote(void* from, size_t from_offset, void* to, size_t to_offset,
- size_t nbytes, TVMContext ctx_to, DLDataType type_hint) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
- this->GetDeviceAPI(ctx_to)->CopyDataFromTo(from, from_offset, to, to_offset, nbytes, cpu_ctx,
- ctx_to, type_hint, nullptr);
+void LocalSession::CopyToRemote(void* from_bytes, DLTensor* to, size_t nbytes) {
+ ICHECK_EQ(nbytes, GetDataSize(*to));
+ std::vector<int64_t> shape(to->shape, to->shape + to->ndim);
+ DLTensor from{from_bytes, {kDLCPU, 0}, to->ndim, to->dtype, shape.data(), nullptr, 0};
+
+ TVMContext ctx_to = to->ctx;
+ this->GetDeviceAPI(ctx_to)->CopyDataFromTo(&from, to, nullptr);
// Copy can happen asynchrously
// synchronize to make sure that copy is completed
this->GetDeviceAPI(ctx_to)->StreamSync(ctx_to, nullptr);
}
-void LocalSession::CopyFromRemote(void* from, size_t from_offset, void* to, size_t to_offset,
- size_t nbytes, TVMContext ctx_from, DLDataType type_hint) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
+void LocalSession::CopyFromRemote(DLTensor* from, void* to_bytes, size_t nbytes) {
+ ICHECK_EQ(nbytes, GetDataSize(*from));
+ std::vector<int64_t> shape(from->shape, from->shape + from->ndim);
Review comment:
no need to construct a vector, can directlyr euse from->shape and from->ndim
##########
File path: src/runtime/rpc/rpc_local_session.cc
##########
@@ -87,26 +88,25 @@ void LocalSession::CallFunc(RPCSession::PackedFuncHandle func, const TVMValue* a
this->EncodeReturn(std::move(rv), encode_return);
}
-void LocalSession::CopyToRemote(void* from, size_t from_offset, void* to, size_t to_offset,
- size_t nbytes, TVMContext ctx_to, DLDataType type_hint) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
- this->GetDeviceAPI(ctx_to)->CopyDataFromTo(from, from_offset, to, to_offset, nbytes, cpu_ctx,
- ctx_to, type_hint, nullptr);
+void LocalSession::CopyToRemote(void* from_bytes, DLTensor* to, size_t nbytes) {
+ ICHECK_EQ(nbytes, GetDataSize(*to));
+ std::vector<int64_t> shape(to->shape, to->shape + to->ndim);
+ DLTensor from{from_bytes, {kDLCPU, 0}, to->ndim, to->dtype, shape.data(), nullptr, 0};
Review comment:
construct DLTensor by fields.
##########
File path: web/emcc/tvmjs_support.cc
##########
@@ -177,33 +177,25 @@ class AsyncLocalSession : public LocalSession {
}
}
- void AsyncCopyToRemote(void* local_from, size_t local_from_offset, void* remote_to,
- size_t remote_to_offset, size_t nbytes, TVMContext remote_ctx_to,
- DLDataType type_hint, FAsyncCallback on_complete) final {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
+ void AsyncCopyToRemote(void* local_from_bytes, DLTensor* remote_to, size_t nbytes,
+ FAsyncCallback on_complete) final {
try {
- this->GetDeviceAPI(remote_ctx_to)
- ->CopyDataFromTo(local_from, local_from_offset, remote_to, remote_to_offset, nbytes,
- cpu_ctx, remote_ctx_to, type_hint, nullptr);
- this->AsyncStreamWait(remote_ctx_to, nullptr, on_complete);
+ this->GetDeviceAPI(remote_to->ctx)
Review comment:
update to the new API that uses DLTensor*
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r581574952
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -144,6 +144,46 @@ void* DeviceAPI::AllocWorkspace(TVMContext ctx, size_t size, DLDataType type_hin
return AllocDataSpace(ctx, size, kTempAllocaAlignment, type_hint);
}
+static size_t GetDataAlignment(const DLDataType dtype) {
+ size_t align = (dtype.bits / 8) * dtype.lanes;
+ if (align < kAllocAlignment) return kAllocAlignment;
+ return align;
+}
+
+void* DeviceAPI::AllocDataSpace(TVMContext ctx, int ndim, const int64_t* shape, DLDataType dtype,
+ Optional<String> mem_scope) {
+ if (!mem_scope.defined() || mem_scope.value() == "global") {
+ // by default, we can always redirect to the flat memory allocations
+ DLTensor temp;
+ temp.data = nullptr;
+ temp.ctx = ctx;
+ temp.ndim = ndim;
+ temp.dtype = dtype;
+ temp.shape = const_cast<int64_t*>(shape);
+ temp.strides = nullptr;
+ temp.byte_offset = 0;
+ size_t size = GetDataSize(temp);
+ size_t alignment = GetDataAlignment(temp.dtype);
+ return AllocDataSpace(ctx, size, alignment, dtype);
+ }
+ LOG(FATAL) << "Device does not support allocate data space with "
+ << "specified memory scope: " << mem_scope.value();
+ return nullptr;
+}
+
+void DeviceAPI::CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream) {
+ // by default, we can always redirect to the flat memory copy operation.
+ size_t nbytes = GetDataSize(*from);
Review comment:
CHECK both from and to are contiguous
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#issuecomment-785935582
cc @junrushao1994 @csullivan would be great if you can help review as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#issuecomment-787473335
Thanks @ZihengJiang @junrushao1994 @csullivan !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ZihengJiang commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
ZihengJiang commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583281947
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -98,20 +109,13 @@ class TVM_DLL DeviceAPI {
virtual void FreeDataSpace(TVMContext ctx, void* ptr) = 0;
/*!
* \brief copy data from one place to another
+ * \note This API is designed to support special memory with shape dependent layout.
+ * We pass in DLTensor* with shape information to support these cases.
* \param from The source array.
- * \param from_offset The byte offeset in the from.
* \param to The target array.
- * \param to_offset The byte offset in the to.
- * \param num_bytes The size of the memory in bytes
- * \param ctx_from The source context
- * \param ctx_to The target context
- * \param type_hint The type of elements, only neded by certain backends.
- * can be useful for cross device endian converison.
* \param stream Optional stream object.
*/
- virtual void CopyDataFromTo(const void* from, size_t from_offset, void* to, size_t to_offset,
- size_t num_bytes, TVMContext ctx_from, TVMContext ctx_to,
- DLDataType type_hint, TVMStreamHandle stream) = 0;
+ virtual void CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream);
Review comment:
same reason that Tianqi metioned beblow: we actually expect `const void*` for `from->data`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583808824
##########
File path: src/runtime/rpc/rpc_endpoint.h
##########
@@ -147,8 +146,7 @@ class RPCEndpoint {
* \param ctx_from The source context.
* \param type_hint Hint of content data type.
*/
- void CopyFromRemote(void* from, size_t from_offset, void* to, size_t to_offset, size_t nbytes,
- TVMContext ctx_from, DLDataType type_hint);
+ void CopyFromRemote(DLTensor* from, void* to_bytes, uint64_t nbytes);
Review comment:
Agreed on const void* for data field protection. Prefer to provide as much direction as possible on ownership, but there's only so much one can indicate with c pointers.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r581219494
##########
File path: include/tvm/runtime/c_runtime_api.h
##########
@@ -572,7 +572,7 @@ TVM_DLL int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignme
* \param out_data The allocated device pointer.
* \return 0 when success, -1 when failure happens
*/
-TVM_DLL int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, size_t ndim, int64_t* shape,
+TVM_DLL int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, int64_t* shape,
Review comment:
const int64_t*
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -593,7 +593,7 @@ int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment, DLDa
API_END();
}
-int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, size_t ndim, int64_t* shape, DLDataType dtype,
+int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, int64_t* shape, DLDataType dtype,
Review comment:
const int64_t
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r581029417
##########
File path: include/tvm/runtime/c_runtime_api.h
##########
@@ -569,22 +583,12 @@ TVM_DLL int TVMDeviceFreeDataSpace(TVMContext ctx, void* ptr);
/*!
* \brief Copy data from one place to another.
- * \param from The source array.
- * \param from_offset The byte offeset in the from.
- * \param to The target array.
- * \param to_offset The byte offset in the to.
- * \param num_bytes The size of the memory in bytes
- * \param ctx_from The source context
- * \param ctx_to The target context
- * \param type_hint The type of elements, only neded by certain backends.
- * can be useful for cross device endian converison.
+ * \param from The source tensor.
+ * \param to The target tensor.
* \param stream Optional stream object.
Review comment:
Add
```
\note This API is designed to support special memory with shape dependent layout.
We pass in DLTensor* with shape information to support these cases.
```
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -99,19 +110,10 @@ class TVM_DLL DeviceAPI {
/*!
* \brief copy data from one place to another
* \param from The source array.
- * \param from_offset The byte offeset in the from.
* \param to The target array.
- * \param to_offset The byte offset in the to.
- * \param num_bytes The size of the memory in bytes
- * \param ctx_from The source context
- * \param ctx_to The target context
- * \param type_hint The type of elements, only neded by certain backends.
- * can be useful for cross device endian converison.
* \param stream Optional stream object.
Review comment:
Add similar note about special memory layout handling
##########
File path: src/runtime/ndarray.cc
##########
@@ -279,6 +279,17 @@ int TVMArrayAlloc(const tvm_index_t* shape, int ndim, int dtype_code, int dtype_
API_END();
}
+TVM_REGISTER_GLOBAL("runtime.TVMArrayAllocWithScope").set_body([](TVMArgs args, TVMRetValue* ret) {
+ int64_t* shape_ptr = static_cast<int64_t*>(static_cast<void*>(args[0]));
Review comment:
avoid pass in raw pointers, consider encoding shape into positional arguments for now
##########
File path: src/runtime/ndarray.cc
##########
@@ -58,36 +59,39 @@ inline void VerifyDataType(DLDataType dtype) {
ICHECK_EQ(dtype.bits & (dtype.bits - 1), 0);
}
-inline size_t GetDataAlignment(const DLTensor& arr) {
- size_t align = (arr.dtype.bits / 8) * arr.dtype.lanes;
- if (align < kAllocAlignment) return kAllocAlignment;
- return align;
-}
-
void ArrayCopyFromBytes(DLTensor* handle, const void* data, size_t nbytes) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
size_t arr_size = GetDataSize(*handle);
ICHECK_EQ(arr_size, nbytes) << "ArrayCopyFromBytes: size mismatch";
ICHECK(IsContiguous(*handle)) << "ArrayCopyFromBytes only support contiguous array for now";
- DeviceAPI::Get(handle->ctx)
- ->CopyDataFromTo(data, 0, handle->data, static_cast<size_t>(handle->byte_offset), nbytes,
- cpu_ctx, handle->ctx, handle->dtype, nullptr);
+
+ DLTensor from;
+ from.data = const_cast<void*>(data);
+ from.ctx = DLContext{kDLCPU, 0};
+ from.ndim = handle->ndim;
+ from.dtype = handle->dtype;
+ from.shape = handle->shape;
+ from.strides = handle->strides;
+ from.byte_offset = handle->byte_offset;
Review comment:
`from.byte_offset = 0`, NOTE (the data view in from starts from 0)
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -144,6 +144,44 @@ void* DeviceAPI::AllocWorkspace(TVMContext ctx, size_t size, DLDataType type_hin
return AllocDataSpace(ctx, size, kTempAllocaAlignment, type_hint);
}
+static size_t GetDataAlignment(const DLDataType dtype) {
+ size_t align = (dtype.bits / 8) * dtype.lanes;
+ if (align < kAllocAlignment) return kAllocAlignment;
+ return align;
+}
+
+void* DeviceAPI::AllocDataSpace(TVMContext ctx, std::vector<int64_t> shape, DLDataType dtype,
+ Optional<String> mem_scope) {
+ if (!mem_scope.defined() || mem_scope.value() == "global") {
+ DLTensor temp;
+ temp.data = nullptr;
+ temp.ctx = ctx;
+ temp.ndim = static_cast<int>(shape.size());
+ temp.dtype = dtype;
+ temp.shape = shape.data();
+ temp.strides = nullptr;
+ temp.byte_offset = 0;
+ size_t size = GetDataSize(temp);
+ size_t alignment = GetDataAlignment(temp.dtype);
+ return AllocDataSpace(ctx, size, alignment, dtype);
Review comment:
add comments, by default, we can always redirect to the flat memory allocations
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -90,6 +91,16 @@ class TVM_DLL DeviceAPI {
*/
virtual void* AllocDataSpace(TVMContext ctx, size_t nbytes, size_t alignment,
DLDataType type_hint) = 0;
+ /*!
+ * \brief Allocate a data space on device.
+ * \param ctx The device context to perform operation.
+ * \param shape The shape of allocated tensor.
+ * \param dtype The type of elements.
+ * \param mem_scope The memory scope of allocated tensor.
+ * \return The allocated device pointer.
+ */
+ virtual void* AllocDataSpace(TVMContext ctx, std::vector<int64_t> shape, DLDataType dtype,
Review comment:
let us consider doing ```int ndim, const int64_t* shape,``` instead, to be more compatible with the DLTensor*'s field without constructing a vector.
##########
File path: src/runtime/ndarray.cc
##########
@@ -58,36 +59,39 @@ inline void VerifyDataType(DLDataType dtype) {
ICHECK_EQ(dtype.bits & (dtype.bits - 1), 0);
}
-inline size_t GetDataAlignment(const DLTensor& arr) {
- size_t align = (arr.dtype.bits / 8) * arr.dtype.lanes;
- if (align < kAllocAlignment) return kAllocAlignment;
- return align;
-}
-
void ArrayCopyFromBytes(DLTensor* handle, const void* data, size_t nbytes) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
size_t arr_size = GetDataSize(*handle);
ICHECK_EQ(arr_size, nbytes) << "ArrayCopyFromBytes: size mismatch";
ICHECK(IsContiguous(*handle)) << "ArrayCopyFromBytes only support contiguous array for now";
- DeviceAPI::Get(handle->ctx)
- ->CopyDataFromTo(data, 0, handle->data, static_cast<size_t>(handle->byte_offset), nbytes,
- cpu_ctx, handle->ctx, handle->dtype, nullptr);
+
+ DLTensor from;
+ from.data = const_cast<void*>(data);
+ from.ctx = DLContext{kDLCPU, 0};
+ from.ndim = handle->ndim;
+ from.dtype = handle->dtype;
+ from.shape = handle->shape;
+ from.strides = handle->strides;
+ from.byte_offset = handle->byte_offset;
+ DeviceAPI::Get(handle->ctx)->CopyDataFromTo(&from, handle, nullptr);
// Synchronize in case data become unavailable later.
DeviceAPI::Get(handle->ctx)->StreamSync(handle->ctx, nullptr);
}
void ArrayCopyToBytes(const DLTensor* handle, void* data, size_t nbytes) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
size_t arr_size = GetDataSize(*handle);
ICHECK_EQ(arr_size, nbytes) << "ArrayCopyToBytes: size mismatch";
ICHECK(IsContiguous(*handle)) << "ArrayCopyToBytes only support contiguous array for now";
- DeviceAPI::Get(handle->ctx)
- ->CopyDataFromTo(handle->data, static_cast<size_t>(handle->byte_offset), data, 0, nbytes,
- handle->ctx, cpu_ctx, handle->dtype, nullptr);
+
+ DLTensor to;
+ to.data = const_cast<void*>(data);
+ to.ctx = DLContext{kDLCPU, 0};
+ to.ndim = handle->ndim;
+ to.dtype = handle->dtype;
+ to.shape = handle->shape;
+ to.strides = handle->strides;
Review comment:
strides = nullptr
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -90,6 +91,16 @@ class TVM_DLL DeviceAPI {
*/
virtual void* AllocDataSpace(TVMContext ctx, size_t nbytes, size_t alignment,
DLDataType type_hint) = 0;
+ /*!
+ * \brief Allocate a data space on device.
Review comment:
Allocate data space with memory scope support
##########
File path: src/runtime/ndarray.cc
##########
@@ -58,36 +59,39 @@ inline void VerifyDataType(DLDataType dtype) {
ICHECK_EQ(dtype.bits & (dtype.bits - 1), 0);
}
-inline size_t GetDataAlignment(const DLTensor& arr) {
- size_t align = (arr.dtype.bits / 8) * arr.dtype.lanes;
- if (align < kAllocAlignment) return kAllocAlignment;
- return align;
-}
-
void ArrayCopyFromBytes(DLTensor* handle, const void* data, size_t nbytes) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
size_t arr_size = GetDataSize(*handle);
ICHECK_EQ(arr_size, nbytes) << "ArrayCopyFromBytes: size mismatch";
ICHECK(IsContiguous(*handle)) << "ArrayCopyFromBytes only support contiguous array for now";
- DeviceAPI::Get(handle->ctx)
- ->CopyDataFromTo(data, 0, handle->data, static_cast<size_t>(handle->byte_offset), nbytes,
- cpu_ctx, handle->ctx, handle->dtype, nullptr);
+
+ DLTensor from;
+ from.data = const_cast<void*>(data);
+ from.ctx = DLContext{kDLCPU, 0};
+ from.ndim = handle->ndim;
+ from.dtype = handle->dtype;
+ from.shape = handle->shape;
+ from.strides = handle->strides;
+ from.byte_offset = handle->byte_offset;
+ DeviceAPI::Get(handle->ctx)->CopyDataFromTo(&from, handle, nullptr);
// Synchronize in case data become unavailable later.
DeviceAPI::Get(handle->ctx)->StreamSync(handle->ctx, nullptr);
}
void ArrayCopyToBytes(const DLTensor* handle, void* data, size_t nbytes) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
size_t arr_size = GetDataSize(*handle);
ICHECK_EQ(arr_size, nbytes) << "ArrayCopyToBytes: size mismatch";
ICHECK(IsContiguous(*handle)) << "ArrayCopyToBytes only support contiguous array for now";
- DeviceAPI::Get(handle->ctx)
- ->CopyDataFromTo(handle->data, static_cast<size_t>(handle->byte_offset), data, 0, nbytes,
- handle->ctx, cpu_ctx, handle->dtype, nullptr);
+
+ DLTensor to;
+ to.data = const_cast<void*>(data);
+ to.ctx = DLContext{kDLCPU, 0};
+ to.ndim = handle->ndim;
+ to.dtype = handle->dtype;
+ to.shape = handle->shape;
+ to.strides = handle->strides;
+ to.byte_offset = handle->byte_offset;
Review comment:
to.byte_offset = 0;
##########
File path: src/runtime/ndarray.cc
##########
@@ -58,36 +59,39 @@ inline void VerifyDataType(DLDataType dtype) {
ICHECK_EQ(dtype.bits & (dtype.bits - 1), 0);
}
-inline size_t GetDataAlignment(const DLTensor& arr) {
- size_t align = (arr.dtype.bits / 8) * arr.dtype.lanes;
- if (align < kAllocAlignment) return kAllocAlignment;
- return align;
-}
-
void ArrayCopyFromBytes(DLTensor* handle, const void* data, size_t nbytes) {
- TVMContext cpu_ctx;
- cpu_ctx.device_type = kDLCPU;
- cpu_ctx.device_id = 0;
size_t arr_size = GetDataSize(*handle);
ICHECK_EQ(arr_size, nbytes) << "ArrayCopyFromBytes: size mismatch";
ICHECK(IsContiguous(*handle)) << "ArrayCopyFromBytes only support contiguous array for now";
- DeviceAPI::Get(handle->ctx)
- ->CopyDataFromTo(data, 0, handle->data, static_cast<size_t>(handle->byte_offset), nbytes,
- cpu_ctx, handle->ctx, handle->dtype, nullptr);
+
+ DLTensor from;
+ from.data = const_cast<void*>(data);
+ from.ctx = DLContext{kDLCPU, 0};
+ from.ndim = handle->ndim;
+ from.dtype = handle->dtype;
+ from.shape = handle->shape;
+ from.strides = handle->strides;
Review comment:
from.strides = nullptr
##########
File path: include/tvm/runtime/c_runtime_api.h
##########
@@ -559,6 +559,20 @@ TVM_DLL int TVMByteArrayFree(TVMByteArray* arr);
TVM_DLL int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment,
DLDataType type_hint, void** out_data);
+/*!
+ * \brief Allocate a data space on device.
+ * \param ctx The device context to perform operation.
+ * \param ndim The number of dimension of the tensor.
+ * \param shape The shape of the tensor.
+ * \param dtype The type of elements.
+ * \param mem_scope The memory scope of the tensor.
Review comment:
can be nullptr, which indicate the default global DRAM
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -144,6 +144,44 @@ void* DeviceAPI::AllocWorkspace(TVMContext ctx, size_t size, DLDataType type_hin
return AllocDataSpace(ctx, size, kTempAllocaAlignment, type_hint);
}
+static size_t GetDataAlignment(const DLDataType dtype) {
+ size_t align = (dtype.bits / 8) * dtype.lanes;
+ if (align < kAllocAlignment) return kAllocAlignment;
+ return align;
+}
+
+void* DeviceAPI::AllocDataSpace(TVMContext ctx, std::vector<int64_t> shape, DLDataType dtype,
+ Optional<String> mem_scope) {
+ if (!mem_scope.defined() || mem_scope.value() == "global") {
+ DLTensor temp;
+ temp.data = nullptr;
+ temp.ctx = ctx;
+ temp.ndim = static_cast<int>(shape.size());
+ temp.dtype = dtype;
+ temp.shape = shape.data();
+ temp.strides = nullptr;
+ temp.byte_offset = 0;
+ size_t size = GetDataSize(temp);
+ size_t alignment = GetDataAlignment(temp.dtype);
+ return AllocDataSpace(ctx, size, alignment, dtype);
+ }
+ LOG(FATAL) << "Device does not support allocate data space with "
+ << "specified memory scope: " << mem_scope.value();
+ return nullptr;
+}
+
+void DeviceAPI::CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream) {
+ size_t nbytes = GetDataSize(*from);
+ CopyDataFromTo(from->data, from->byte_offset, to->data, to->byte_offset, nbytes, from->ctx,
Review comment:
add comments, by default, we can always redirect to the flat memory copy operation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583163694
##########
File path: src/runtime/rpc/rpc_endpoint.h
##########
@@ -147,8 +146,7 @@ class RPCEndpoint {
* \param ctx_from The source context.
* \param type_hint Hint of content data type.
*/
- void CopyFromRemote(void* from, size_t from_offset, void* to, size_t to_offset, size_t nbytes,
- TVMContext ctx_from, DLDataType type_hint);
+ void CopyFromRemote(DLTensor* from, void* to_bytes, uint64_t nbytes);
Review comment:
This was discussed in the initial proposal. What we really wanted was a DLTensor with const void* in the data field. Making const DLTensor* won't provide much protection as expected, other than the syntax highlighting the fact that from should be constant.
Eventually I think DLTensor* is fine here to be consistent with https://github.com/apache/tvm/blob/main/include/tvm/runtime/c_runtime_api.h#L425
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r583160747
##########
File path: include/tvm/runtime/c_runtime_api.h
##########
@@ -559,6 +559,23 @@ TVM_DLL int TVMByteArrayFree(TVMByteArray* arr);
TVM_DLL int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment,
DLDataType type_hint, void** out_data);
+/*!
+ * \brief Allocate a data space on device with special memory scope.
+ * \note The memory could use a special multi-dimensional memory layout.
+ * That is why we pass shape and dtype instead of raw number of bytes.
+ * \param ctx The device context to perform operation.
+ * \param ndim The number of dimension of the tensor.
+ * \param shape The shape of the tensor.
+ * \param dtype The type of elements.
+ * \param mem_scope The memory scope of the tensor,
+ * can be nullptr, which indicate the default global DRAM
+ * \param out_data The allocated device pointer.
+ * \return 0 when success, -1 when failure happens
+ */
+TVM_DLL int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, const int64_t* shape,
+ DLDataType dtype, const char* mem_scope,
+ void** out_data);
+
Review comment:
In that case we would expect the DeviceAPI to create auxiliary `struct Data` that wraps these opaque memory pointers with the memory tag instead. Inside the DeviceAPI the Data is being unwrapped to get the necessary information. This is also important because DLTensor as it is do not have standardization for memory scope, and unlikely we will have one in the futue
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r580596610
##########
File path: python/tvm/runtime/ndarray.py
##########
@@ -253,42 +254,35 @@ def numpyasarray(np_data):
return arr, shape
-def empty(shape, dtype="float32", ctx=context(1, 0)):
+def empty(shape, dtype="float32", ctx=context(1, 0), mem_scope=None):
"""Create an empty array given shape and device
Parameters
----------
shape : tuple of int
- The shape of the array
+ The shape of the array.
dtype : type or str
The data type of the array.
ctx : TVMContext
- The context of the array
+ The context of the array.
+
+ mem_scope : str
+ The memory scope of the array.
Returns
-------
arr : tvm.nd.NDArray
The array tvm supported.
"""
- shape = c_array(tvm_shape_index_t, shape)
- ndim = ctypes.c_int(len(shape))
- handle = TVMArrayHandle()
+ arr = np.array(shape, 'int64')
+ ptr = arr.ctypes.data_as(ctypes.POINTER(ctypes.c_int64))
+ shape_ptr = ctypes.cast(ptr, ctypes.c_void_p)
+ ndim = len(shape)
dtype = DataType(dtype)
- check_call(
- _LIB.TVMArrayAlloc(
- shape,
- ndim,
- ctypes.c_int(dtype.type_code),
- ctypes.c_int(dtype.bits),
- ctypes.c_int(dtype.lanes),
- ctx.device_type,
- ctx.device_id,
- ctypes.byref(handle),
- )
- )
- return _make_array(handle, False, False)
+ arr = _ffi_api.TVMArrayAllocWithScope(shape_ptr, ndim, dtype, ctx, mem_scope)
Review comment:
What necessitates the change from calling via _ffi_api vs _LIB?
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -90,6 +90,17 @@ class TVM_DLL DeviceAPI {
*/
virtual void* AllocDataSpace(TVMContext ctx, size_t nbytes, size_t alignment,
DLDataType type_hint) = 0;
+ /*!
+ * \brief Allocate a data space on device with memory scope support.
+ * \param ctx The device context to perform operation.
+ * \param ndim The number of dimension of allocated tensor.
+ * \param shape The shape of allocated tensor.
+ * \param dtype The type of elements.
+ * \param mem_scope The memory scope of allocated tensor.
+ * \return The allocated device pointer.
+ */
+ virtual void* AllocDataSpace(TVMContext ctx, int ndim, const int64_t* shape, DLDataType dtype,
+ Optional<String> mem_scope = NullOpt);
Review comment:
In the RFC you mention introducing `mem_scope` as an enum or other structure. In this implementation we are opting for an optional string. Why the change from RFC?
I'm considering the case for how a backend could interpret different sub scopes, e.g. `texture:activation` or `texture:weight`. The string format provides more flexibility, just want to make sure our thinking is aligned on this.
##########
File path: src/runtime/rpc/rpc_endpoint.h
##########
@@ -147,8 +146,7 @@ class RPCEndpoint {
* \param ctx_from The source context.
* \param type_hint Hint of content data type.
*/
- void CopyFromRemote(void* from, size_t from_offset, void* to, size_t to_offset, size_t nbytes,
- TVMContext ctx_from, DLDataType type_hint);
+ void CopyFromRemote(DLTensor* from, void* to_bytes, uint64_t nbytes);
Review comment:
All over we use mutable `from` tensor, is there a good reason?
##########
File path: src/runtime/minrpc/minrpc_server.h
##########
@@ -209,30 +220,39 @@ class MinRPCServer {
}
void HandleCopyToRemote() {
- uint64_t handle, offset, num_bytes;
- TVMContext ctx;
- DLDataType type_hint;
-
- this->Read(&handle);
- this->Read(&offset);
+ DLTensor* arr = this->ArenaAlloc<DLTensor>(1);
+ uint64_t data_handle;
+ this->Read(&data_handle);
+ arr->data = reinterpret_cast<void*>(data_handle);
+ this->Read(&(arr->ctx));
+ this->Read(&(arr->ndim));
+ this->Read(&(arr->dtype));
+ arr->shape = this->ArenaAlloc<int64_t>(arr->ndim);
+ this->ReadArray(arr->shape, arr->ndim);
+ arr->strides = nullptr;
+ this->Read(&(arr->byte_offset));
+ uint64_t num_bytes;
this->Read(&num_bytes);
- this->Read(&ctx);
- this->Read(&type_hint);
- int call_ecode = 0;
- if (ctx.device_type == kDLCPU) {
- uint8_t* dptr = reinterpret_cast<uint8_t*>(handle) + offset;
+ int call_ecode = 0;
+ if (arr->ctx.device_type == kDLCPU) {
+ uint8_t* dptr = reinterpret_cast<uint8_t*>(data_handle) + arr->byte_offset;
this->ReadArray(dptr, num_bytes);
} else {
uint8_t* temp_data = this->ArenaAlloc<uint8_t>(num_bytes);
this->ReadArray(temp_data, num_bytes);
-
- call_ecode =
- TVMDeviceCopyDataFromTo(temp_data, 0, reinterpret_cast<void*>(handle), offset, num_bytes,
- DLContext{kDLCPU, 0}, ctx, type_hint, nullptr);
+ DLTensor temp;
+ temp.data = temp_data;
+ temp.ctx = DLContext{kDLCPU, 0};
+ temp.ndim = arr->ndim;
+ temp.dtype = arr->dtype;
+ temp.shape = arr->shape;
+ temp.strides = nullptr;
+ temp.byte_offset = 0;
Review comment:
Blocks like this are repeated quite a bit, a make_(dl)tensor helper would be nice.
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -98,20 +109,13 @@ class TVM_DLL DeviceAPI {
virtual void FreeDataSpace(TVMContext ctx, void* ptr) = 0;
/*!
* \brief copy data from one place to another
+ * \note This API is designed to support special memory with shape dependent layout.
+ * We pass in DLTensor* with shape information to support these cases.
* \param from The source array.
- * \param from_offset The byte offeset in the from.
* \param to The target array.
- * \param to_offset The byte offset in the to.
- * \param num_bytes The size of the memory in bytes
- * \param ctx_from The source context
- * \param ctx_to The target context
- * \param type_hint The type of elements, only neded by certain backends.
- * can be useful for cross device endian converison.
* \param stream Optional stream object.
*/
- virtual void CopyDataFromTo(const void* from, size_t from_offset, void* to, size_t to_offset,
- size_t num_bytes, TVMContext ctx_from, TVMContext ctx_to,
- DLDataType type_hint, TVMStreamHandle stream) = 0;
+ virtual void CopyDataFromTo(DLTensor* from, DLTensor* to, TVMStreamHandle stream);
Review comment:
`const DLTensor* from` ?
##########
File path: src/runtime/rpc/rpc_local_session.cc
##########
@@ -27,6 +27,7 @@
#include <tvm/runtime/registry.h>
#include <memory>
+#include <vector>
Review comment:
Looks like this is no longer necessary, remove if so.
##########
File path: include/tvm/runtime/c_runtime_api.h
##########
@@ -559,6 +559,23 @@ TVM_DLL int TVMByteArrayFree(TVMByteArray* arr);
TVM_DLL int TVMDeviceAllocDataSpace(DLContext ctx, size_t nbytes, size_t alignment,
DLDataType type_hint, void** out_data);
+/*!
+ * \brief Allocate a data space on device with special memory scope.
+ * \note The memory could use a special multi-dimensional memory layout.
+ * That is why we pass shape and dtype instead of raw number of bytes.
+ * \param ctx The device context to perform operation.
+ * \param ndim The number of dimension of the tensor.
+ * \param shape The shape of the tensor.
+ * \param dtype The type of elements.
+ * \param mem_scope The memory scope of the tensor,
+ * can be nullptr, which indicate the default global DRAM
+ * \param out_data The allocated device pointer.
+ * \return 0 when success, -1 when failure happens
+ */
+TVM_DLL int TVMDeviceAllocDataSpaceWithScope(DLContext ctx, int ndim, const int64_t* shape,
+ DLDataType dtype, const char* mem_scope,
+ void** out_data);
+
Review comment:
For supporting device to device copying between memory types we could consider including a TVMArrayCopyFromToWithScope? The device backend may or may not be able to infer the storage type from the raw to/from handles alone. This shouldn't be an issue for OpenCL, but I can imagine a device api that uses completely opaque memory pointers and relies on user management. In which case having a from_scope and to_scope would be helpful.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7488: [Runtime] Special Memory Scope Support
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7488:
URL: https://github.com/apache/tvm/pull/7488#discussion_r581193506
##########
File path: include/tvm/runtime/device_api.h
##########
@@ -28,6 +28,7 @@
#include <tvm/runtime/packed_func.h>
#include <string>
+#include <vector>
Review comment:
This is no longer needed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org