[PATCH] D33989: [OpenCL] Allow targets to select address space per type

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: include/clang/Basic/TargetInfo.h:1041
+    default:
+      return LangAS::Default;
+    }
----------------
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.


https://reviews.llvm.org/D33989