From 47819608bc0aacf8f8d3c3eb6f4f8c598742211a Mon Sep 17 00:00:00 2001 From: tqchen Date: Sun, 12 Feb 2023 11:46:59 -0500 Subject: [PATCH] Fix out of bound enum conversion This PR fixes Wenum-constexpr-conversion which is introduced in recent version of clang. Explciitly use integer instead of enum to store the device that may contain invalid options. --- include/tvm/runtime/ndarray.h | 8 -------- include/tvm/target/virtual_device.h | 18 ++++++++++++++---- src/target/virtual_device.cc | 12 ++++++------ 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/include/tvm/runtime/ndarray.h b/include/tvm/runtime/ndarray.h index b6a4cfe453c1..119d0f7fd339 100644 --- a/include/tvm/runtime/ndarray.h +++ b/include/tvm/runtime/ndarray.h @@ -42,14 +42,6 @@ namespace tvm { // alias DLDevice using Device = DLDevice; -// A 'null' device type, does not correspond to any DLDeviceType enum. -// TODO(mbs): This is to help us as we transition away from representing the 'homogenous' case -// as a singleton target map indexed by the invalid DLDeviceType '0'. -constexpr DLDeviceType kNullDeviceType = static_cast(0); - -// An 'invalid' device type, does not correspond to any DLDeviceType enum. -constexpr DLDeviceType kInvalidDeviceType = static_cast(-1); - namespace runtime { /*! diff --git a/include/tvm/target/virtual_device.h b/include/tvm/target/virtual_device.h index c26ae5befe66..9d8c91403309 100644 --- a/include/tvm/target/virtual_device.h +++ b/include/tvm/target/virtual_device.h @@ -44,6 +44,16 @@ namespace tvm { */ using MemoryScope = String; +// NOTE: cannot use enum as they are out of bound of the original enum +// and results in an undefined behavior +// A 'null' device type, does not correspond to any DLDeviceType enum. +// TODO(mbs): This is to help us as we transition away from representing the 'homogenous' case +// as a singleton target map indexed by the invalid DLDeviceType '0'. +constexpr int kNullDeviceType = 0; + +// An 'invalid' device type, does not correspond to any DLDeviceType enum. +constexpr int kInvalidDeviceType = -1; + /*! * \brief Describes at compile time the constraints on where data is to be stored at runtime * down to the (virtual) device and memory scope level, and how to compile code to compute that @@ -229,7 +239,7 @@ class VirtualDeviceNode : public AttrsNode { * Physical Devices" above. */ Device ToDevice() const { - ICHECK(device_type() != kInvalidDeviceType); + ICHECK(device_type_int != kInvalidDeviceType); ICHECK(virtual_device_id != -1); Device device; device.device_type = device_type(); @@ -262,7 +272,7 @@ class VirtualDevice : public ObjectRef { public: /*! * \brief Construct a virtual device. - * \param device_type The device type for the virtual device, or \p kInvalidDeviceType if + * \param device_type_int The device type for the virtual device, or \p kInvalidDeviceType if * unconstrained. If \p target is defined then must match its \p target->GetTargetDeviceType(). * \param virtual_device_id The device id for the virtual device, or -1 if unconstrained. * \param target The target describing how to compile for the virtual device, or null if @@ -271,7 +281,7 @@ class VirtualDevice : public ObjectRef { * unconstrained. * \return The virtual device. */ - explicit VirtualDevice(DLDeviceType device_type = kInvalidDeviceType, int virtual_device_id = -1, + explicit VirtualDevice(int device_type_int = kInvalidDeviceType, int virtual_device_id = -1, Target target = {}, MemoryScope memory_scope = {}); /*! \brief Returns the unique fully unconstrained \p VirtualDevice. */ @@ -349,7 +359,7 @@ class VirtualDevice : public ObjectRef { class VirtualDeviceCache { public: /*! \brief Returns the unique \p VirtualDevice representing given fields. */ - VirtualDevice Make(DLDeviceType device_type = kInvalidDeviceType, int virtual_device_id = -1, + VirtualDevice Make(int device_type = kInvalidDeviceType, int virtual_device_id = -1, Target target = {}, MemoryScope memory_scope = {}); /*! diff --git a/src/target/virtual_device.cc b/src/target/virtual_device.cc index 39bb11ff157b..3842776a6fd4 100644 --- a/src/target/virtual_device.cc +++ b/src/target/virtual_device.cc @@ -66,13 +66,13 @@ TVM_STATIC_IR_FUNCTOR(ReprPrinter, vtable) p->stream << ")"; }); -VirtualDevice::VirtualDevice(DLDeviceType device_type, int virtual_device_id, Target target, +VirtualDevice::VirtualDevice(int device_type_int, int virtual_device_id, Target target, MemoryScope memory_scope) { - ICHECK(!target.defined() || device_type == target->GetTargetDeviceType()) + ICHECK(!target.defined() || device_type_int == target->GetTargetDeviceType()) << "target " << target->ToDebugString() << " has device type " - << target->GetTargetDeviceType() << " but virtual device has device type " << device_type; + << target->GetTargetDeviceType() << " but virtual device has device type " << device_type_int; auto node = make_object(); - node->device_type_int = device_type; + node->device_type_int = device_type_int; node->virtual_device_id = virtual_device_id; node->target = std::move(target); node->memory_scope = std::move(memory_scope); @@ -166,8 +166,8 @@ VirtualDevice VirtualDevice::Default(const VirtualDevice& lhs, const VirtualDevi defaulted_memory_scope); } -VirtualDevice VirtualDeviceCache::Make(DLDeviceType device_type, int virtual_device_id, - Target target, MemoryScope memory_scope) { +VirtualDevice VirtualDeviceCache::Make(int device_type, int virtual_device_id, Target target, + MemoryScope memory_scope) { VirtualDevice prototype(device_type, virtual_device_id, std::move(target), std::move(memory_scope)); if (prototype->IsFullyUnconstrained()) {