[PATCH] D21567: [OpenCL] Generate struct type for sampler_t and function call for the initializer

Anastasia Stulova via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 23 07:10:10 PDT 2016

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.
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.


More information about the cfe-commits mailing list