[cfe-dev] OpenCL support

[Anton Lokhmotov]

Hi Peter and Mike,

Many thanks for your comments!

> > @@ -1192,6 +1192,8 @@ private:
> >    bool IsTrivial : 1; // sunk from CXXMethodDecl
> >    bool HasImplicitReturnZero : 1;
> >
> > +  bool IsOpenCLKernel : 1;
> 
> A better way of storing this information would be to use an attribute
> (Attr subclass).  See the include/clang/Basic/Attr.td file.
> ...
> We should only add a new field if we expect it to be used often.

We have considered this option but attributes have too complicated a
semantics which would make handling this qualifier unwieldy.  Bit-fields
probably get stored in a 32-bit word, which means there are 20 more unused
bits in every FunctionDecl object anyway.  We do use Clang attributes for
the optional function qualifiers:
   * __attribute__((vec_type_hint(<typen>)))
   * __attribute__((work_group_size_hint(X, Y, Z)))
   * __attribute__((reqd_work_group_size(X, Y, Z)))

Mike Gist wrote:
> You could also consider placing all kernel functions in a 'kernel'
> section, or adding a function attribute for kernels.
Unlike in Clang, function attribute bit-fields in LLVM are pretty crowded
(only couple of bits are unused?).  Besides, we do not want to represent
differently the kernel qualifier and optional kernel qualifiers, which
require storing <typen> and X, Y, Z values. (I don't even want to think how
one would mangle the optional qualifiers into a kernel name and then
demangle.)

That's why we propose to use metadata for this purpose.  Does anyone have a
better idea?

Peter Collingbourne wrote:
> I don't think it is a good idea to use LangOptions for this.
> LangOptions is an input parameter so we shouldn't modify it during
> parsing/semantic analysis.  Modifying the LangOptions could also affect
> clients which reuse a LangOptions object expecting it to be unchanged.
We have addressed this by using an OpenCLOptions struct in Preprocessor.

Mike Gist wrote:
> cl_khr_byte_addressable_store is probably a fairly well supported 1.0
> extension that you could add too :) Those will have to be disabled
> by default for 1.0 anyhow.
Done (+ 32-bit atomics).

Peter Collingbourne wrote:

> > @@ -230,7 +232,7 @@ KEYWORD(__func__                    , KEYALL)
> >
> >  // C++ 2.11p1: Keywords.
> >  KEYWORD(asm                         , KEYCXX|KEYGNU)
> > -KEYWORD(bool                        , BOOLSUPPORT|KEYALTIVEC)
> > +KEYWORD(bool                        , BOOLSUPPORT|KEYALTIVEC|KEYOPENCL)
> > ...
> Isn't this rendered unnecessary by this code? (from
> lib/Frontend/CompilerInvocation.cpp):
> 
> > // OpenCL and C++ both have bool, true, false keywords.
> > Opts.Bool = Opts.OpenCL || Opts.CPlusPlus;
Indeed.  We have removed this from our patch.  (The drawback is that the
user of the OpenCL mode will also need to turn on the Bool mode if she
doesn't use CompilerInvocation.  For AltiVec, bool gets supported
automatically.)

Please find attached the rework of the first patch plus the second patch on
OpenCL keywords.

Many thanks,
Anton.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 00002-keywords.patch
Type: application/octet-stream
Size: 13640 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20101216/bf401cc2/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 00001-kernel-extensions-pragmas.patch
Type: application/octet-stream
Size: 17474 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20101216/bf401cc2/attachment-0001.obj>