<div dir="ltr"><div>The pointer type is only during conversion to llvm intermediate. During all clang stages, they are still fully independent, non-pointer types. Alas, I can't defend the "defined as pointer" argument since it was not my choice - it was already there. I'm <span style="line-height:1.5;font-size:13.1999998092651px">indifferent</span><span style="line-height:1.5;font-size:13.1999998092651px"> to how they are represented when lowering. I think the important thing here is that Clang enforces the proper language semantics.</span><br></div><div><br></div>Regarding the data-layout, I think that'll open the doors for every language out there to demand equal rights to add stuff onto the data-layout. I don't think the llvm community would want that, but that's just my opinion. That change would also be way too invasive for what I wanted to do with this patch. If indeed that is the direction to go, then someone greater than I will have to pick that up.<br></div><br><div class="gmail_quote"><div dir="ltr">On Thu, 11 Jun 2015 at 10:01 Sam Parker <<a href="mailto:sam.parker@arm.com">sam.parker@arm.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Pedro,<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I had noticed that event_t is also handled this way, but it still seems odd to me to declare something as a pointer type when it is not a pointer (sorry if this is what is blinding me). But as OpenCL is gaining popularity, and clang being a popular driver, I wonder if it would be a good idea to encode the size and alignment of OpenCL types in the target  datalayout?<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers,<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">sam<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p><p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Pedro Ferreira [mailto:<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>] <br><b>Sent:</b> 11 June 2015 09:01<br><b>To:</b> Sam Parker; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br><b>Subject:</b> Re: [PATCH] OpenCL: Add new types for OpenCL 2.0<u></u><u></u></span></p></div></div><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal" style="margin-bottom:12.0pt">Hi Sam,<u></u><u></u></p><div><p class="MsoNormal">I don't understand what you're trying to say.<u></u><u></u></p></div><div><p class="MsoNormal">You quoted the standard which says that the "size of those types is implementation defined"; you then infer that this is the reason why it can /not/ make it a pointer to opaque struct. I would think the other way around: it's because the size is implementation defined that we can use whatever representation we wish.<u></u><u></u></p></div><div><p class="MsoNormal">Further to that, this is how event_t is already defined (I didn't add it).<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">OpenCL implementations will likely either patch clang to do this or add a typedef in the implicit header which will end up generating the same llvm intermediate anyway.<u></u><u></u></p></div><div><p class="MsoNormal">What these structs map to in final HW code is indeed target defined, so I suppose your comment on "hooks to get the specific types" would be accurate.<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">In any case, if the representation of those types is not acceptable for Clang, I'll strip them and leave only the images in.<u></u><u></u></p></div><div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">Pedro<u></u><u></u></p></div></div><p class="MsoNormal"><u></u> <u></u></p><div><div><p class="MsoNormal">On Thu, 11 Jun 2015 at 08:41 Sam Parker <<a href="mailto:sam.parker@arm.com" target="_blank">sam.parker@arm.com</a>> wrote:<u></u><u></u></p></div><blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm"><div><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Pedro,</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I was looking at these types a couple of months back, what stumped me is that ndrange_t is not a pointer type as I had originally tried. You too have made it a pointer-to-a-struct type but I feel this is wrong. The spec says, ‘values returned by applying the sizeof operator to the queue_t, clk_event_t, nrange_t and reserve_id_t types is implementation defined.’ So I understand that it would not be valid to make this types as pointers and that implementations would probably just have to create definitions for them in the distributed header file; unless there were some hooks to the backend to get the specific implementation defined types.</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Cheers,</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">sam</span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p><p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> <a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a>] <b>On Behalf Of </b>Pedro Ferreira<br><b>Sent:</b> 11 June 2015 08:19<br><b>To:</b> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br><b>Subject:</b> [PATCH] OpenCL: Add new types for OpenCL 2.0</span><u></u><u></u></p></div></div><div><div><p class="MsoNormal"> <u></u><u></u></p><div><p class="MsoNormal">Hi all, <br><br>This patch adds the new OpenCL types for 2.0 described at <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__www.khronos.org_registry_cl_sdk_2.0_docs_man_xhtml_otherDataTypes.html&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=42YnWExwxwpeU6GPDY2_3RFxCqQakUbj_CXZsMsQ2jU&s=REOBNoaDio7qDyIDCqmXhxFvZYjMOK6vuXAttjOVsNI&e=" target="_blank">https://www.khronos.org/registry/cl/sdk/2.0/docs/man/xhtml/otherDataTypes.html</a> <u></u><u></u></p><div><p class="MsoNormal">I also opened <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_show-5Fbug.cgi-3Fid-3D23794&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=42YnWExwxwpeU6GPDY2_3RFxCqQakUbj_CXZsMsQ2jU&s=TAV4suAMaHgdIPA83Da3pQl7c68On7bAFWtnrUbt_Uk&e=" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=23794</a> for this. I keep forgetting you prefer patches sent to this mailing list.<br>This also adds lldb entries (fixes switch warnings). <br><br>The types are: <br><br>image2d_depth_t <br>image2d_array_depth_t <br>image2d_msaa_t <br>image2d_array_msaa_t <br>image2d_msaa_depth_t <br>image2d_array_msaa_depth_t <br>queue_t <br>ndrange_t <br>clk_event_t <br>reserve_id_t <br><br>let me know if something looks wrong,<u></u><u></u></p></div><div><p class="MsoNormal">Pedro<u></u><u></u></p></div></div></div></div></blockquote></div></div></div></div></blockquote></div>