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 6aac36f
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 18 deletions.
39 changes: 32 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,39 @@ 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
*
* Additional enumerators to supplement those provided by
* DLPack's `DLDeviceType` enumeration.
*
* MAINTAINERS NOTE #1: We need to ensure that the two enumerations
* don't accidentally use the same integer value.
* 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: As of DLPack v0.7, the definition for
* `DLDeviceType` specifies an underlying storage type of
* `int32_t`. That guarntees that a variable of type
* `DLDeviceType` is capable of holding any of the integers
* provided by these two enumerations.
* However, the `int32_t` specification only applies when the
* header file is compiled as C++, so this may have a latent
* bug when compiled as C code.
*/
#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 6aac36f

Please sign in to comment.