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

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: include/clang/AST/BuiltinTypes.def:164
@@ +163,3 @@
+// Internal OpenCL sampler initializer type.
+BUILTIN_TYPE(OCLSamplerInit, OCLSamplerInitTy)
+
----------------
> However, we want it to be translated to __sampler_initializer* type.

This is precisely what I am not able to understand. Why do we want it to be  __sampler_initializer* type? I don't think we want that. If it was a sampler type originally in OpenCL, it should be kept a sampler type in LLVM too.

================
Comment at: include/clang/AST/OperationKinds.def:328
@@ +327,3 @@
+// Convert an integer initializer to an OpenCL sampler initializer.
+CAST_OPERATION(IntToOCLSamplerInitializer)
+
----------------
> In codegen, we want to insert a call of __transform_sampler_initializer for the reference of variable a, not the definition of variable a.
 I think the original discussion was to insert the call on the declaration of sampler variable as an initializer. This would allow us to use variable itself "as is" everywhere else.

So if let's say we compile:
  constant sampler_t a = 0;
  kernel void f() {
     g(a);
  }

the code we are generating would be equivalent to:

  kernel void f() {
    __sampler* a = __transform_sampler_initializer(0);
    g(a);
  }

Why are we not using this approach?

I think we discussed later that we could generate a struct of initializers instead of just int (0 -> struct sampler_initializer). Here is the description: http://lists.llvm.org/pipermail/cfe-dev/2016-June/049389.html
But general principle still was as described above...

================
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 see. This is just because OpenCL doesn't allocate actual constants for different allowed sampler modes whereas SPIR does.

================
Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:52
@@ +51,3 @@
+                           Ctx, "__sampler"),
+                           CGM.getContext().getTargetAddressSpace(
+                           LangAS::opencl_constant));
----------------
If you do conversion struct ptr <-> opaque ptr in this function, it should be fine? I would imagine images and other OpenCL types have the same requirement.


http://reviews.llvm.org/D21567