[cfe-dev] OpenCL & SPIR specific types - proposal and patch

[Anton Lokhmotov]

Hi Guy,

While I do not quite agree with representing sampler variables as i32, I
think this will do for now.  I can later propose our solution based on
metadata for discussion.

Specific comments:

373-374:
+  case BuiltinType::OCLSampler:
+    return DBuilder.createBasicType("sampler_t",
"opencl_sampler_t"?

453-475:
+  case BuiltinType::OCLImage1d:
+    return llvm::PointerType::get(llvm::StructType::create(
+                           CGM.getLLVMContext(),"opencl.image1d_t"), 0);
+  case BuiltinType::OCLImage1dArray:
+    return llvm::PointerType::get(llvm::StructType::create(
+                           CGM.getLLVMContext(),"opencl.image1d_array_t"),
0);
+  case BuiltinType::OCLImage1dBuffer:
+    return llvm::PointerType::get(llvm::StructType::create(
+                           CGM.getLLVMContext(),"opencl.image1d_buffer_t"),
0);
+  case BuiltinType::OCLImage2d:
+    return llvm::PointerType::get(llvm::StructType::create(
+                           CGM.getLLVMContext(),"opencl.image2d_t"), 0);
+  case BuiltinType::OCLImage2dArray:
+    return llvm::PointerType::get(llvm::StructType::create(
+                           CGM.getLLVMContext(),"opencl.image2d_array_t"),
0);
+  case BuiltinType::OCLImage3d:
+    return llvm::PointerType::get(llvm::StructType::create(
+                           CGM.getLLVMContext(),"opencl.image3d_t"), 0);
+  case BuiltinType::OCLSampler:
+    return llvm::IntegerType::get(CGM.getLLVMContext(),32);
+  case BuiltinType::OCLEvent:
+    return llvm::PointerType::get(llvm::StructType::create(
+                           CGM.getLLVMContext(),"opencl.event_t"), 0);

Niggle: insert space after "CGM.getLLVMContext()".

Comment: I think the address spaces should be set up like this: global for
the image types, constant for the sampler type, and private for the event
type.  

For example:

  case BuiltinType::OCLImage1d:
    const unsigned AddrSpace =
Context.getTargetAddressSpace(LangAS::opencl_global);
    return llvm::PointerType::get(llvm::StructType::create(
                           CGM.getLLVMContext(), "opencl.image1d_t"),
AddrSpace);
  ...
  case BuiltinType::OCLEvent:
    const unsigned AddrSpace =
Context.getTargetAddressSpace(LangAS::opencl_private);
    return llvm::PointerType::get(llvm::StructType::create(
                           CGM.getLLVMContext(), "opencl.event_t"),
AddrSpace);

  case BuiltinType::OCLSampler:
    const unsigned AddrSpace =
Context.getTargetAddressSpace(LangAS::opencl_constant);
    return llvm::PointerType::get(llvm::StructType::create(
                           CGM.getLLVMContext(), "opencl.sampler_t"),
AddrSpace);

(OK, the latter will only make sense when the sampler type is represented as
an opaque type, so ignore it for now.)

787-800:
+      assert(Step->Type->isSamplerT() && 
+             "Sampler initialization on non sampler type.");
      assert(Step->Type->isSamplerT() && "Sampler initialization on non
sampler type.");

+      if (!isConst) {
+        S.Diag(Kind.getLocation(),
diag::err_sampler_initializer_not_constant);
+      }
+
+      if (!SourceType->isIntegerType()) {
+        S.Diag(Kind.getLocation(),
diag::err_sampler_initializer_not_integer)
+          << SourceType;
+      }

Single statements don't need braces.


+def err_sampler_initializer_not_integer : Error<
+  "non integer type %0 cannot initialize sampler_t">;

How about:
*  "sampler_t initialization requires integer">;

Other than that, LGTM.

Thanks,
Anton.