[cfe-dev] [OpenCL patch] kernel attributes implementation

[Pekka Jääskeläinen]

Hi,

I spotted that the "noinline" attribute for kernels is still
added in TCE's targetinfo. It might not be needed anymore
but I'll check and submit a separate patch if that's the case.
(It prevents inlining also when the kernel function is
called from another kernel function in which case inlining
might be desirable?)

A slight stylistic issue:

   else if(AttrName->isStr("vec_type_hint")) {
+    if(T.get() && !T.isInvalid()) {
+      ExprResult ArgExpr = Actions.ActOnDummyTypeExpr(T.get());
+      ArgExprs.push_back(ArgExpr.release());
+    }
+  }

I think the correct style is to have a whitespace after 'if'?

Similar here:
+      }else {

Otherwise, this looks good to me now (to my eyes that are
not very familiar with the Clang code base).

+1 for committing after these cleanups.

02.01.2012 17:52, Benyei, Guy kirjoitti:
> Hi Pekka,
> Attached the updated patch - this one implements the whole kernel metadata in CodeGenFunction.cpp (also moved the reqd_work_group_size attribute from the tce target info).
>
> Please review this new patch.
>
> Thanks
>     Guy
>
>
> -----Original Message-----
> From: Pekka Jääskeläinen [mailto:pekka.jaaskelainen at tut.fi]
> Sent: Monday, January 02, 2012 13:13
> To: Benyei, Guy
> Cc: cfe-dev at cs.uiuc.edu
> Subject: Re: [cfe-dev] [OpenCL patch] kernel attributes implementation
>
> Hi Guy,
>
> Could you change your patch to implement the attributes for
> all targets like discussed below? Otherwise I have to soon
> resubmit this work to make these features work for all
> targets supported by pocl (which covers all "non-SIMT" LLVM targets
> at the moment) which feels a bit like wasted rework.
>
> I originally implemented the reqd_work_group_size attribute
> as metadata for all targets (instead of the TCE target specific
> implementation) which was rejected. But as the new discussion
> didn't bring any reasons against having these attributes as
> metadata by default, I'd propose resubmitting your patches with
> generic implementation next to the opencl.kernels metadata.
>
> The actual OpenCL implementations can ignore the spurious
> metadata at will or convert it to intrinsics or whatever
> is the best for the case. Certainly the harm should be
> insignificant in such cases. The immediate benefit is that
> these attributes can then be supported by (at least) the pocl
> OpenCL implementation for all LLVM targets.
>
> 20.12.2011 13:12, Benyei, Guy kirjoitti:
>> Hi Pekka,
>>
>> I agree that the right place to implement OpenCL specific metadata
>> shouldn't be in target specific, since the attributes are not target
>> specific. IMO, the right solution would be to implement this metadata
>> in CodeGenFunction.cpp, next to the implementation of the
>> "opencl.kernels" metadata.
>>
>> Anyway, I chosen to implement is for the TCE target only due to the
>> fact that reqd_work_group_size is implemented in this target only. It
>> can be changed, of course.
>>
>> Thanks Guy
>>
>> -----Original Message----- From: cfe-dev-bounces at cs.uiuc.edu
>> [mailto:cfe-dev-bounces at cs.uiuc.edu] On Behalf Of Pekka J??skel?inen
>> Sent: Tuesday, December 20, 2011 12:50 To: cfe-dev at cs.uiuc.edu
>> Subject: Re: [cfe-dev] [OpenCL patch] kernel attributes
>> implementation
>>
>> On 12/20/2011 11:18 AM, Benyei, Guy wrote:
>>> I've implemented the handling of these new attributes in the TCE
>>> target info, since reqd_work_group_size attribute is already
>>> implemented there. I also use this target for testing.
>>
>> Thanks! :)
>>
>> There was some discussion about implementing these OpenCL C kernel
>> attributes some time ago [1]. The discussion somehow died.
>>
>> The question raised there was why cannot we add these by default
>> when compiling OpenCL? This would enable pocl [2] to support these
>> OpenCL features for all LLVM targets. The OpenCL implementations that
>> implement the attributes in some other way can just ignore the
>> metadata (except the required work group size cannot be just ignored,
>> AFAIK).
>>
>> [1]
>> http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-December/046204.html
>> [2] https://launchpad.net/pocl
>>
>
>


-- 
Pekka Jääskeläinen