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 indifferent to how they are represented when lowering. I think the important thing here is that Clang enforces the proper language semantics.
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. On Thu, 11 Jun 2015 at 10:01 Sam Parker <sam.par...@arm.com> wrote: > Hi Pedro, > > > > 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? > > > > Cheers, > > sam > > > > > > *From:* Pedro Ferreira [mailto:arkang...@gmail.com] > *Sent:* 11 June 2015 09:01 > *To:* Sam Parker; cfe-commits@cs.uiuc.edu > *Subject:* Re: [PATCH] OpenCL: Add new types for OpenCL 2.0 > > > > Hi Sam, > > I don't understand what you're trying to say. > > 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. > > Further to that, this is how event_t is already defined (I didn't add it). > > > > 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. > > 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. > > > > In any case, if the representation of those types is not acceptable for > Clang, I'll strip them and leave only the images in. > > > > Pedro > > > > On Thu, 11 Jun 2015 at 08:41 Sam Parker <sam.par...@arm.com> wrote: > > Hi Pedro, > > > > 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. > > > > Cheers, > > sam > > > > > > *From:* cfe-commits-boun...@cs.uiuc.edu [mailto: > cfe-commits-boun...@cs.uiuc.edu] *On Behalf Of *Pedro Ferreira > *Sent:* 11 June 2015 08:19 > *To:* 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=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=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 > >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
