[PATCH] Don't include <stddef.h>

Including a standard or system header isn't allowed in OpenCL.

The type "size_t" needs to be explicitely defined now.

This will only work correctly for 32-bit targets. This should be typedef __SIZE_TYPE__ size_t to be portable

Including a standard or system header isn't allowed in OpenCL.

The type "size_t" needs to be explicitely defined now.

v2: Use __SIZE_TYPE__ instead of unsigned int.

This means we loose support for ptrdiff_t (typedef __PTRDIFF_TYPE__ ptrdiff_t;), which is an OpenCL required type.

Moreover, we loose NULL in this way. Although NULL is not strictly required, it’s useful to have around.

Jeroen

This means we loose support for ptrdiff_t (typedef __PTRDIFF_TYPE__ ptrdiff_t;), which is an OpenCL required type.

Moreover, we loose NULL in this way. Although NULL is not strictly required, it’s useful to have around.

Jeroen

Everything that stddef typically provides needs to be copied somewhere into libclc’s headers. This patch should probably include the other integer types, and NULL

This means we loose support for ptrdiff_t (typedef __PTRDIFF_TYPE__
ptrdiff_t;), which is an OpenCL required type.

I'll add that type.

Moreover, we loose NULL in this way. Although NULL is not strictly
required, it’s useful to have around.

Are you fine with the following #define?

#define NULL ((void*)0)

Including a standard or system header isn't allowed in OpenCL.

I’m not sure this is correct. Could you point me to the specific text that states this? In the OpenCL 1.2 spec I can only find is 6.9.f which does not mention stddef.h.

Thanks,

Jeroen

6.9.f states

  1. The library functions defined in the C99 standard headers assert.h, ctype.h, complex.h, errno.h, fenv.h, float.h, inttypes.h, limits.h, locale.h, setjmp.h, signal.h, stdarg.h, stdio.h, stdlib.h, string.h, tgmath.h, time.h, wchar.h and wctype.h are not available and cannot be included by a program.

That’s what I found, but that doesn’t mention stddef.h. So I don’t quite see why we cannot include stddef.

Jeroen

To be honest, I didn't read the specification thoroughly. The build of
libclc fails on FreeBSD if stddef.h is included, because a header pulled
by stddef.h defines variadic macros. Those macros are not supported by
OpenCL according to clang.

I then asked on the #llvm IRC channel how I could exclude those macros
when building an OpenCL program. I have been told the problem rather
comes from libclc which shouldn't include any standard or system C header.

I then removed that #include, defined size_t and it built.

That’s what I found, but that doesn’t mention stddef.h. So I don’t quite see why we cannot include stddef.

Jeroen

That seems clearly like an accident. OpenCL defines all of the standard library functions provided as “builtins”. There’s no need to include a header, and the implementation relying on system provided headers isn’t a great plan.

This means we loose support for ptrdiff_t (typedef __PTRDIFF_TYPE__
ptrdiff_t;), which is an OpenCL required type.

I'll add that type.

Moreover, we loose NULL in this way. Although NULL is not strictly
required, it’s useful to have around.

Are you fine with the following #define?

#define NULL ((void*)0)

That seems to be the one that is currently picked up from stddef. So, yes.

Jeroen

I’m not sure if that will work correctly with address spaces. There is some null predefined (__null maybe)? which might be required

That’s what I found, but that doesn’t mention stddef.h. So I don’t quite see why we cannot include stddef.

Jeroen

That seems clearly like an accident. OpenCL defines all of the standard library functions provided as “builtins”. There’s no need to include a header, and the implementation relying on system provided headers isn’t a great plan.

I think I always assumed that the clang/llvm version of stddef.h was picked up during compilation (which just defines, size_t, ptrdiff_t, and NULL; and which seems to be perfectly safe). However, this is apparently a misunderstanding from my part, as Jean-Sebastien mentions in his email that the standard FreeBSD version is picked up in his case. So, yes, I agree that ripping out the include is probably right thing to do.

Jeroen

I think clang has be taught not to complain about the address space in case of ((void*)0)? I could not trigger any warning, but I might simply not know how to set the warning level sufficiently high enough.

I don’t think __null exists.

Jeroen

Including a standard or system header isn't allowed in OpenCL.

The type "size_t" needs to be explicitely defined now.

v2: Use __SIZE_TYPE__ instead of unsigned int.
v3: Define ptrdiff_t and NULL.

Including a standard or system header isn't allowed in OpenCL.

The type "size_t" needs to be explicitely defined now.

v2: Use __SIZE_TYPE__ instead of unsigned int.
v3: Define ptrdiff_t and NULL.

Oops, ignore this patch, __null doesn't exist. Consider the next patch :slight_smile:

LGTM.

Jeroen

If the patch is fine, can someone please commit it? :slight_smile: