Anastasia added a comment. Btw, I am missing tests for generated __translate_sampler_initializer which I think we had at some point.
================ Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:84 @@ +83,3 @@ + SamplerTy = llvm::PointerType::get(llvm::StructType::create( + CGM.getLLVMContext(), "__opencl_sampler_t"), + CGM.getContext().getTargetAddressSpace( ---------------- Related to this thread: http://lists.llvm.org/pipermail/cfe-dev/2016-July/050126.html. I am guessing leaving the old naming scheme should be fine. ================ Comment at: lib/Sema/SemaInit.cpp:6907 @@ -6905,2 +6906,3 @@ case SK_OCLSamplerInit: { - assert(Step->Type->isSamplerT() && + // Sampler initialzation have 6 cases: + // 1. function argument passing ---------------- There are 5 cases it seems :) ================ Comment at: lib/Sema/SemaInit.cpp:6947 @@ +6946,3 @@ + // Do not diagnose if the file-scope variable does not have initializer + // since this has already beend diagnosed when parsing the variable + // declaration. ---------------- beend -> been ================ Comment at: lib/Sema/SemaInit.cpp:6949 @@ +6948,3 @@ + // declaration. + if (!Var->getInit() || !isa<ImplicitCastExpr>(Var->getInit())) + break; ---------------- Do we accept declaring global sampler_t variable without an initializer? ================ Comment at: lib/Sema/SemaInit.cpp:6956 @@ +6955,3 @@ + } + } + ---------------- Could you use else here instead of adding TakeLiteral variable to avoid going to the next compound statement? ================ Comment at: lib/Sema/SemaInit.cpp:6965 @@ +6964,3 @@ + // the initializer. + if (!Init->isConstantInitializer(S.Context, false)) + break; ---------------- Should this generate an error? ================ Comment at: test/SemaOpenCL/sampler_t.cl:51 @@ -14,1 +50,3 @@ foo(glb_smp); + foo(glb_smp2); + foo(glb_smp3); ---------------- Are these extra calls below adding any extra testing? ================ Comment at: test/SemaOpenCL/sampler_t.cl:83 @@ -28,2 +82,3 @@ *smp2; //expected-error{{invalid argument type 'sampler_t' to unary expression}} + foo(smp1+1); //expected-error{{invalid operands to binary expression ('sampler_t' and 'int')}} } ---------------- Does this line add extra testing? https://reviews.llvm.org/D21567 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
