Page MenuHomePhabricator

[OpenCL] Added addrspace_cast operator
Needs ReviewPublic

Authored by Anastasia on Apr 3 2019, 4:44 AM.



This change adds extra cast operator addrspace_cast described earlier in this RFC:

This operator is intended for casting between pointers to objects in different address spaces and follows similar logic as const_cast in C++.


int* gen = ...;
global int* gl = addrspace_cast<global int*>(gen);

Note that functionality of casting pointers to different address spaces has been previously removed from all other cast operators and can only be currently done using C style casts.

This commit only enables this change for OpenCL, but if agreed it can be changed to apply to C++ too (it needs to be enabled as a valid keyword). However, for C++ it might need to be added as a Clang only extension?

Diff Detail

Event Timeline

Anastasia created this revision.Apr 3 2019, 4:44 AM
Anastasia marked an inline comment as done.Apr 3 2019, 5:04 AM
Anastasia added inline comments.

If I mark it as CXX keyword this will make it available in C++ too because the rest of the patch is not specific to OpenCL but perhaps we also need to emit a warning that this is an extension?

Ping! I would appreciate any feedback to this change please. Thanks you!

When compiling this I get the following warning:

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:1213:10: warning: enumeration value 'CXXAddrspaceCastExprClass' not handled in switch

The fix is probably to just add the missing case with the other *CastExprClass cases.

There's also a similar warning for clang/tools/libclang/CXCursor.cpp:132.


Maybe the documentation should be updated to include the new cast operator?


The first comma should be removed.


This chunk doesn't apply cleanly to master anymore it seems. This line was also modified.


I would keep this empty line.


The documentation above is really C++ specific. Maybe we should add a comment saying that this cast is not from C++ 5.2p1 to avoid any confusion?


Shouldn't this line (and the similar one in the header file) be updated to include address space cast?


Still learning here, so could you/someone tell me if I understood things correctly? :)

The type is "dependent" when templates are involved, right? And, here, when we don't know all the concrete types we defer the checking to a later compiler phase? And that later compiler phase would be implemented by "TreeTransform", right?


The formatting looks a bit funny here.


Just to make sure I understand things correctly: OpenCL is true when dealing with C or C++ mode for OpenCL, right?


Wouldn't this limit usage of the cast unnecessarily? I'm thinking this could be transformed to a NOP, which could be beneficial to make (template) code simpler to write.