[cfe-dev] OpenCL support

[Peter Collingbourne]

Hi Anton,

On Fri, Dec 10, 2010 at 06:21:54PM -0000, Anton Lokhmotov wrote:
> 2) We've introduced a new function  *getLangOptionsPtrUnsafe(), because the
> OpenCL pragmas handler must be able to modify the language options for the
> current context, and LangOptions structure is the most natural place to
> store the OpenCL EXTENSIONS and FP_CONTRACT states.  Given such a use,
> perhaps constness should be taken away from the getLangOptions()?

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.
The approach taken for other pragmas that affect semantic analysis is
to store data in the Sema object, and I think that is what we should
do here.  For example, look at how "#pragma pack" is implemented.

> Please review and comment.

Here are some initial comments:

> @@ -1192,6 +1192,8 @@ private:
>    bool IsTrivial : 1; // sunk from CXXMethodDecl
>    bool HasImplicitReturnZero : 1;
>  
> +  bool IsOpenCLKernel : 1;
> +
>    /// \brief End part of this FunctionDecl's source range.
>    ///
>    /// We could compute the full range in getSourceRange(). However, when we're

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.
Fields are quick to access but they increase the size of every
FunctionDecl object (we're using only 12 bits at the moment but may
wish to use more in future).  The __kernel qualifier is a relatively
uncommon qualifier and does not need to be accessed often so it should
be added as an attribute.

> @@ -230,7 +232,7 @@ KEYWORD(__func__                    , KEYALL)
>  
>  // C++ 2.11p1: Keywords.
>  KEYWORD(asm                         , KEYCXX|KEYGNU)
> -KEYWORD(bool                        , BOOLSUPPORT|KEYALTIVEC)
> +KEYWORD(bool                        , BOOLSUPPORT|KEYALTIVEC|KEYOPENCL)
>  KEYWORD(catch                       , KEYCXX)
>  KEYWORD(class                       , KEYCXX)
>  KEYWORD(const_cast                  , KEYCXX)
> @@ -238,7 +240,7 @@ KEYWORD(delete                      , KEYCXX)
>  KEYWORD(dynamic_cast                , KEYCXX)
>  KEYWORD(explicit                    , KEYCXX)
>  KEYWORD(export                      , KEYCXX)
> -KEYWORD(false                       , BOOLSUPPORT|KEYALTIVEC)
> +KEYWORD(false                       , BOOLSUPPORT|KEYALTIVEC|KEYOPENCL)
>  KEYWORD(friend                      , KEYCXX)
>  KEYWORD(mutable                     , KEYCXX)
>  KEYWORD(namespace                   , KEYCXX)
> @@ -252,7 +254,7 @@ KEYWORD(static_cast                 , KEYCXX)
>  KEYWORD(template                    , KEYCXX)
>  KEYWORD(this                        , KEYCXX)
>  KEYWORD(throw                       , KEYCXX)
> -KEYWORD(true                        , BOOLSUPPORT|KEYALTIVEC)
> +KEYWORD(true                        , BOOLSUPPORT|KEYALTIVEC|KEYOPENCL)
>  KEYWORD(try                         , KEYCXX)
>  KEYWORD(typename                    , KEYCXX)
>  KEYWORD(typeid                      , KEYCXX)

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;

Thanks,
-- 
Peter