LGTM, Thanks.

Is there any chance to get this commit cherry-picked to 10.x branch?

Is there any chance to get this commit cherry-picked to 10.x branch?

Rename color to depth

In D71460#1783390, @asavonic wrote:

What about get_image_num_mip_levels functions defined in the extension specification?

Edit: I mean, should the get_image_num_mip_levels(write_only img) function be only available if cl_khr_mipmap_image_writes extension is supported, or cl_khr_mipmap_image is enough?

LGTM! @joey, any idea when it will be landed?

The patch has been updated. Now it checks if the argument isNullPointerConstant and emits null pointer directly in this case.

In D59985#1454676, @Anastasia wrote:

In D59985#1454660, @AlexeySotkin wrote:

In D59985#1454629, @Anastasia wrote:

In D59985#1453312, @AlexeySotkin wrote:

Alternative way to fix it is to use isNullPointerConstant like we do in SemaOpenCLBuiltinEnqueueKernel. So in case we have a zero literal value we can emit ConstantPointerNull directly, without EmitScalarExpr .

Ok and if it's not 0 the code gets rejected?

It must be rejected by SemaOpenCLBuiltinEnqueueKernel and it is already done https://godbolt.org/z/MFN3VU

Ok, cool. Perhaps this is indeed a cleaner approach then?

In D59985#1454629, @Anastasia wrote:

In D59985#1453312, @AlexeySotkin wrote:

Alternative way to fix it is to use isNullPointerConstant like we do in SemaOpenCLBuiltinEnqueueKernel. So in case we have a zero literal value we can emit ConstantPointerNull directly, without EmitScalarExpr .

Ok and if it's not 0 the code gets rejected?

Alternative way to fix it is to use isNullPointerConstant like we do in SemaOpenCLBuiltinEnqueueKernel. So in case we have a zero literal value we can emit ConstantPointerNull directly, without EmitScalarExpr .

Fix resolving of block invoke function in case of sequence of assignments.

Fix ObjC lit tests failure

In D43783#1215573, @Anastasia wrote:

In D43783#1212485, @yaxunl wrote:

In D43783#1204353, @svenvh wrote:

Sorry for digging up an old commit...

Apparently this broke block arguments, e.g. the following test case:

int foo(int (^ bl)(void)) {
  return bl();
}

int get21() {
  return foo(^{return 21;});
}

int get42() {
  return foo(^{return 42;});
}

In particular, the VarDecl that getBlockExpr() sees doesn't have an initializer when the called block comes from an argument (causing clang to crash).

Sorry for the delay. I think block should not be allowed as function argument since this generally leads indirect function calls therefore requires support of function pointer. It will rely on optimizations to get rid of indirect function calls.

The idea was to allow blocks as function parameters because they are statically known at each function call. This can be resolved later at IR level instead of frontend. I am also not sure there can be other corner cases where it wouldn't work in Clang since we can't leverage analysis passes here. I feel this might be a risky thing to do in Clang. Currently it doesn't work for the examples provided and therefore breaking the compliance with the spec.

Ping

In D49725#1183316, @aaron.ballman wrote:

FYI: @asavonic, the email address you have associated with your commit id is AlexeySotkin@/etc/mailname which is getting stuck in the moderation queue as not being signed up to the mailing list. You may want to correct your svn information as I am not certain what our list software will think of that domain.

In D46015#1078317, @stuart wrote:

In D46015#1078260, @AlexeySotkin wrote:

In D46015#1078235, @stuart wrote:

In D46015#1078217, @AlexeySotkin wrote:

There should not be need for bitcast. Could give an example ? Thanks.

If I have a write_only pipe as the argument to get_pipe_max_packets(), and this uses a single __get_pipe_num_packets() function taking a read_only pipe, we will automatically get a bitcast:

%20 = call i32 bitcast (i32 (%opencl.pipe_ro_t*, i32, i32)* @__get_pipe_max_packets to i32 (%opencl.pipe_wo_t*, i32, i32)*)(%opencl.pipe_wo_t* %19, i32 4, i32 4)

Sorry, but I don't quite understand what does get_pipe_max_packets(), uses __get_pipe_num_packets() mean. Could you clarify? Possibly OpenCL C source example could help.

I mean that without these two separate versions, the call to __get_pipe_num_packets() that is emitted can include a bitcast.

For example:

void foo(read_only pipe int r, write_only pipe int w) {
  get_pipe_num_packets(w);
  get_pipe_num_packets(r);
}

get_pipe_num_packets(w) is seen first, causing i32 @__get_pipe_num_packets(%opencl.pipe_wo_t*, i32, i32) to be implicitly declared.

When the call to __get_pipe_num_packets() is emitted, this will be with an autogenerated bitcast from the type of the implicit declaration, i.e. i32 (%opencl.pipe_wo_t*, i32, i32)* to the type in the emitted expression, i.e. i32 (%opencl.pipe_ro_t*, i32, i32)*.

Here is the relevant section of IR:

%0 = load %opencl.pipe_wo_t*, %opencl.pipe_wo_t** %w.addr, align 8
%1 = call i32 @__get_pipe_num_packets(%opencl.pipe_wo_t* %0, i32 4, i32 4)
%2 = load %opencl.pipe_ro_t*, %opencl.pipe_ro_t** %r.addr, align 8
%3 = call i32 bitcast (i32 (%opencl.pipe_wo_t*, i32, i32)* @__get_pipe_num_packets to i32 (%opencl.pipe_ro_t*, i32, i32)*)(%opencl.pipe_ro_t* %2, i32 4, i32 4)

If we swap the two uses of get_pipe_num_packets() in the example above, then the type of the implicit declaration will be i32 (%opencl.pipe_ro_t*, i32, i32)* and bitcasts will instead be automatically generated when using get_pipe_num_packets() with a write_only pipe. It seems especially unfortunate that the type of the implicit declaration varies depending on the access qualifier of the first use.

In D46015#1078235, @stuart wrote:

In D46015#1078217, @AlexeySotkin wrote:

There should not be need for bitcast. Could give an example ? Thanks.

If I have a write_only pipe as the argument to get_pipe_max_packets(), and this uses a single __get_pipe_num_packets() function taking a read_only pipe, we will automatically get a bitcast:

%20 = call i32 bitcast (i32 (%opencl.pipe_ro_t*, i32, i32)* @__get_pipe_max_packets to i32 (%opencl.pipe_wo_t*, i32, i32)*)(%opencl.pipe_wo_t* %19, i32 4, i32 4)

There should not be need for bitcast. Could give an example ? Thanks.

It is not clear why we need two versions of get_pipe_num_packets and get_pipe_max_packets builtins. There is only one instruction per builtin in the SPIR-V spec. I think splitting the IR type is enough for translation to SPIR-V purposes.

Replacing #define macros with explicit declarations

Added tests

Rebasing on tip of trank

Changing case in the variable name

Setting UnnamedAddr to Global

Now there should be both :)

Squashing commits

Adding test checking address space of array initializer