Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give DLDeviceType a sentinel value #111

Open
cconvey opened this issue Oct 21, 2022 · 1 comment
Open

Give DLDeviceType a sentinel value #111

cconvey opened this issue Oct 21, 2022 · 1 comment

Comments

@cconvey
Copy link

cconvey commented Oct 21, 2022

The DLDeviceType enum enumerates the list of device types.

The TVM project defines another enumeration, TVMDeviceExtType, that provides a supplemental set of devices / enumerators. It's important that there's no overlap of the integers provided by DLDeviceType and TVMDeviceExtType.

Unfortunately there's currently no good mechanism to notice when changes to either project lead to both using the same integer value in those enumerations.

We could address this by adding a sentinel value to DLDeviceType, e.g.:

typedef enum {
  kDLDeviceType_Begin = 1,
  kDLCPU = kDLDeviceType_Begin,
   ...
  kDLWebGPU = 15,
  /*! \brief Qualcomm Hexagon DSP */
  kDLHexagon = 16,
  kDLDeviceType_End, // all DLDeviceType enumerators are guaranteed to be numerically lower than this integer
} DLDeviceType;   

With this in place, TVM could safely avoid problems using something like this:

typedef enum {
  kDLAOCL = kDLDeviceType_End,
  kDLSDAccel,
  kOpenGL,
  kDLMicroDev,
  kDLWebGPU,
  // AddExtraTVMType which is not in DLPack here
} TVMDeviceExtType;

or this:

typedef enum {
  kDLAOCL = ...,
  ...
} TVMDeviceExtType;

// Relies on kDLAOCL having the lowest integer value in TVMDeviceExtType.
static_assert(KDLAOCL > kDLEnumEnd);
``
@cconvey
Copy link
Author

cconvey commented Oct 24, 2022

Update: I noticed that TVM also assumes that:

  • DLDeviceType does not define enumerators for -1 and 0, but
  • the compiler does pick an underlying data type that's capable of expressing -1 and 0.

Probably the right solution is for TVM to stop making such strong assumptions about this enumeration, especially in code that needs to compile as C rather than C++.

But if DLPack wants to help TVM avoid problems, then this enumeration should provide both kDLDeviceType_Begin and kDLDeviceType_End.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant