[cfe-dev] OpenCL & SPIR specific types - proposal and patch
Eric Christopher
echristo at gmail.com
Tue Oct 9 12:37:12 PDT 2012
+ case BuiltinType::OCLEvent: {
+ if (OCLEvtTy.Verify())
+ return OCLEvtTy;
+ OCLEvtTy =
+ DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,
+ "opencl_event_t", TheCU,
getOrCreateMainFile(),
+ 0);
+ unsigned Size =
CGM.getContext().getTypeSize(CGM.getContext().VoidPtrTy);
+ OCLEvtTy = DBuilder.createPointerType(OCLEvtTy, Size);
+ return OCLEvtTy;
+ }
Are these actually all pointer to struct forward declared types in the
language that have no header that they'd normally be declared inside of?
Comments about these would be good, including the general layout.
Also yes, please try very hard to refactor these so that there is less
duplicated code.
-eric
On Tue, Oct 9, 2012 at 4:36 AM, Benyei, Guy <guy.benyei at intel.com> wrote:
> And now with unified line-endings…****
>
> ** **
>
> [image: email_signature_guy_new2]****
>
> ** **
>
> *From:* cfe-dev-bounces at cs.uiuc.edu [mailto:cfe-dev-bounces at cs.uiuc.edu] *On
> Behalf Of *Benyei, Guy
> *Sent:* Tuesday, October 09, 2012 13:26
> *To:* Richard Smith
> *Cc:* cfe-dev at cs.uiuc.edu; Anton.Lokhmotov at arm.com
>
> *Subject:* Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch
> ****
>
> ** **
>
> Richard,****
>
> Thanks for your very helpful review.****
>
> ** **
>
> Attached an updated patch – please review.****
>
> ** **
>
> Thanks****
>
> [image: email_signature_guy_new2]****
>
> ** **
>
> *From:* metafoo at gmail.com [mailto:metafoo at gmail.com <metafoo at gmail.com>] *On
> Behalf Of *Richard Smith
> *Sent:* Sunday, October 07, 2012 20:35
> *To:* Benyei, Guy
> *Cc:* Villmow, Micah; Tanya Lattner; Anton.Lokhmotov at arm.com;
> cfe-dev at cs.uiuc.edu
> *Subject:* Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch
> ****
>
> ** **
>
> +BUILTIN_TYPE(Image1d_T, Image1dTTy)****
>
> +BUILTIN_TYPE(Image1dArray_T, Image1dArrayTTy)****
>
> +BUILTIN_TYPE(Image1dBuffer_T, Image1dBufferTTy)****
>
> +BUILTIN_TYPE(Image2d_T, Image2dTTy)****
>
> +BUILTIN_TYPE(Image2dArray_T, Image2dArrayTTy)****
>
> +BUILTIN_TYPE(Image3d_T, Image3dTTy)****
>
> ** **
>
> The other builtin types corresponding to *_t keywords don't have the '_T'
> as part of the builtin name (Char16, Char32, WChar_S, WChar_U). Including
> the _T here is inconsistent. Also, how would you feel about giving these
> identifiers an OpenCL prefix, as we do for the ObjC builtin types?****
>
> ** **
>
> + bool isImage1dT() const; // OpenCL image1d_t****
>
> + bool isImage1dArrayT() const; // OpenCL image1d_t****
>
> + bool isImage1dBufferT() const; // OpenCL image1d_t****
>
> + bool isImage2dT() const; // OpenCL image2d_t****
>
> + bool isImage2dArrayT() const; // OpenCL image2d_t****
>
> + bool isImage3dT() const; // OpenCL image3d_t****
>
> ** **
>
> Some of these comments are wrong.****
>
> ** **
>
> +inline bool Type::isImage1dT() const {****
>
> + if (const BuiltinType *BT = dyn_cast<BuiltinType>(CanonicalType))****
>
> + return BT->getKind() == BuiltinType::Image1d_T;****
>
> + return false;****
>
> +}****
>
> ** **
>
> Please write this as "return
> isSpecificBuiltinType(BuiltinType::Image1d_T);"****
>
> ** **
>
> + Width = Target->getPointerWidth(0); // Currently these types are
> pointers****
>
> + Align = Target->getPointerAlign(0); // to opaque types****
>
> ** **
>
> Please put the comment by itself on the line before this (and add a full
> stop).****
>
> ** **
>
> + case BuiltinType::Event_T: Encoding = llvm::dwarf::DW_ATE_unsigned;****
>
> ** **
>
> It seems that you're giving these types the size and alignment of a
> pointer, but emitting debug info as if they're 'unsigned int'. Is that
> really the way this is supposed to work?****
>
> ** **
>
> + llvm::Type *ResultType;****
>
> +****
>
> + switch (cast<BuiltinType>(T)->getKind()) {****
>
> + default: break;****
>
> + case BuiltinType::Image1d_T:****
>
> + ResultType = llvm::PointerType::get(llvm::StructType::create(****
>
> + CGM.getLLVMContext(),"opencl.image1d_t"), 0);*
> ***
>
> + break;****
>
> ** **
>
> In your 'default' case, you return an uninitialized value. Please change
> this to return the PointerType directly, rather than pushing it through a
> local variable, and add an llvm_unreachable to your default case.****
>
> ** **
>
> + friend class CodeGenTypes;****
>
> [...]****
>
> + ResultType = CGM.OpenCLRuntime->convertOpenCLSpecificType(Ty);****
>
> ** **
>
> Please remove the 'friend' declaration and instead use
> CGM.getOpenCLRuntime() here.****
>
> ** **
>
> Please also add tests for the PCH serialization/deserialization of these
> types.****
>
> ** **
>
> On Sun, Oct 7, 2012 at 2:07 AM, Benyei, Guy <guy.benyei at intel.com> wrote:*
> ***
>
> Anton, ****
>
> I’ve fixed my patch according to your comments. I’ve also did some
> changes. See the patch attached.****
>
> ****
>
> Tanya,****
>
> I think the OpenCL specific types, including the sampler and the event
> types should be well distinguishable in IR format. The fact, that a 32 bit
> integer exposes a superset of the sampler functionality in some perspective
> is just a coincidence. The sampler type itself could contain additional
> information (for optimizations and other architecture specifics), and it
> doesn’t have to hold the initializer value in the bitwise or integer format.
> ****
>
> The backend should be able to recognize the sampler type from the IR.****
>
> In the attached patch, I’ve moved the OpenCL specific type conversion to
> the CGOpenCLRuntime class – this class was meant to be overloaded by OpenCL
> vendors to implement vendor specific behavior (codegen for OpenCL locals is
> implemented there), and so if a vendor prefers using the i32
> representation, he could implement it there. ****
>
> In the next increment this class should also contain a sampler
> initialization function, so sampler initialization could be implemented
> also in a vendor specific way.****
>
> ****
>
> Please review this new patch****
>
> ****
>
> Thanks****
>
> [image: email_signature_guy_new2]****
>
> ****
>
> *From:* Villmow, Micah [mailto:Micah.Villmow at amd.com]
> *Sent:* Thursday, October 04, 2012 22:15
> *To:* Tanya Lattner; Benyei, Guy
> *Cc:* cfe-dev at cs.uiuc.edu; Anton.Lokhmotov at arm.com
> *Subject:* RE: [cfe-dev] OpenCL & SPIR specific types - proposal and patch
> ****
>
> ****
>
> There needs to be a way to differentiate between an integer and a sampler
> by only looking at the type. The sampler itself is an opaque type in
> OpenCL. The only requirement is that it is initialized with a 32bit
> unsigned integer, not that the type itself is an integer.****
>
> ****
>
> Micah****
>
> ****
>
> *From:* Tanya Lattner [mailto:lattner at apple.com <lattner at apple.com>]
> *Sent:* Thursday, October 04, 2012 11:49 AM
> *To:* Benyei, Guy
> *Cc:* cfe-dev at cs.uiuc.edu; Villmow, Micah; Anton.Lokhmotov at arm.com
> *Subject:* Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch
> ****
>
> ****
>
> I'm hoping to have comments on this patch tomorrow, but since I have
> proposed several patches to Clang for the sampler type (and have another in
> revision).. can you explain why you want to change the type from an integer
> to a pointer? ****
>
> ****
>
> -Tanya****
>
> ****
>
> On Oct 3, 2012, at 8:06 AM, "Benyei, Guy" <guy.benyei at intel.com> wrote:***
> *
>
> ****
>
> I’d like to renew the discussion about making the OpenCL specific types
> first class citizens in Clang.****
>
> ****
>
> I think this change is required by the OpenCL specifications, since these
> type names are keywords of the OpenCL C language.****
>
> This change is also needed in order to enable efficient checking of OpenCL
> restrictions on these types (OpenCL 1.2 spec, section 6.9).****
>
> Furthermore, the proposed change will turn these types to pointers to
> opaque types, which means that it will hide the actual (vendor specific)
> implementation, so the OpenCL vendors using Clang will be able to implement
> these types in their own way.****
>
> ****
>
> This change would also be a basis for the implementation of SPIR
> generation by Clang. The SPIR discussion and spec can be found here:****
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024132.html****
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024178.html****
>
> ****
>
> Earlier discussion about the OpenCL types was started by Anton Lokhmotov:*
> ***
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-May/015297.html****
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-April/014741.html****
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-March/014118.html****
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-March/014121.html****
>
> ****
>
> ****
>
> <image001.png>****
>
> ****
>
> <opencl_types.patch>****
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited****
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.****
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev****
>
> ****
>
> ** **
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited****
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.****
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev****
>
> ** **
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
> _______________________________________________
> 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/20121009/64437e59/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.png
Type: image/png
Size: 24800 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20121009/64437e59/attachment.png>
More information about the cfe-dev
mailing list