kenba/opencl3

Missing enumeration types?

Closed this issue · 3 comments

The method opencl3::device::get_device_ids takes a u64 for the device_type while the underlying method clGetDeviceIDs takes an enum for device_type.

Why here and in other similar circumstances is it being obfuscated to a u64 when using an enum type would be far clearer?

kenba commented

Thank you for your observation @JonathanWoollett-Light.

The underlying method clGetDeviceIDs takes a cl_device_type for device_type , i.e. a cl_bitfield . The reason

Why here and in other similar circumstances is it being obfuscated to a u64

is because bindgen converts the cl_bitfield in cl.h to a u64.

Furthermore, the values of cl_device_type are defined in cl.h as:

/* cl_device_type - bitfield */
#define CL_DEVICE_TYPE_DEFAULT                      (1 << 0)
#define CL_DEVICE_TYPE_CPU                          (1 << 1)
#define CL_DEVICE_TYPE_GPU                          (1 << 2)
#define CL_DEVICE_TYPE_ACCELERATOR                  (1 << 3)
#ifdef CL_VERSION_1_2
#define CL_DEVICE_TYPE_CUSTOM                       (1 << 4)
#endif
#define CL_DEVICE_TYPE_ALL                          0xFFFFFFFF

because some of the values may be the may be ored in a query, see: OpenCL API: Querying Devices.

cl_device_type is not being obfuscated to a u64, it is a u64.
However, there are other types which would be clearer as enum types, such as cl_platform_info or cl_device_info.
In fact they were enum types in previous versions of the library, but keeping them up to date with new OpenCL extensions became too onerous, so the enum types were removed. See kenba/cl3#13.

I am familiar with this form of implementing options in C using bit-fields but it is a poor user interface when more explicit alternatives are available.

If the user can only query for one of the device types:

enum DeviceType {
    Default = (1 << 0),
    Cpu = (1 << 1),
    Gpu = (1 << 2),
    Accelerator = (1 << 3),
    Custom = (1 << 4),
    All = 0xFFFFFFFF
}
fn get_device_ids(device: DeviceType) {
    let device = device as u64;
}
fn main() {
    get_device_ids(DeviceType::Cpu);
}

If the user can query for multiple of these types:

struct DeviceType(u64);
impl DeviceType {
    pub fn new() -> Self {
        Self(0)
    }
    /// Queries for: The default OpenCL device in the system.
    pub fn default(self) -> Self {
        Self(self.0 | (1<<0))
    }
    /// Queries for: An OpenCL device that is the host processor. The host processor runs the OpenCL 
    /// implementations and is a single or multi-core CPU.
    pub fn cpu(self) -> Self {
        Self(self.0 | (1 << 1))
    }
    /// Queries for: An OpenCL device that is a GPU. By this we mean that the device can also be used to 
    /// accelerate a 3D API such as OpenGL or DirectX.
    pub fn gpu(self) -> Self {
        Self(self.0 | (1 << 2))
    }
    // ...
    /// Queries for: All OpenCL devices available in the platform, except for CL_DEVICE_TYPE_CUSTOM devices.
    pub fn all() -> Self {
        Self(0xFFFFFFFF)
    }
    fn unwrap(self) -> u64 {
        self.0
    }
}
fn get_device_ids(device: DeviceType) {
    let device = device.unwrap();
}
fn main() {
    get_device_ids(DeviceType::new().cpu().gpu());
}

I'm still learning so I'm not 100% sure which case applies here, but I know in either case there is a more explicit definition than a simple u64. Both examples here restrict the input to only valid values and offer a more explicit definition of the parameter.

In fact they were enum types in previous versions of the library, but keeping them up to date with new OpenCL extensions became too onerous, so the enum types were removed. See kenba/cl3#13.

Notably here by using a new type pattern with a public inner u64 like DeviceType(pub u64); you can both illustrate and make explicit typical usage while allowing the user the same access if it necessary (this can effectively applied to all bit fields and enum types via minor alterations to this pattern).

The easiest simple improvement could be specific scoping:

mod DeviceType {
    pub const DEFAULT: u64 = (1 << 0);
    pub const CPU: u64 = (1 << 1);
    // ...
}
fn get_device_ids(device: u64) {
}
fn main() {
    get_device_ids(DeviceType::CPU | DeviceType::DEFAULT);
}
kenba commented

Thank you @JonathanWoollett-Light for your opinion on the: "poor user interface ... of implementing options in C using bit-fields".

However, I believe that you are overlooking the fundamental purpose of this library: it is a Rust interface to the OpenCL C API, so it conforms to the OpenCL C API, which uses bit-fields.

You have quoted my reference to issue kenba/cl3#13, but it seems that you either haven’t read it or you didn’t understand it.
In short, it is possible to provide more explicit definitions of these types in Rust, but doing so is a maintenance overhead. So I explicitly chose not to provide more explicit definitions of these types.

I'm sorry that you don't like my choice. But I hope that you can at least understand it, or even learn to appreciate it.