[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Can you add a codegen test for 1.2, 2.0?

Also, I am wondering whether this is valid for OpenCL C++. Do we need special handling for string literal?



================
Comment at: lib/AST/Expr.cpp:870
+  if (C.getLangOpts().OpenCL && Ty.getAddressSpace() == LangAS::Default)
+    Ty = C.getAddrSpaceQualType(Ty, LangAS::opencl_constant);
+
----------------
Anastasia wrote:
> Anastasia wrote:
> > bader wrote:
> > > Anastasia wrote:
> > > > bader wrote:
> > > > > As `Ty` is passed by value, shouldn't we accept only data located in constant address space?
> > > > Do you mean to assert? Currently it should be passed with `constant` AS but I thought the idea is to modify this function so we can accept `Default` AS too but then replace by `constant`.
> > > > 
> > > Yes, but according to your other comment this idea doesn't work.
> > > 
> > > > I have added the address space to the creation of StringLiteral, but unfortunately it doesn't seems like we can remove similar code in other places because **QualType created for StringLiteral is also used elsewhere and has to match (contain right address space).** I.e. here is it used further down to create PredefinedExpr.
> > > 
> > > 
> > > 
> > > 
> > Indeed for the current cases it doesn't... there might be some uncaught ones though where it can be useful. But we can add it later as well. So I am thinking to undo this?
> I quite liked the idea initially, but unfortunately it seems like it's not applicable to the current use cases.
Can you extract this as 


```
QualType ASTContext::getStringLiteralType(QualType);
```
then use it here and below?




================
Comment at: test/SemaOpenCL/predefined-expr.cl:1
+// RUN: %clang_cc1 %s -verify
+
----------------
can you also check for cl 2.0?


https://reviews.llvm.org/D46049