OpenCL-Headers icon indicating copy to clipboard operation
OpenCL-Headers copied to clipboard

Signed and Unsigned Mismatches

Open bashbaug opened this issue 6 years ago • 1 comments

One of our engineers reported an issue in the OpenCL headers recently regarding signed and unsigned mismatch compiler warnings in some cases with modern compilers at higher warning levels. The crux of the issue is that many of the OpenCL types are unsigned, e.g.:

typedef cl_uint             cl_command_type;

But, the corresponding enums are signed, e.g.:

/* cl_command_type */
#define CL_COMMAND_NDRANGE_KERNEL                   0x11F0
#define CL_COMMAND_TASK                             0x11F1
#define CL_COMMAND_NATIVE_KERNEL                    0x11F2
...

This can lead to an error in code like the following (see godbolt example here):

#define MY_VALUE 0x10

template <typename T1, typename T2>
bool is_eq(const T1& lhs, const T2& rhs) {
    if (lhs == rhs) {
        return true;
    }
    return false;
}

void f() {
    unsigned x = 2;
    is_eq(x, MY_VALUE);
}

This seems like a rather narrow use-case, but it's actually fairly common in googletest unit tests:

const cl_command_type command_type;
clGetEventInfo(event, CL_EVENT_COMMAND_TYPE, sizeof(command_type), &command_type, nullptr);
EXPECT_EQ(CL_COMMAND_TYPE_NDRANGE_KERNEL, command_type);

Should we define most OpenCL enums - all but error codes I think? - as explicitly unsigned? I don't think this will break existing code, and it will fix this compile warning:

/* cl_command_type */
#define CL_COMMAND_NDRANGE_KERNEL                   0x11F0u
#define CL_COMMAND_TASK                             0x11F1u
#define CL_COMMAND_NATIVE_KERNEL                    0x11F2u
...

bashbaug avatar Aug 09 '19 17:08 bashbaug

This seems a sensible change. I wondered if it might create new warnings for existing printf code that uses "%d" to print these values, but a quick experiment suggests not.

alycm avatar Sep 02 '19 17:09 alycm