yaxunl added inline comments.

================
Comment at: include/clang/Basic/TargetInfo.h:1041
+    default:
+      return LangAS::Default;
+    }
----------------
Anastasia wrote:
> bader wrote:
> > 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.
> We could create a bug to Khronos OpenCL C spec and discuss it on the next 
> call just to make sure... but otherwise this change seems reasonable enough!
Can you or Alexey bring this issue to the WG? Thanks.


https://reviews.llvm.org/D33989



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to