Anastasia added inline comments. ================ Comment at: include/clang/AST/BuiltinTypes.def:164 @@ +163,3 @@ +// Internal OpenCL sampler initializer type. +BUILTIN_TYPE(OCLSamplerInit, OCLSamplerInitTy) + ---------------- I can't get why is this necessary to represent initializer as a special Clang type? Could we avoid this additional complexity?
================ Comment at: include/clang/AST/OperationKinds.def:328 @@ +327,3 @@ +// Convert an integer initializer to an OpenCL sampler initializer. +CAST_OPERATION(IntToOCLSamplerInitializer) + ---------------- Could we just have only int->sampler conversion? Without having an extra type for initializer? ================ Comment at: include/clang/Basic/DiagnosticGroups.td:876 @@ +875,3 @@ +// A warning group for warnings about code that clang accepts when +// compiling OpenCL C/C++ but which is not compatible with the SPIR spec. +def SpirCompat : DiagGroup<"spir-compat">; ---------------- I don't understand the description really? Why not compatible? ================ Comment at: include/clang/Driver/CC1Options.td:690 @@ -689,1 +689,3 @@ HelpText<"OpenCL only. Allow denormals to be flushed to zero">; +def cl_sampler_type : Separate<["-"], "cl-sampler-type">, + HelpText<"OpenCL only. Specify type of sampler to emit. Valid values: \"opaque\"(default), \"i32\"">; ---------------- Any reason to have this flag and support different sampler representations in Clang? ================ Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:52 @@ +51,3 @@ + return llvm::PointerType::get(llvm::StructType::create( + Ctx, "__sampler"), + CGM.getContext().getTargetAddressSpace( ---------------- Could you please keep coherency in type naming i.e. add "opencl." prefix. http://reviews.llvm.org/D21567 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
