<div dir="ltr"><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">Would help to actually attach the thing.</span><div style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></div><div style="font-size:13.1999998092651px;line-height:19.7999992370605px">Is the patch to lldb good? It's not really important for me - I just didn't want to add warnings to the lldb build.</div><div style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></div><div style="font-size:13.1999998092651px;line-height:19.7999992370605px">(this time adding cfe commits)</div><br><div class="gmail_quote"><div dir="ltr">On Fri, 19 Jun 2015 at 08:23 Pedro Ferreira <<a href="mailto:arkangath@gmail.com">arkangath@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Attached the patch after "svn diff --diff-cmd=diff -x-U0 | clang-format-diff.py -i" <br><br><div>On the file you mentioned (Type.h), further up there was already a line with over 80 characters - the limit was exceeded by the comment, so I figured that comments were an exemption.</div><div>FWIW, the line is "bool isObjCIndependentClassType() const; // __attribute__((objc_independent_class))", which is 91 characters long.</div></div><div dir="ltr"><br><div class="gmail_quote"><div dir="ltr">On Fri, 19 Jun 2015 at 04:32 Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jun 18, 2015 at 5:41 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><p dir="ltr">Please run clang-format on your patch. You still have lines over 80-columns for example.</p></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>+ bool isImage2dDepthT() const; // Opencl image_2d_depth_t<br></div><div><div>+ bool isImage2dArrayDepthT() const; // Opencl image_2d_array_depth_t</div><div>+ bool isImage2dMSAAT() const; // Opencl image_2d_msaa_t</div><div>+ bool isImage2dArrayMSAAT() const; // Opencl image_2d_array_msaa_t</div><div>+ bool isImage2dMSAATDepth() const; // Opencl image_2d_msaa_depth_t</div><div>+ bool isImage2dArrayMSAATDepth() const; // Opencl image_2d_array_msaa_depth_t</div></div><div><br></div><div>Here's an example of an over-80-columns line. Please also fix the capitalization to "OpenCL" here to match the surrounding code. </div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><p dir="ltr">Thanks.</p><span><font color="#888888"><p>-eric</p></font></span><div><div>
<br><div class="gmail_quote"><div dir="ltr">On Wed, Jun 17, 2015, 7:38 AM Pedro Ferreira <<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Updated from head SVN - no conflicts.<div>Still runs without failures.</div></div><div dir="ltr"><div><br><br><div class="gmail_quote"><div dir="ltr">On Wed, 17 Jun 2015 at 15:08 Pedro Ferreira <<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Sorry, that was my bad. I forgot to set my editor back to llvm indentation settings and it inserted tabs instead of spaces.<br><br><div>Line 1000 of SemaTypes is 79 characters long, which is the largest (longest) line in the patch. It is the same length of line 981 from where I copy-pasted the code (by your suggestion).</div><div><br></div><div>Grep claims the attached patch has no tabs now :)</div></div><div dir="ltr"><br><div class="gmail_quote"><div dir="ltr">On Wed, 17 Jun 2015 at 14:47 Anastasia Stulova <<a href="mailto:anastasia.stulova@arm.com" target="_blank">anastasia.stulova@arm.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">I think there are still lines that are too long (especially in SemaType.cpp). Have you run clang-format on your changes?<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Otherwise, no further comments from my side.<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Anastasia<u></u><u></u></span></p><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p><p class="MsoNormal"><b><span style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif"> Pedro Ferreira [mailto:<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>] <br><b>Sent:</b> 15 June 2015 09:16<br><b>To:</b> Eric Christopher; Anastasia Stulova; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a></span></p></div></div><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:10pt;font-family:Tahoma,sans-serif"><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:12pt">There were a couple lines with > 80 columns, and this new patch version fixes them.<u></u><u></u></p><div><p class="MsoNormal"><u></u> <u></u></p></div><p class="MsoNormal"><u></u> <u></u></p><div><div><p class="MsoNormal">On Fri, 12 Jun 2015 at 21:03 Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:<u></u><u></u></p></div><blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm"><div><p class="MsoNormal">Drive by review here.<u></u><u></u></p><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">I was making sure there was debug info support for the bits, thanks for adding it though I'm not seeing any tests ;)<u></u><u></u></p><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">I'm pretty sure you have some 80-column violations and other formatting things, could you clang-format your patch?<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">Thanks!<u></u><u></u></p></div></div></div><div><div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">-eric<u></u><u></u></p></div></div></div><div><div><p class="MsoNormal">On Fri, Jun 12, 2015 at 4:20 AM Pedro Ferreira <<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>> wrote:<u></u><u></u></p></div></div><div><blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm"><div><p class="MsoNormal">Awesome, thanks for the tips.<br>Updated version attached.<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><div><div><p class="MsoNormal"><u></u> <u></u></p><div><div><p class="MsoNormal">On Thu, 11 Jun 2015 at 19:23 Anastasia Stulova <<a href="mailto:Anastasia.Stulova@arm.com" target="_blank">Anastasia.Stulova@arm.com</a>> wrote:<u></u><u></u></p></div><blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm"><p class="MsoNormal" style="margin-bottom:12pt">CodeGen tests looks good!<br><br>Regarding the extension, could you diagnose it during the type checking instead. That way it will be cover all cases. You can look at the CL2.0 atomic type implementation in SemaType.cpp ConvertDeclSpecToType. Also please reuse the same error err_type_requires_extension instead of adding the new one. Please, add Sema test demonstating the error handling works correctly.<br><br>Thanks,<br>Anastasia<br>________________________________________<br>From: Pedro Ferreira [<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>]<br>Sent: Thursday, June 11, 2015 12:50 PM<br>To: Anastasia Stulova; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>Subject: Re: [PATCH] OpenCL: Add new types for OpenCL 2.0<br><br>Ok, found out the right place to diagnose the extension and added the tests.<br>I am not particularly convinced that was the best way to do it; comments welcome.<br><br>Pedro<br><br>On Thu, 11 Jun 2015 at 11:43 Pedro Ferreira <<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a><mailto:<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>>> wrote:<br>Actually, I spoke too soon - I found a test with -cl-std=CL2.0. I missed that.<br><br>On Thu, 11 Jun 2015 at 11:40 Pedro Ferreira <<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a><mailto:<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>>> wrote:<br>The codegen test would imply adding a -cl-std=2.0 option to Clang, which it currently does not have. This is because the types should only be recognised if the CL 2.0 standard is explicitly asked for (the default is to operate on 1.2 mode). Adding that option is a peripheral issue. I've added the types on the header test under the appropriate "#if defined" but when I tried to do the same on the .cl file, I found out that the test parser does not recognise the preprocessor macro and therefore was causing the test to (incorrectly) fail. As such, I reverted the test.<br><br>As for the AS for the other types, I copy-pasted the code from event_t. That's the reason why I'm actually using the "0". Are you suggesting I should change event_t to use something else, and by consequence the new types too? That would be a separate issue.<br>My guess is that these types are allocated on the stack, which by llvm convention will always be 0.<br><br>The new types are used by new builtins. I don't think there are any other special semantics to it.<br><br>I've added extension checks on the MSAA types, but I'm not sure if this is the right place. New patch attached.<br><br>Pedro<br><br>On Thu, 11 Jun 2015 at 10:33 Anastasia Stulova <<a href="mailto:Anastasia.Stulova@arm.com" target="_blank">Anastasia.Stulova@arm.com</a><mailto:<a href="mailto:Anastasia.Stulova@arm.com" target="_blank">Anastasia.Stulova@arm.com</a>>> wrote:<br>Hi Pedro,<br><br>Could we also add a Codegen test? Also it would be better not to use constant directly as address space as the mapping could ideally be changed. Is there any reason why you generate pointers to private AS?<br><br>Are there any operations allowed on new types? Any semantical checks needed?<br><br>If MSAA types are part of an extension and not a part of the general standard we should ideally diagnose that extension is enabled when they are being used.<br><br>Regards,<br>Anastasia<br>________________________________________<br>From: <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>> [<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>>] On Behalf Of Pedro Ferreira [<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a><mailto:<a href="mailto:arkangath@gmail.com" target="_blank">arkangath@gmail.com</a>>]<br>Sent: Thursday, June 11, 2015 8:18 AM<br>To: <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><mailto:<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>><br>Subject: [PATCH] OpenCL: Add new types for OpenCL 2.0<br><br>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=9Bdb39VF2l8sby_fS5dvbUnJSVMkbUEkua5v-UqAuGY&s=5Dqa4a6V-GRZkKn3l59ia5wJtJJzBqEjUrQlOV-8t-w&e=" target="_blank">https://www.khronos.org/registry/cl/sdk/2.0/docs/man/xhtml/otherDataTypes.html</a><<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://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=</a>><br>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=9Bdb39VF2l8sby_fS5dvbUnJSVMkbUEkua5v-UqAuGY&s=DaxUQk4vxUwKYs93ZTAVu1S6Hdg2CQ1J96KmGJWtfDg&e=" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=23794</a><<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://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=</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,<br>Pedro<br><br>-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.<br><br>ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590<br>ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782<br><br><br>-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.<br><br>ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590<br>ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782<u></u><u></u></p></blockquote></div></div></div></blockquote></div><div><blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0cm 0cm 0cm 6pt;margin-left:4.8pt;margin-right:0cm"><p class="MsoNormal">_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><u></u><u></u></p></blockquote></div></blockquote></div></div></div></div></blockquote></div></div></blockquote></div></div></div></blockquote></div></div></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div></div></div></blockquote></div></div></blockquote></div></div>