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.

-erik

-- 
Erik Schnetter <[email protected]>
http://www.perimeterinstitute.ca/personal/eschnetter/

My email is as private as my paper mail. I therefore support encrypting
and signing email messages. Get my PGP key from http://pgp.mit.edu/.

Attachment: OpenCL-long.diff
Description: Binary data

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to