[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 5 inline comments as done.
yaxunl added inline comments.


================
Comment at: lib/Sema/SemaType.cpp:6810
+                                          QualType &T, TypeAttrLocation TAL) {
+  if (!State.getSema().getLangOpts().OpenCL ||
+      T.getAddressSpace() != LangAS::Default)
----------------
Anastasia wrote:
> I think this could be checked before calling the function.
will do.


================
Comment at: lib/Sema/SemaType.cpp:6863
+  unsigned ImpAddr;
+  bool IsStatic = D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static;
+  // Put OpenCL automatic variable in private address space.
----------------
Anastasia wrote:
> Do we cover `extern` too?
will add.


================
Comment at: lib/Sema/SemaType.cpp:6872
+      ImpAddr = LangAS::opencl_private;
+    else if (IsStatic)
+      ImpAddr = LangAS::opencl_global;
----------------
Anastasia wrote:
> I think we can't have this case for CL1.2 see s6.8. But I think it could happen for `extern` though.
Right. I will remove setting implicit addr space for static var for CL1.2.

For extern var, for CL2.0 I will set implicit addr space to global.

However, for CL1.2 since only constant addr space is supported for file-scope var, I can only set the implicit addr space of an extern var to be constant. However I feel this may cause more confusion than convenience, therefore I will not set implicit addr space for extern var for CL1.2. If user does not use constant addr space with extern var explicitly, they will see diagnostics that extern var must have constant addr space. This is also the current behavior before my change.


https://reviews.llvm.org/D35082