Skip to content

Commit

Permalink
[DLPack][runtime] Update DLPack to v0.7
Browse files Browse the repository at this point in the history
- Update the `3rdparty/dlpack` git submodule from v0.5 to v0.7, so that
the `DLDeviceType` enumeration has an explicitly-stated underlying
storage type.  This addresses a compiler warning generated by clang
15.0.3.

- Remove `kDLHexagon` and `kDLWebGPU` from `TVMDeviceExtType`, because
those enumerators are now provided by `DLDeviceType`.

- Renumber the members of `TVMDeviceExtType` to reduce the chance of
unnoticed collision with members of `DLDeviceType`.
  • Loading branch information
Christian Convey committed Oct 24, 2022
1 parent 3131cdc commit 90ed6f8
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 18 deletions.
40 changes: 33 additions & 7 deletions include/tvm/runtime/c_runtime_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,40 @@ extern "C" {
/*! \brief type of array index. */
typedef int64_t tvm_index_t;

/*! \brief Extension device types in TVM */
/*! \brief Extension device types in TVM
*
* An extended version of DLPack's `DLDeviceType` enumeration.
*
* These enumerators are a supsetset of those provided by
* `DLDeviceType`.
*
* MAINTAINERS NOTE #1: We need to ensure that these two
* enumerations don't use conflicting symbols for the same
* integer. Currently this requires manual verification.
* Discussed here: https://github.com/dmlc/dlpack/issues/111
* As of DLPack v0.7, the highest-valued enumerator in
* `DLDeviceType` is kDLHexagon = 16.
*
* MAINTAINERS NOTE #2: It's important that when C/C++ compilers
* choose the underlying storage type for `DLDeviceType` and
* `TVMDeviceExtType`, the underlying type can store whatever
* integer values appear in *either* enumeration.
* For C++, we enforce this by specifying `int32_t` for both
* enumerations. For C, this is currently unenforced, and
* may be a latent bug.
*/
#ifdef __cplusplus
typedef enum : int32_t {
#else
typedef enum {
kDLAOCL = 5,
kDLSDAccel = 6,
kOpenGL = 11,
kDLMicroDev = 13,
kDLHexagon = 14,
kDLWebGPU = 15
#endif
// To help avoid accidental conflicts between `DLDeviceType`
// and this enumeration, start numbering the new enumerators
// a little higher than (currently) seems necessary.
kDLAOCL = kDLHexagon + 10,
kDLSDAccel,
kOpenGL,
kDLMicroDev,
// AddExtraTVMType which is not in DLPack here
} TVMDeviceExtType;

Expand Down
4 changes: 2 additions & 2 deletions src/runtime/aot_executor/aot_executor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ AotExecutor::AotExecutor(tvm::runtime::Module module, const std::vector<Device>&
ICHECK_EQ(devices_[0].device_id, expected_device.device_id)
<< "At this time, AOTExecutor supports only execution on kDLCPU 0";
// TODO(tvm-team): Temporary hack since Hexagon is defined different than kDLCPU.
bool is_valid_device = (TVMDeviceExtType(devices_[0].device_type) == kDLHexagon) ||
(DLDeviceType(devices_[0].device_type) == kDLCPU);
bool is_valid_device =
(devices_[0].device_type == kDLHexagon) || (devices_[0].device_type == kDLCPU);
CHECK(is_valid_device)
<< "At this time, AOTExecutor supports only execution on kDLCPU 0 or kDLHexagon 0";

Expand Down
4 changes: 1 addition & 3 deletions src/runtime/hexagon/hexagon_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@
} \
} while (0)

inline bool IsHexagonDevice(DLDevice dev) {
return TVMDeviceExtType(dev.device_type) == kDLHexagon;
}
inline bool IsHexagonDevice(DLDevice dev) { return dev.device_type == kDLHexagon; }

constexpr int kHexagonAllocAlignment = 2048;

Expand Down
6 changes: 3 additions & 3 deletions src/runtime/hexagon/hexagon_device_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void* HexagonDeviceAPI::AllocDataSpace(Device dev, int ndim, const int64_t* shap

// NOTE: This check should be superfluous, but it's probably a good idea to leave it in
// until the AoT executor's multi-device dispatch code is mature. --cconvey 2022-08-26
CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon)
CHECK(dev.device_type == kDLHexagon)
<< "dev.device_type: " << dev.device_type << " DeviceName(" << dev.device_type
<< "): " << DeviceName(dev.device_type) << "";

Expand Down Expand Up @@ -162,14 +162,14 @@ void HexagonDeviceAPI::FreeWorkspace(Device dev, void* data) {
void* HexagonDeviceAPI::AllocVtcmWorkspace(Device dev, int ndim, const int64_t* shape,
DLDataType dtype, Optional<String> mem_scope) {
// must be Hexagon device (not CPU)
CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon) << "dev.device_type: " << dev.device_type;
CHECK(dev.device_type == kDLHexagon) << "dev.device_type: " << dev.device_type;
CHECK((ndim == 1 || ndim == 2) && "Hexagon Device API supports only 1d and 2d allocations");
return AllocDataSpace(dev, ndim, shape, dtype, mem_scope);
}

void HexagonDeviceAPI::FreeVtcmWorkspace(Device dev, void* ptr) {
// must be Hexagon device (not CPU)
CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon) << "dev.device_type: " << dev.device_type;
CHECK(dev.device_type == kDLHexagon) << "dev.device_type: " << dev.device_type;
FreeDataSpace(dev, ptr);
}

Expand Down
3 changes: 1 addition & 2 deletions src/runtime/hexagon/hexagon_device_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,7 @@ class HexagonDeviceAPI final : public DeviceAPI {
*/
bool IsValidDevice(DLDevice dev) {
// Added kDLCPU since we use hexagon as a sub-target of LLVM which by default maps to kDLCPU
return (TVMDeviceExtType(dev.device_type) == kDLHexagon) ||
(DLDeviceType(dev.device_type) == kDLCPU);
return (dev.device_type == kDLHexagon) || (dev.device_type == kDLCPU);
}

//! \brief Manages runtime HexagonBuffer allocations
Expand Down

0 comments on commit 90ed6f8

Please sign in to comment.