Anastasia added a comment.

I don't think it works correctly yet.

Since ndrange_t is a copyable type i.e. we should be able to allocate the space 
for it at compile time and to copy it. See spec example s6.13.17.2:

  ndrange_t ndrange = ndrange_1d(...);

This implies that compiler should:

1. Generate a proper copy of an object of ndrange_t. Currently this code:

  ndrange_t n1; ndrange_t n2; n1 =n2;

is compiled to:

  %ndrange_t = type { i32, [3 x i64], [3 x i64], [3 x i64] }
  ...
  %n1 = alloca %ndrange_t
  %n2 = alloca %ndrange_t
  %0 = load %ndrange_t, %ndrange_t* %n2
  store %ndrange_t %0, %ndrange_t* %n1

Which has a load and a store instruction of the whole struct size. This should 
ideally be handled with llvm.memcpy intrinsic like all other struct type 
objects.

2. Make sizeof(ndrange_t) return its actual size. This might be important to 
use this type correctly in the CL code.

The Spec doesn't list this type as an implementation defined behavior in s6.3.k

  The behavior of applying the sizeof operator to the bool, image2d_t, 
image3d_t, image2d_array_t, image2d_depth_t, image2d_array_depth_t, image1d_t, 
image1d_buffer_t or image1d_array_t, sampler_t, clk_event_t, queue_t and 
event_t types is implementation-defined.


================
Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:43
@@ +42,3 @@
+
+  return llvm::StructType::create(EleTypes, "ndrange_t");
+}
----------------
yaxunl wrote:
> yaxunl wrote:
> > struct name should be "struct.ndrange_t" to allow library code to access it.
> Sorry, should be "struct.__ndrange_t" to avoid conflict with builtin type 
> ndrange_t.
Is there any conflict really? I think it should be Ok to keep 
"struct.ndrange_t", since we transform it to a struct and don't declare as 
struct.


Repository:
  rL LLVM

https://reviews.llvm.org/D23086



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to