bader added inline comments.

Comment at: include/clang/Basic/TargetInfo.h:1041
+    default:
+      return LangAS::Default;
+    }
yaxunl wrote:
> bader wrote:
> > yaxunl wrote:
> > > I think the default (including even_t, clk_event_t, queue_t, 
> > > reserved_id_t) should be global since these opaque OpenCL objects are 
> > > pointers to some shared resources. These pointers may be an automatic 
> > > variable themselves but the memory they point to should be global. Since 
> > > these objects are dynamically allocated, assuming them in private address 
> > > space implies runtime needs to maintain a private memory pool for each 
> > > work item and allocate objects from there. Considering the huge number of 
> > > work items in typical OpenCL applications, it would be very inefficient 
> > > to implement these objects in private memory pool. On the other hand, a 
> > > global memory pool seems much reasonable.
> > > 
> > > Anastasia/Alexey, any comments on this? Thanks.
> > I remember we discussed this a couple of time in the past. 
> > The address space for variables of these types is not clearly stated in the 
> > spec, so I think the right way to treat it - it's implementation defined.
> > On the other hand your reasoning on using global address space as default 
> > AS makes sense in general - so can we put additional clarification to the 
> > spec to align it with the proposed implementation?
> I think it is unnecessary to specify the implementation details in the OpenCL 
> spec.
> It is also unnecessary for SPIR-V spec since the pointee address space of 
> OpenCL opaque struct is not encoded in SPIR-V.
> We can use the commonly accepted address space definition in the TargetInfo 
> base class. If a target chooses to use different address space for specific 
> opaque objects, it can override it in its own virtual function.
> I think it is unnecessary to specify the implementation details in the OpenCL 
> spec.

Agree, but my point was about specifying the intention in the specification.
For instance, OpenCL spec says that image objects are located in global memory.
It says nothing about objects like events, queues, etc., so I assumed that if 
it says nothing an implementation is allowed to choose the memory region, but 
it might be false assumption.

cfe-commits mailing list

Reply via email to