[cfe-dev] OpenCL support

[Krister Wombell]

Some observations about storing the kernel information in metadata:


In our CL implementation at Ziilabs we represented all the kernel attributes
as separate NamedMDNodes.  The kernel name is concatenated with a set of
known postfixes. For example 'kernel foo(…)' produces nodes with names
'foo_is_kernel', 'foo_work_group_size_hint', 'foo_vec_type_hint' etc. The
MDNodes carry Values appropriate to the attribute type (e.g. 3 integers for
the work_group_size_hint).


At any point passes (including machine passes) can lookup up whatever
interesting attribute it wants and obtain the values in a fairly
straightforward manner.


I think this tends to work better than storing the names of kernels in a
list hanging off from one NamedMDNode:

(1) passes can just concatenate the name of the function with a postfix and
look up the metadata, there's no second phase of then searching through a
list to find the name of the function it's interested in.

(2) it's not obvious how you're going to extend the list approach to carry
attribute values. Suppose you add support for the work_group_size_hint (it
carries 3 integer values) how are you proposing that the presence of the
attribute and it's data be represented?


Thanks, Krister.



On Wed, Dec 22, 2010 at 11:54 PM, Anton Lokhmotov
<Anton.Lokhmotov at arm.com>wrote:

> Hi Peter,
>
> > There are several examples of existing attributes in the Clang source
> > code which you should be able to follow.  Perhaps you can explain in
> > more detail why you cannot use attributes here?
>
> We feared that too much code would be generated when using attributes. (As
> you know, we must be cautious of anything negatively affecting the compiler
> footprint.)  However, we've re-implemented the kernel function qualifier
> using an attribute instead of a field.  Please review.
>
>
> > > +    // OpenCL1.1 enables these extensions by default
> > > +    cl_khr_global_int32_base_atomics =
> > > +      cl_khr_global_int32_extended_atomics =
> > > +      cl_khr_local_int32_base_atomics =
> > > +      cl_khr_local_int32_extended_atomics =
> > > +      cl_khr_byte_addressable_store = 1;
> >
> > I presume you misread Table 4.3 from the spec, which states
> > that those extensions must be supported, which is not the same
> > thing as being enabled.
>
> Indeed.  They are disabled now.
>
>
> > Are you planning to have a mechanism for specifying which extensions
> > are supported by a particular implementation?  My initial thoughts
> > are that we can add a member function to TargetInfo which returns an
> > OpenCLOptions struct containing this information.
>
> Yes, something along these lines.
>
>
> > > +  unsigned cl_fp_contract : 1; // OpenCL FP_CONTRACT state.
> >
> > I'm not sure this should be an OpenCL-specific option.  In future
> > we would like to support #pragma STDC FP_CONTRACT, and ideally we'd
> > like to be able to reuse OpenCL's FP_CONTRACT support.  I'd suggest
> > just making this a Sema field.
> ...
> > You should split this PragmaHandler into separate handlers for
> > #pragma OPENCL FP_CONTRACT and #pragma OPENCL EXTENSION.  Then
> > there's no need to do the string matching yourself, and it will
> > be easier to reuse the FP_CONTRACT support.
>
> Good point.  We've refactored FP_CONTRACT support into a separate
> ParseFPContractPragma method.  Note that in OpenCL the default value is ON,
> whilst in C99 it is undefined.
>
>
> > Comments on the second patch to come...
> I'm attaching it again for convenience...
>
>
> Many thanks,
> Anton.
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20101223/01437af4/attachment.html>