adstraw commented on a change in pull request #10558:
URL: https://github.com/apache/tvm/pull/10558#discussion_r825151297
##########
File path: src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc
##########
@@ -119,6 +119,13 @@ void HexagonDeviceAPIv2::FreeWorkspace(Device dev, void*
data) {
workspace_allocations_.erase(it);
}
+void* HexagonDeviceAPIv2::AllocVtcmWorkspace(Device dev, int ndim, const
int64_t* shape,
+ DLDataType dtype,
Optional<String> mem_scope) {
+ return AllocDataSpace(dev, ndim, shape, dtype, mem_scope);
Review comment:
> I think we should add an assert here that ndim is either 1 or 2, the
supported number of physical dimensions.
Done. Added a check for `ndim` == 1 as follows:
```
// Forcing contiguous allocation, for now
// TODO(Straw): Enable discontiguous allocation
CHECK_EQ(ndim, 1);
```
The Hexagon Device API is forcing / assuming contiguous (1d) memory
allocations in all cases including for this PR. The 6 places in the Hexagon
Device API where 1d memory allocations are forced / assumed are called out in
this PR. The original comment and `TODO` items called out RFC 39 which has now
landed along with PR #9727. I have edited the `TODO` items accordingly. These
`TODO` items should be handled in a follow-up PR once all the pieces are in
place for discontiguous allocation.
> Does this call AllocDataSpace as implemented by DeviceAPI, or as
implemented by HexagonDeviceAPIV2? I only see an override of
HexagonDeviceAPIv2::AllocDataSpace for the overload without a mem_scope
argument.
It calls HexagonDeviceAPIv2::AllocDataSpace
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
