<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; "><div><div>Sorry for the delay, I was out of town.</div><div><br></div><div>+static bool TryOCLSamplerInitialization(Sema &S,</div><div>+ InitializationSequence &Sequence,</div><div>+ QualType DestType) {</div><div>+ if (!S.getLangOpts().OpenCL || !DestType->isSamplerT())</div><div>+ return false;</div><div>+</div><div>+ Sequence.AddOCLSamplerInitStep(DestType);</div><div>+}</div></div><div><br></div><div>generates a new warning since it doesn't return anything after the first check.</div><div><br></div><div>There are also a bunch of warnings generated because TST_image1d_t are not handled in switch statements. Is there other code missing from this patch? For example, CIndexUSRs.cpp, CIndex.cpp, SemaTemplateVariadic.cpp.</div><div><br></div><div>I also don't believe sampler is quite right. Running the test, I see:</div><div><div>opencl_types.cl:3:11: error: initializing 'sampler_t' with an expression of incompatible type</div><div> 'int'</div><div>sampler_t glb_smp = 7;</div><div> ^ ~</div><div>opencl_types.cl:25:12: error: initializing 'sampler_t' with an expression of incompatible type</div><div> 'int'</div><div> sampler_t smp = 5;</div></div><div><br></div><div>Also, I don't think this implementation supports function overloading (which we use all over the place for builtins). So for example:</div><div><br></div><div><div>void __OVERLOAD__ bar(image1d_t X) {</div><div>}</div><div>void __OVERLOAD__ bar(image2d_t X) {</div><div>}</div></div><div><br></div><div>this causes the compiler to crash.</div><div><br></div><div><div>Assertion failed: (OldFn->isDeclaration() && "Shouldn't replace non-declaration"), function EmitGlobalFunctionDefinition, file CodeGenModule.cpp, line 1908.</div><div>0 clang 0x00000001057e91fe PrintStackTrace(void*) + 46</div><div>1 clang 0x00000001057e97a9 SignalHandler(int) + 297</div><div>2 libsystem_c.dylib 0x00007fff8aee78ea _sigtramp + 26</div><div>3 libsystem_c.dylib 0x00007fff5c9b74e8 _sigtramp + 18446744072932359192</div><div>4 clang 0x00000001057e94cb raise + 27</div><div>5 clang 0x00000001057e9582 abort + 18</div><div>6 clang 0x00000001057e9561 __assert_rtn + 129</div><div>7 clang 0x0000000103619a98 clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::GlobalDecl) + 424</div><div>8 clang 0x0000000103616da2 clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl) + 498</div><div>9 clang 0x000000010361901e clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) + 782</div><div>10 clang 0x000000010361f98d clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) + 269</div><div>11 clang 0x0000000103664684 (anonymous namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) + 100</div><div>12 clang 0x0000000103607d75 clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) + 181</div><div>13 clang 0x000000010367852b clang::ParseAST(clang::Sema&, bool, bool) + 507</div><div>14 clang 0x000000010330f7f8 clang::ASTFrontendAction::ExecuteAction() + 312</div><div>15 clang 0x0000000103607252 clang::CodeGenAction::ExecuteAction() + 1266</div><div>16 clang 0x000000010330f40b clang::FrontendAction::Execute() + 235</div><div>17 clang 0x00000001032d766f clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 863</div><div>18 clang 0x000000010325d4f7 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 999</div><div>19 clang 0x0000000103245d66 cc1_main(char const**, char const**, char const*, void*) + 1094</div><div>20 clang 0x0000000103256c89 main + 473</div><div>21 libdyld.dylib 0x00007fff95e387e1 start + 0</div><div>22 libdyld.dylib 0x0000000000000007 start + 18446603338001446950</div><div>Stack dump:</div><div>0.<span class="Apple-tab-span" style="white-space:pre"> </span>Program arguments: ../../Debug+Asserts/bin/clang -cc1 -emit-llvm -O0 -o - x.cl </div><div>1.<span class="Apple-tab-span" style="white-space:pre"> </span><eof> parser at end of file</div><div>2.<span class="Apple-tab-span" style="white-space:pre"> </span>x.cl:9:20: LLVM IR generation of declaration 'read_imagef'</div><div>3.<span class="Apple-tab-span" style="white-space:pre"> </span>x.cl:9:20: Generating code for declaration 'read_imagef'</div><div>Illegal instruction</div></div><div><br></div><div>I'm also not clear on if sampler's should be mangled too. I had done this in my implementation of sampler.</div><div><br></div><div>How much testing have you done with this patch? Is it just on the test case provided? Do you have a reference implementation in Clang of OpenCL that passes conformance?</div><div><br></div><div>-Tanya</div><div><br></div><div><br></div><div><br></div><br><div><div>On Oct 24, 2012, at 4:36 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); ">Hi all,<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); ">Please review this patch – I think I’ve fixed every controversial part of the original patch. The sampler type is now represented as i32, but it becomes a clang builtin type to enable efficient restriction checking. Also fixed every remark I’ve got for the different versions of the patch.<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></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><image001.png></span><o:p></o:p></span></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></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" style="color: purple; text-decoration: underline; ">cfe-dev-bounces@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>[mailto:cfe-<a href="mailto:dev-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline; ">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>Sunday, October 21, 2012 16:00<br><b>To:</b><span class="Apple-converted-space"> </span>Tanya Lattner<br><b>Cc:</b><span class="Apple-converted-space"> </span>Richard Smith;<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>;<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><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); ">Tanya,<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); ">I’ve fixed the width/alignment and the debug info remarks, and also removed the sampler initialization function for now.<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); ">The main idea in passing through the OpenCLRuntime for initializing samplers is enabling the initialization of non-integer vendor specific samplers later on. It’s not required for the current implementation, but it will be needed later on to support other representations of the sampler type, like in SPIR.<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); ">I prefer to leave sampler_t implemented with an i32 representation rather than removing it – the built-in types are needed for clean restriction checking.<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); "> Guy<o:p></o:p></span></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><image001.png></span><o:p></o:p></span></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></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>Tanya Lattner [<a href="mailto:lattner@apple.com" 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>Saturday, October 20, 2012 07:45<br><b>To:</b><span class="Apple-converted-space"> </span>Benyei, Guy<br><b>Cc:</b><span class="Apple-converted-space"> </span>Richard Smith;<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;<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><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; ">Guy,<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; "><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::OCLSampler:<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::OCLEvent:<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::OCLImage1d:<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::OCLImage1dArray:<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::OCLImage1dBuffer:<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::OCLImage2d:<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::OCLImage2dArray:<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::OCLImage3d:<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ // Currently these types are pointers 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; ">+ Width = Target->getPointerWidth(0);<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);<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 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></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">Is wrong if sampler is i32 I think.<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; ">Also, probably wrong:<o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ case BuiltinType::OCLSampler:<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ return getOrCreateStructPtrType("opencl_sampler_t", OCLSamplerDITy);<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; ">Why do this?<o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ } else if (type->isSamplerT()) {<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ CGM.getOpenCLRuntime().initializeOpenCLSampler(this, lvalue.getAddress(), EmitScalarExpr(init));<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; ">I also don't understand why you need to go through the OpenCLRuntime for this.<o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ if (D->getType()->isSamplerT()) {<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ getOpenCLRuntime().initializeOpenCLSampler(NULL, GV, Init);<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ } else {<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; ">+ GV->setInitializer(Init);<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; ">I would rather just see sampler ripped out for now. I have no issues with the images/event code.<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><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></div><div><p style="margin-right: 0in; margin-left: 0in; font-size: 12pt; font-family: 'Times New Roman', serif; "><span style="color: rgb(31, 73, 125); ">…</span><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><span><opencl_types6.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></body></html>