Thanks - I'll get to that once the Clang one is reviewed/approved. On Thu, 2 Jul 2015 at 16:18 Eric Christopher <echri...@gmail.com> wrote:
> Ah. You should send it to the lldb list. > > On Thu, Jul 2, 2015, 12:56 AM Pedro Ferreira <arkang...@gmail.com> wrote: > >> The one I had attached on the very first mail :) >> Re-attaching just in case it got lost. >> >> I was not able to test it - I meant only to add switch cases to silence >> build warnings. >> >> >> On Wed, 1 Jul 2015 at 22:57 Eric Christopher <echri...@gmail.com> wrote: >> >>> What lldb patch? >>> >>> -eric >>> >>> On Tue, Jun 30, 2015 at 3:07 AM Pedro Ferreira <arkang...@gmail.com> >>> wrote: >>> >>>> Ping. >>>> Any further comments on this one? >>>> >>>> On Wed, 24 Jun 2015 at 10:43 Pedro Ferreira <arkang...@gmail.com> >>>> wrote: >>>> >>>>> Would help to actually attach the thing. >>>>> >>>>> 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. >>>>> >>>>> (this time adding cfe commits) >>>>> >>>>> On Fri, 19 Jun 2015 at 08:23 Pedro Ferreira <arkang...@gmail.com> >>>>> wrote: >>>>> >>>>>> Attached the patch after "svn diff --diff-cmd=diff -x-U0 | >>>>>> clang-format-diff.py -i" >>>>>> >>>>>> 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. >>>>>> FWIW, the line is "bool isObjCIndependentClassType() const; // >>>>>> __attribute__((objc_independent_class))", which is 91 characters long. >>>>>> >>>>>> On Fri, 19 Jun 2015 at 04:32 Richard Smith <rich...@metafoo.co.uk> >>>>>> wrote: >>>>>> >>>>>>> On Thu, Jun 18, 2015 at 5:41 PM, Eric Christopher < >>>>>>> echri...@gmail.com> wrote: >>>>>>> >>>>>>>> Please run clang-format on your patch. You still have lines over >>>>>>>> 80-columns for example. >>>>>>>> >>>>>>> + bool isImage2dDepthT() const; // Opencl >>>>>>> image_2d_depth_t >>>>>>> + bool isImage2dArrayDepthT() const; // Opencl >>>>>>> image_2d_array_depth_t >>>>>>> + bool isImage2dMSAAT() const; // Opencl >>>>>>> image_2d_msaa_t >>>>>>> + bool isImage2dArrayMSAAT() const; // Opencl >>>>>>> image_2d_array_msaa_t >>>>>>> + bool isImage2dMSAATDepth() const; // Opencl >>>>>>> image_2d_msaa_depth_t >>>>>>> + bool isImage2dArrayMSAATDepth() const; // Opencl >>>>>>> image_2d_array_msaa_depth_t >>>>>>> >>>>>>> Here's an example of an over-80-columns line. Please also fix the >>>>>>> capitalization to "OpenCL" here to match the surrounding code. >>>>>>> >>>>>>>> Thanks. >>>>>>>> >>>>>>>> -eric >>>>>>>> >>>>>>>> On Wed, Jun 17, 2015, 7:38 AM Pedro Ferreira <arkang...@gmail.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Updated from head SVN - no conflicts. >>>>>>>>> Still runs without failures. >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wed, 17 Jun 2015 at 15:08 Pedro Ferreira <arkang...@gmail.com> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Sorry, that was my bad. I forgot to set my editor back to llvm >>>>>>>>>> indentation settings and it inserted tabs instead of spaces. >>>>>>>>>> >>>>>>>>>> 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). >>>>>>>>>> >>>>>>>>>> Grep claims the attached patch has no tabs now :) >>>>>>>>>> >>>>>>>>>> On Wed, 17 Jun 2015 at 14:47 Anastasia Stulova < >>>>>>>>>> anastasia.stul...@arm.com> wrote: >>>>>>>>>> >>>>>>>>>>> I think there are still lines that are too long (especially in >>>>>>>>>>> SemaType.cpp). Have you run clang-format on your changes? >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Otherwise, no further comments from my side. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Anastasia >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> *From:* Pedro Ferreira [mailto:arkang...@gmail.com] >>>>>>>>>>> *Sent:* 15 June 2015 09:16 >>>>>>>>>>> *To:* Eric Christopher; Anastasia Stulova; >>>>>>>>>>> cfe-commits@cs.uiuc.edu >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> *Subject:* Re: [PATCH] OpenCL: Add new types for OpenCL 2.0 >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> There were a couple lines with > 80 columns, and this new patch >>>>>>>>>>> version fixes them. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Fri, 12 Jun 2015 at 21:03 Eric Christopher < >>>>>>>>>>> echri...@gmail.com> wrote: >>>>>>>>>>> >>>>>>>>>>> Drive by review here. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> I was making sure there was debug info support for the bits, >>>>>>>>>>> thanks for adding it though I'm not seeing any tests ;) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> I'm pretty sure you have some 80-column violations and other >>>>>>>>>>> formatting things, could you clang-format your patch? >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Thanks! >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -eric >>>>>>>>>>> >>>>>>>>>>> On Fri, Jun 12, 2015 at 4:20 AM Pedro Ferreira < >>>>>>>>>>> arkang...@gmail.com> wrote: >>>>>>>>>>> >>>>>>>>>>> Awesome, thanks for the tips. >>>>>>>>>>> Updated version attached. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Pedro >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Thu, 11 Jun 2015 at 19:23 Anastasia Stulova < >>>>>>>>>>> anastasia.stul...@arm.com> wrote: >>>>>>>>>>> >>>>>>>>>>> CodeGen tests looks good! >>>>>>>>>>> >>>>>>>>>>> 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. >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> Anastasia >>>>>>>>>>> ________________________________________ >>>>>>>>>>> From: Pedro Ferreira [arkang...@gmail.com] >>>>>>>>>>> Sent: Thursday, June 11, 2015 12:50 PM >>>>>>>>>>> To: Anastasia Stulova; cfe-commits@cs.uiuc.edu >>>>>>>>>>> Subject: Re: [PATCH] OpenCL: Add new types for OpenCL 2.0 >>>>>>>>>>> >>>>>>>>>>> Ok, found out the right place to diagnose the extension and >>>>>>>>>>> added the tests. >>>>>>>>>>> I am not particularly convinced that was the best way to do it; >>>>>>>>>>> comments welcome. >>>>>>>>>>> >>>>>>>>>>> Pedro >>>>>>>>>>> >>>>>>>>>>> On Thu, 11 Jun 2015 at 11:43 Pedro Ferreira <arkang...@gmail.com >>>>>>>>>>> <mailto:arkang...@gmail.com>> wrote: >>>>>>>>>>> Actually, I spoke too soon - I found a test with -cl-std=CL2.0. >>>>>>>>>>> I missed that. >>>>>>>>>>> >>>>>>>>>>> On Thu, 11 Jun 2015 at 11:40 Pedro Ferreira <arkang...@gmail.com >>>>>>>>>>> <mailto:arkang...@gmail.com>> wrote: >>>>>>>>>>> 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. >>>>>>>>>>> >>>>>>>>>>> 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. >>>>>>>>>>> My guess is that these types are allocated on the stack, which >>>>>>>>>>> by llvm convention will always be 0. >>>>>>>>>>> >>>>>>>>>>> The new types are used by new builtins. I don't think there are >>>>>>>>>>> any other special semantics to it. >>>>>>>>>>> >>>>>>>>>>> I've added extension checks on the MSAA types, but I'm not sure >>>>>>>>>>> if this is the right place. New patch attached. >>>>>>>>>>> >>>>>>>>>>> Pedro >>>>>>>>>>> >>>>>>>>>>> On Thu, 11 Jun 2015 at 10:33 Anastasia Stulova < >>>>>>>>>>> anastasia.stul...@arm.com<mailto:anastasia.stul...@arm.com>> >>>>>>>>>>> wrote: >>>>>>>>>>> Hi Pedro, >>>>>>>>>>> >>>>>>>>>>> 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? >>>>>>>>>>> >>>>>>>>>>> Are there any operations allowed on new types? Any semantical >>>>>>>>>>> checks needed? >>>>>>>>>>> >>>>>>>>>>> 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. >>>>>>>>>>> >>>>>>>>>>> Regards, >>>>>>>>>>> Anastasia >>>>>>>>>>> ________________________________________ >>>>>>>>>>> From: cfe-commits-boun...@cs.uiuc.edu<mailto: >>>>>>>>>>> cfe-commits-boun...@cs.uiuc.edu> [ >>>>>>>>>>> cfe-commits-boun...@cs.uiuc.edu<mailto: >>>>>>>>>>> cfe-commits-boun...@cs.uiuc.edu>] On Behalf Of Pedro Ferreira [ >>>>>>>>>>> arkang...@gmail.com<mailto:arkang...@gmail.com>] >>>>>>>>>>> Sent: Thursday, June 11, 2015 8:18 AM >>>>>>>>>>> To: cfe-commits@cs.uiuc.edu<mailto:cfe-commits@cs.uiuc.edu> >>>>>>>>>>> Subject: [PATCH] OpenCL: Add new types for OpenCL 2.0 >>>>>>>>>>> >>>>>>>>>>> Hi all, >>>>>>>>>>> >>>>>>>>>>> This patch adds the new OpenCL types for 2.0 described at >>>>>>>>>>> https://www.khronos.org/registry/cl/sdk/2.0/docs/man/xhtml/otherDataTypes.html >>>>>>>>>>> <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=> >>>>>>>>>>> < >>>>>>>>>>> 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= >>>>>>>>>>> > >>>>>>>>>>> I also opened https://llvm.org/bugs/show_bug.cgi?id=23794 >>>>>>>>>>> <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=> >>>>>>>>>>> < >>>>>>>>>>> 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=> >>>>>>>>>>> for this. I keep forgetting you prefer patches sent to this mailing >>>>>>>>>>> list. >>>>>>>>>>> This also adds lldb entries (fixes switch warnings). >>>>>>>>>>> >>>>>>>>>>> The types are: >>>>>>>>>>> >>>>>>>>>>> image2d_depth_t >>>>>>>>>>> image2d_array_depth_t >>>>>>>>>>> image2d_msaa_t >>>>>>>>>>> image2d_array_msaa_t >>>>>>>>>>> image2d_msaa_depth_t >>>>>>>>>>> image2d_array_msaa_depth_t >>>>>>>>>>> queue_t >>>>>>>>>>> ndrange_t >>>>>>>>>>> clk_event_t >>>>>>>>>>> reserve_id_t >>>>>>>>>>> >>>>>>>>>>> let me know if something looks wrong, >>>>>>>>>>> Pedro >>>>>>>>>>> >>>>>>>>>>> -- 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. >>>>>>>>>>> >>>>>>>>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 >>>>>>>>>>> 9NJ, Registered in England & Wales, Company No: 2557590 >>>>>>>>>>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge >>>>>>>>>>> CB1 9NJ, Registered in England & Wales, Company No: 2548782 >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- 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. >>>>>>>>>>> >>>>>>>>>>> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 >>>>>>>>>>> 9NJ, Registered in England & Wales, Company No: 2557590 >>>>>>>>>>> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge >>>>>>>>>>> CB1 9NJ, Registered in England & Wales, Company No: 2548782 >>>>>>>>>>> >>>>>>>>>>> _______________________________________________ >>>>>>>>>>> cfe-commits mailing list >>>>>>>>>>> cfe-commits@cs.uiuc.edu >>>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>>>>>>>>> >>>>>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> cfe-commits mailing list >>>>>>>> cfe-commits@cs.uiuc.edu >>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>>>>>> >>>>>>>> _______________________________________________ >>>> cfe-commits mailing list >>>> cfe-commits@cs.uiuc.edu >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> >>>
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
