<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><base href="x-msg://3051/"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Given there is some discussion on sampler still, I suggest you remove it from your patch so the rest can get a final review and hopefully get into the tree.<div><br></div><div>-Tanya</div><div><br><div><div>On Oct 9, 2012, at 1:49 PM, "Benyei, Guy" <<a href="mailto:guy.benyei@intel.com">guy.benyei@intel.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div lang="EN-US" link="blue" vlink="purple" style="font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div class="WordSection1" style="page: WordSection1; "><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">I’ve refactored the code to remove the repeated parts, and also fixed Anton Lokhmotov’s comments.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Please review.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Thanks<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "><span><image001.png></span><o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span></div><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(181, 196, 223); padding: 3pt 0in 0in; "><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; ">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; "><span class="Apple-converted-space"> </span><a href="mailto:metafoo@gmail.com" style="color: purple; text-decoration: underline; ">metafoo@gmail.com</a><span class="Apple-converted-space"> </span>[mailto:metafoo@<a href="http://gmail.com" style="color: purple; text-decoration: underline; ">gmail.com</a>]<span class="Apple-converted-space"> </span><b>On Behalf Of<span class="Apple-converted-space"> </span></b>Richard Smith<br><b>Sent:</b><span class="Apple-converted-space"> </span>Tuesday, October 09, 2012 21:26<br><b>To:</b><span class="Apple-converted-space"> </span>Benyei, Guy<br><b>Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:cfe-dev@cs.uiuc.edu" style="color: purple; text-decoration: underline; ">cfe-dev@cs.uiuc.edu</a>; Villmow, Micah; Tanya Lattner;<span class="Apple-converted-space"> </span><a href="mailto:Anton.Lokhmotov@arm.com" style="color: purple; text-decoration: underline; ">Anton.Lokhmotov@arm.com</a><br><b>Subject:</b><span class="Apple-converted-space"> </span>Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch<o:p></o:p></span></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">CGDebugInfo.cpp: Can you factor out the repeated code into a helper function?<o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><o:p> </o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">On Tue, Oct 9, 2012 at 4:36 AM, Benyei, Guy <<a href="mailto:guy.benyei@intel.com" target="_blank" style="color: purple; text-decoration: underline; ">guy.benyei@intel.com</a>> wrote:<o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">And now with unified line-endings…</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "><image001.png></span><o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(181, 196, 223); padding: 3pt 0in 0in; "><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; ">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; "><span class="Apple-converted-space"> </span><a href="mailto:cfe-dev-bounces@cs.uiuc.edu" target="_blank" style="color: purple; text-decoration: underline; ">cfe-dev-bounces@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>[mailto:<a href="mailto:cfe-dev-bounces@cs.uiuc.edu" target="_blank" style="color: purple; text-decoration: underline; ">cfe-dev-bounces@cs.uiuc.edu</a>]<span class="Apple-converted-space"> </span><b>On Behalf Of<span class="Apple-converted-space"> </span></b>Benyei, Guy<br><b>Sent:</b><span class="Apple-converted-space"> </span>Tuesday, October 09, 2012 13:26<br><b>To:</b><span class="Apple-converted-space"> </span>Richard Smith<br><b>Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank" style="color: purple; text-decoration: underline; ">cfe-dev@cs.uiuc.edu</a>;<span class="Apple-converted-space"> </span><a href="mailto:Anton.Lokhmotov@arm.com" target="_blank" style="color: purple; text-decoration: underline; ">Anton.Lokhmotov@arm.com</a></span><o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><br><b>Subject:</b><span class="Apple-converted-space"> </span>Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch<o:p></o:p></div></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Richard,</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Thanks for your very helpful review.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Attached an updated patch – please review.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Thanks</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "><image001.png></span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(181, 196, 223); padding: 3pt 0in 0in; "><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; ">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; "><span class="Apple-converted-space"> </span><a href="mailto:metafoo@gmail.com" target="_blank" style="color: purple; text-decoration: underline; ">metafoo@gmail.com</a><span class="Apple-converted-space"> </span>[<a href="mailto:metafoo@gmail.com" target="_blank" style="color: purple; text-decoration: underline; ">mailto:metafoo@gmail.com</a>]<span class="Apple-converted-space"> </span><b>On Behalf Of<span class="Apple-converted-space"> </span></b>Richard Smith<br><b>Sent:</b><span class="Apple-converted-space"> </span>Sunday, October 07, 2012 20:35<br><b>To:</b><span class="Apple-converted-space"> </span>Benyei, Guy<br><b>Cc:</b><span class="Apple-converted-space"> </span>Villmow, Micah; Tanya Lattner;<span class="Apple-converted-space"> </span><a href="mailto:Anton.Lokhmotov@arm.com" target="_blank" style="color: purple; text-decoration: underline; ">Anton.Lokhmotov@arm.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank" style="color: purple; text-decoration: underline; ">cfe-dev@cs.uiuc.edu</a><br><b>Subject:</b><span class="Apple-converted-space"> </span>Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch</span><o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+BUILTIN_TYPE(Image1d_T, Image1dTTy)<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+BUILTIN_TYPE(Image1dArray_T, Image1dArrayTTy)<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+BUILTIN_TYPE(Image1dBuffer_T, Image1dBufferTTy)<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+BUILTIN_TYPE(Image2d_T, Image2dTTy)<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+BUILTIN_TYPE(Image2dArray_T, Image2dArrayTTy)<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+BUILTIN_TYPE(Image3d_T, Image3dTTy)<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">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?<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ bool isImage1dT() const; // OpenCL image1d_t<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ bool isImage1dArrayT() const; // OpenCL image1d_t<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ bool isImage1dBufferT() const; // OpenCL image1d_t<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ bool isImage2dT() const; // OpenCL image2d_t<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ bool isImage2dArrayT() const; // OpenCL image2d_t<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ bool isImage3dT() const; // OpenCL image3d_t<o:p></o:p></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">Some of these comments are wrong.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+inline bool Type::isImage1dT() const {<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ if (const BuiltinType *BT = dyn_cast<BuiltinType>(CanonicalType))<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ return BT->getKind() == BuiltinType::Image1d_T;<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ return false;<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+}<o:p></o:p></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">Please write this as "return isSpecificBuiltinType(BuiltinType::Image1d_T);"<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ Width = Target->getPointerWidth(0); // Currently these types are pointers<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ Align = Target->getPointerAlign(0); // to opaque types<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">Please put the comment by itself on the line before this (and add a full stop).<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ case BuiltinType::Event_T: Encoding = llvm::dwarf::DW_ATE_unsigned;<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">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?<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ llvm::Type *ResultType;<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ switch (cast<BuiltinType>(T)->getKind()) {<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ default: break;<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ case BuiltinType::Image1d_T:<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ ResultType = llvm::PointerType::get(llvm::StructType::create(<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ CGM.getLLVMContext(),"opencl.image1d_t"), 0);<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ break;<o:p></o:p></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">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.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ friend class CodeGenTypes;<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">[...]<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ ResultType = CGM.OpenCLRuntime->convertOpenCLSpecificType(Ty);<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">Please remove the 'friend' declaration and instead use CGM.getOpenCLRuntime() here.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">Please also add tests for the PCH serialization/deserialization of these types.<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">On Sun, Oct 7, 2012 at 2:07 AM, Benyei, Guy <<a href="mailto:guy.benyei@intel.com" target="_blank" style="color: purple; text-decoration: underline; ">guy.benyei@intel.com</a>> wrote:<o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Anton,</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">I’ve fixed my patch according to your comments. I’ve also did some changes. See the patch attached.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Tanya,</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">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.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">The backend should be able to recognize the sampler type from the IR.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">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.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">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.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Please review this new patch</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); ">Thanks</span><o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125); "> </span><o:p></o:p></div><div><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(181, 196, 223); padding: 3pt 0in 0in; "><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; ">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; "><span class="Apple-converted-space"> </span>Villmow, Micah [mailto:<a href="mailto:Micah.Villmow@amd.com" target="_blank" style="color: purple; text-decoration: underline; ">Micah.Villmow@amd.com</a>]<span class="Apple-converted-space"> </span><br><b>Sent:</b><span class="Apple-converted-space"> </span>Thursday, October 04, 2012 22:15<br><b>To:</b><span class="Apple-converted-space"> </span>Tanya Lattner; Benyei, Guy<br><b>Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank" style="color: purple; text-decoration: underline; ">cfe-dev@cs.uiuc.edu</a>;<span class="Apple-converted-space"> </span><a href="mailto:Anton.Lokhmotov@arm.com" target="_blank" style="color: purple; text-decoration: underline; ">Anton.Lokhmotov@arm.com</a><br><b>Subject:</b><span class="Apple-converted-space"> </span>RE: [cfe-dev] OpenCL & SPIR specific types - proposal and patch</span><o:p></o:p></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; ">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.</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "> </span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; ">Micah</span><o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "> </span><o:p></o:p></div><div style="border-style: none none none solid; border-left-width: 1.5pt; border-left-color: blue; padding: 0in; "><div><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(181, 196, 223); padding: 3pt 0in 0in; "><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; ">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif; "><span class="Apple-converted-space"> </span>Tanya Lattner [<a href="mailto:lattner@apple.com" target="_blank" style="color: purple; text-decoration: underline; ">mailto:lattner@apple.com</a>]<span class="Apple-converted-space"> </span><br><b>Sent:</b><span class="Apple-converted-space"> </span>Thursday, October 04, 2012 11:49 AM<br><b>To:</b><span class="Apple-converted-space"> </span>Benyei, Guy<br><b>Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank" style="color: purple; text-decoration: underline; ">cfe-dev@cs.uiuc.edu</a>; Villmow, Micah;<span class="Apple-converted-space"> </span><a href="mailto:Anton.Lokhmotov@arm.com" target="_blank" style="color: purple; text-decoration: underline; ">Anton.Lokhmotov@arm.com</a><br><b>Subject:</b><span class="Apple-converted-space"> </span>Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch</span><o:p></o:p></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">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? <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">-Tanya<o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">On Oct 3, 2012, at 8:06 AM, "Benyei, Guy" <<a href="mailto:guy.benyei@intel.com" target="_blank" style="color: purple; text-decoration: underline; ">guy.benyei@intel.com</a>> wrote:<o:p></o:p></div></div><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></p><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; ">I’d like to renew the discussion about making the OpenCL specific types first class citizens in Clang.</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; ">I think this change is required by the OpenCL specifications, since these type names are keywords of the OpenCL C language.</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; ">This change is also needed in order to enable efficient checking of OpenCL restrictions on these types (OpenCL 1.2 spec, section 6.9).</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; ">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.</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; ">This change would also be a basis for the implementation of SPIR generation by Clang. The SPIR discussion and spec can be found here:</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "><a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024132.html" target="_blank" style="color: purple; text-decoration: underline; "><span style="color: purple; ">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024132.html</span></a></span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "><a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024178.html" target="_blank" style="color: purple; text-decoration: underline; "><span style="color: purple; ">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-September/024178.html</span></a></span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; ">Earlier discussion about the OpenCL types was started by Anton Lokhmotov:</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "><a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-May/015297.html" target="_blank" style="color: purple; text-decoration: underline; "><span style="color: purple; ">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-May/015297.html</span></a></span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "><a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-April/014741.html" target="_blank" style="color: purple; text-decoration: underline; "><span style="color: purple; ">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-April/014741.html</span></a></span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "><a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-March/014118.html" target="_blank" style="color: purple; text-decoration: underline; "><span style="color: purple; ">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-March/014118.html</span></a></span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "><a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-March/014121.html" target="_blank" style="color: purple; text-decoration: underline; "><span style="color: purple; ">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-March/014121.html</span></a></span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "><image001.png></span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 11pt; font-family: Calibri, sans-serif; "> </span><o:p></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 13.5pt; font-family: Helvetica, sans-serif; "><opencl_types.patch></span><o:p></o:p></div><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 13.5pt; font-family: Helvetica, sans-serif; ">---------------------------------------------------------------------<br>Intel Israel (74) Limited</span><o:p></o:p></p><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 13.5pt; font-family: Helvetica, sans-serif; ">This e-mail and any attachments may contain confidential material for<br>the sole use of the intended recipient(s). Any review or distribution<br>by others is strictly prohibited. If you are not the intended<br>recipient, please contact the sender and delete all copies.</span><o:p></o:p></p><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="font-size: 13.5pt; font-family: Helvetica, sans-serif; ">_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank" style="color: purple; text-decoration: underline; "><span style="color: purple; ">cfe-dev@cs.uiuc.edu</span></a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank" style="color: purple; text-decoration: underline; "><span style="color: purple; ">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</span></a></span><o:p></o:p></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; ">---------------------------------------------------------------------<br>Intel Israel (74) Limited<o:p></o:p></p><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; ">This e-mail and any attachments may contain confidential material for<br>the sole use of the intended recipient(s). Any review or distribution<br>by others is strictly prohibited. If you are not the intended<br>recipient, please contact the sender and delete all copies.<o:p></o:p></p><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><br>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank" style="color: purple; text-decoration: underline; ">cfe-dev@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank" style="color: purple; text-decoration: underline; ">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><o:p></o:p></p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "> <o:p></o:p></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><o:p> </o:p></div><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; ">---------------------------------------------------------------------<br>Intel Israel (74) Limited<o:p></o:p></p><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; ">This e-mail and any attachments may contain confidential material for<br>the sole use of the intended recipient(s). Any review or distribution<br>by others is strictly prohibited. If you are not the intended<br>recipient, please contact the sender and delete all copies.<o:p></o:p></p><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><o:p> </o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; "><o:p> </o:p></div></div></div><span><opencl_types4.patch></span><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; ">---------------------------------------------------------------------<br>Intel Israel (74) Limited</p><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; ">This e-mail and any attachments may contain confidential material for<br>the sole use of the intended recipient(s). Any review or distribution<br>by others is strictly prohibited. If you are not the intended<br>recipient, please contact the sender and delete all copies.</p></div></blockquote></div><br></div></body></html>