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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
