OpenCL C "long" should always have 64 bits

[Michele Scandale]

On 08/31/2013 09:05 PM, Erik Schnetter wrote:
> OpenCL C requires that the type "long" always has 64 bits, independent of the architecture. Without proper support for "long", "double" can also not be supported in OpenCL C, so correcting this is important.
>
> A patch to implement this was suggested some time ago, but was not committed. I have extended this patch, and tested it on both x86_64 and i386. I attach the patch below.
>
> This patch consists of four hunks. The first uses PointerWidth instead of LongWidth to determine whether an architecture uses 32-bit or 64-bit registers. Using LongWidth does not work with OpenCL C since long there has always 64 bits, whereas I expect all architectures to use the same number of bits for a C long and a C pointer.
>
> The second hunk overrides some target-specific choices for type widths in TargetInfo::setForcedLangOptions when the language is OpenCL C. These widths are required by OpenCL C.
>
> As extension to OpenCL C, I have set long long to be a 128-bit type. This is described by the OpenCL standard in a "reserved for future use" section. The choice would be either to disable long long (how?), or to make it a 128-bit type. The latter seemed easier.
>
> The last two hunks set two command-line options that are also required for OpenCL C: "char" is always signed, and "errno" does not exist. Other parts of Clang artificially set command line options depending on the target architecture (and independent of the language), so this patch ignores the respective command line options when the language is OpenCL C. In the future, "-fno-builtins" may also be added to this -- I am not quite sure what this option does or why it is necessary, but we currently always use it when compiling OpenCL C code.
>
>
> The original patch caused some discussion, and I want to answer to some of the points raised in this discussion:
>
> (1) One suggestion is that changing sizeof(long) is actually defining a new target triple, and should be implemented as such. I want to disagree with this. First, OpenCL kernels do not have access to libc, operating system calls, or anything where the "usual" ABI for a system would matter -- OpenCL kernels are self-contained. Thus, introducing a new target triple seems overkill. Second, all that OpenCL C does is use the name "long" for a type that is called "long long" in C. The name used in the source code of a language is independent of the ABI. This is also evident from the fact that this patch only changes Clang, and not LLVM -- the emitted byte code will use the type "i64", and the back-end never knows whether the type was called "long" or "long long" in the source code.
>
> (2) At another point, the question was raised whether changing the meaning of "long" could break the target-lowering code that converts from byte code to machine language. Again, this seems impossible since byte code contains no reference to the type name, and uses integer and bit-widths only.
>
> As to whether setForcedLangOptions is a good place for this patch -- I don't know, as I am not an expert in Clang. Other places seem possible, e.g. the TargetInfo constructors. However, putting the code there would mean that (a) Opts.OpenCL needs to be available there (it currently isn't), and (b) all TargetInfo constructors need to remain in sync with the OpenCL C specification.
>
> One suggestion was to split TargetInfo into two, creating a new class LanguageInfo, where presumably C and C++ would look at TargetInfo to choose their size for "long", whereas OpenCL would not. This may be elegant, but given that the current patch has only about 20 lines I wonder whether this is overkill as well. To think about this, maybe one should consider other potential future languages for Clang (CUDA? UPC?).
>
>
> I would appreciate feedback.
>

You should redefine also IntWidth, IntAlign, LongLongWidth and LongLongAlign:
they are also fixed.

I not sure if stddef.h can be included or not in OpenCL (this header file does
not appear in the list of standard header files that cannot be included), but if
it is possible, then WCharType and WIntType should be redefined too.

Currently OpenCL supports only architectures with 32bit or 64 bit addresses and
specify that size_t, ptrdiff_t, intptr_t, uintptr_t must be integers with size
that match the address bit size, so your approach seems fine.

The address space map IMO should not be imposed, because it represents how a
given OpenCL address space should be mapped to target address space that the
target backend is able to handle.

Regards,
-Michele