[PATCH] [OpenCL] Add string literals to the constant address space.

[Tanya Lattner]

Regarding the change to SemaType. My one concern is that the type is now "const", which I think is confusing to the user when they see the error message and also I think it emits warnings if someone does const __constant in their source. What I did was change the IsModifiable() function in lib/AST/ExprClassification.cpp to treat anything in constaddr space as ConstQualified in . What do you think about this approach instead?

Otherwise, the patch looks ok to me.

-Tanya


On May 31, 2013, at 6:35 AM, Joey Gouly <joey.gouly at arm.com> wrote:

>  I forgot to add the CodeGen parts of this patch to the review, here it is.
> 
> http://llvm-reviews.chandlerc.com/D894
> 
> CHANGE SINCE LAST DIFF
>  http://llvm-reviews.chandlerc.com/D894?vs=2205&id=2209#toc
> 
> Files:
>  lib/CodeGen/CodeGenModule.cpp
>  lib/Frontend/CompilerInvocation.cpp
>  lib/Sema/SemaExpr.cpp
>  lib/Sema/SemaType.cpp
>  test/CodeGenOpenCL/opencl_types.cl
>  test/CodeGenOpenCL/str_literals.cl
>  test/SemaOpenCL/address-spaces.cl
>  test/SemaOpenCL/str_literals.cl
> 
> Index: lib/CodeGen/CodeGenModule.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -2668,11 +2668,16 @@
>   llvm::Constant *C =
>       llvm::ConstantDataArray::getString(CGM.getLLVMContext(), str, false);
> 
> +  // OpenCL v1.1 s6.5.3: a string literal is in the constant address space.
> +  unsigned AddrSpace = 0;
> +  if (CGM.getLangOpts().OpenCL)
> +    AddrSpace = CGM.getContext().getTargetAddressSpace(LangAS::opencl_constant);
> +
>   // Create a global variable for this string
> -  llvm::GlobalVariable *GV =
> -    new llvm::GlobalVariable(CGM.getModule(), C->getType(), constant,
> -                             llvm::GlobalValue::PrivateLinkage,
> -                             C, GlobalName);
> +  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
> +      CGM.getModule(), C->getType(), constant,
> +      llvm::GlobalValue::PrivateLinkage, C, GlobalName, 0,
> +      llvm::GlobalVariable::NotThreadLocal, AddrSpace);
>   GV->setAlignment(Alignment);
>   GV->setUnnamedAddr(true);
>   return GV;
> Index: lib/Frontend/CompilerInvocation.cpp
> ===================================================================
> --- lib/Frontend/CompilerInvocation.cpp
> +++ lib/Frontend/CompilerInvocation.cpp
> @@ -1014,6 +1014,7 @@
>     Opts.LaxVectorConversions = 0;
>     Opts.DefaultFPContract = 1;
>     Opts.NativeHalfType = 1;
> +    Opts.ConstStrings = 1;
>   }
> 
>   if (LangStd == LangStandard::lang_cuda)
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp
> +++ lib/Sema/SemaExpr.cpp
> @@ -1467,6 +1467,11 @@
>   if (getLangOpts().CPlusPlus || getLangOpts().ConstStrings)
>     StrTy.addConst();
> 
> +  // OpenCL v1.1 s6.5.3: a string literal is in the constant address space.
> +  if (getLangOpts().OpenCL) {
> +    StrTy = Context.getAddrSpaceQualType(StrTy, LangAS::opencl_constant);
> +  }
> +
>   // Get an array type for the string, according to C99 6.4.5.  This includes
>   // the nul terminator character as well as the string length for pascal
>   // strings.
> Index: lib/Sema/SemaType.cpp
> ===================================================================
> --- lib/Sema/SemaType.cpp
> +++ lib/Sema/SemaType.cpp
> @@ -3829,6 +3829,10 @@
> 
>   unsigned ASIdx = static_cast<unsigned>(addrSpace.getZExtValue());
>   Type = S.Context.getAddrSpaceQualType(Type, ASIdx);
> +
> +  // Constant address space implies const qualifier
> +  if(S.getLangOpts().OpenCL && ASIdx == LangAS::opencl_constant)
> +    Type = S.Context.getConstType(Type);
> }
> 
> /// Does this type have a "direct" ownership qualifier?  That is,
> Index: test/CodeGenOpenCL/opencl_types.cl
> ===================================================================
> --- test/CodeGenOpenCL/opencl_types.cl
> +++ test/CodeGenOpenCL/opencl_types.cl
> @@ -1,7 +1,7 @@
> // RUN: %clang_cc1 %s -emit-llvm -o - -O0 | FileCheck %s
> 
> constant sampler_t glb_smp = 7;
> -// CHECK: global i32 7
> +// CHECK: constant i32 7
> 
> void fnc1(image1d_t img) {}
> // CHECK: @fnc1(%opencl.image1d_t*
> Index: test/CodeGenOpenCL/str_literals.cl
> ===================================================================
> --- /dev/null
> +++ test/CodeGenOpenCL/str_literals.cl
> @@ -0,0 +1,6 @@
> +// RUN: %clang_cc1 %s -emit-llvm -o - -ffake-address-space-map | FileCheck %s
> +
> +__constant char * __constant x = "hello world";
> +
> +// CHECK: addrspace(3) unnamed_addr constant
> +// CHECK: @x = addrspace(3) constant i8 addrspace(3)*
> Index: test/SemaOpenCL/address-spaces.cl
> ===================================================================
> --- test/SemaOpenCL/address-spaces.cl
> +++ test/SemaOpenCL/address-spaces.cl
> @@ -9,5 +9,5 @@
>   int *ip;
>   ip = gip; // expected-error {{assigning '__global int *' to 'int *' changes address space of pointer}}
>   ip = &li; // expected-error {{assigning '__local int *' to 'int *' changes address space of pointer}}
> -  ip = &ci; // expected-error {{assigning '__constant int *' to 'int *' changes address space of pointer}}
> +  ip = &ci; // expected-error {{assigning 'const __constant int *' to 'int *' changes address space of pointer}}
> }
> Index: test/SemaOpenCL/str_literals.cl
> ===================================================================
> --- /dev/null
> +++ test/SemaOpenCL/str_literals.cl
> @@ -0,0 +1,12 @@
> +// RUN: %clang_cc1 %s -verify
> +// expected-no-diagnostics
> +
> +__constant char * __constant x = "hello world";
> +
> +void foo(__constant char * a) {
> +
> +}
> +
> +void bar() {
> +  foo("hello world");
> +}
> <D894.3.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits