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

Alexey Bader via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 14 02:22:54 PDT 2017


bader added inline comments.


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


================
Comment at: lib/Basic/Targets.cpp:2367
 
-  LangAS::ID getOpenCLImageAddrSpace() const override {
+  virtual LangAS::ID
+  getOpenCLTypeAddrSpace(BuiltinType::Kind K) const override {
----------------
I think the rule LLVM applies is: use virtual in base classes and override w/o virtual in derived classes.
'virtual' is not necessary here.


https://reviews.llvm.org/D33989





More information about the cfe-commits mailing list