yaxunl added a comment.
Can you add missing types to test/CodeGenOpenCL/opencl_types.cl ?
================
Comment at: include/clang/Basic/TargetInfo.h:1034
+ virtual LangAS::ID getOpenCLTypeAddrSpace(BuiltinType::Kind K) const {
+ switch (K) {
+#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix)
\
----------------
I think it is a good idea to make this function a central place for all OpenCL
types.
Can you also add sampler and pipe type then update CGOpenCLRuntime::getPipeType
and CGOpenCLRuntime::getSamplerType to use this function to get address space
for sampler and pipe?
================
Comment at: include/clang/Basic/TargetInfo.h:1041
+ default:
+ return LangAS::Default;
+ }
----------------
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.
================
Comment at: lib/AST/ASTContext.cpp:1775
case BuiltinType::OCLSampler: {
- auto AS = getTargetAddressSpace(LangAS::opencl_constant);
+ AS = getTargetAddressSpace(LangAS::opencl_constant);
Width = Target->getPointerWidth(AS);
----------------
All OpenCL types may be unified as
```
AS = getOpenCLTypeAddrSpace(TK);
Width = Target->getPointerWidth(AS);
Align = Target->getPointerAlign(AS);
```
where `TK = cast<BuiltinType>(T)->getKind();`
================
Comment at: lib/Basic/Targets.cpp:2376
+ default:
+ return LangAS::Default;
+ }
----------------
should return TargetInfo::getOpenCLTypeAddrSpace
https://reviews.llvm.org/D33989
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
