[PATCH] D28136: [OpenCL] Implement as_type operator as alias of __builtin_astype.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D28136#697673, @echuraev wrote:

> In https://reviews.llvm.org/D28136#634356, @Anastasia wrote:
>
> > This has been discussed during the initial review for the header:
> >  http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160425/157187.html
> >
> > The main issue is after preprocessing the header the original function name is no longer available in diagnostics reported. The spec defines as_type as a builtin function and not a macro. Additionally your patch would allow as_type to be used with extra type (not only those defined in spec). Also I don't see the problem to implement as_type with just simply calling a builtin. It should be inlined later anyways.
>
>
> I think that this patch is really necessary because in some cases previous implementation doesn't give a diagnostic. Please see test case that I have added to this review. With current implementation //char// variable will be cast to //int//. You can see the following part of llvm ir:
>
>   %0 = load i8, i8* %src, align 1
>   %conv = sext i8 %0 to i32
>   %call = call i32 @_Z6as_inti(i32 %conv) #2
>   
>
> So there is a bug and we didn't get error that the size of char isn't equal to size of int. Program is compiled without any problems.
>  If as_type functions will be defined in macro then we will have the following message:
>
>   error: invalid reinterpretation: sizes of 'float' and 'char' must match
>   int dst = as_int( src );
>             ^~~~~~~~~~~~~~~
>   ././llvm/tools/clang/lib/Headers/opencl-c-common.h:6615:21: note: expanded from macro 'as_int'
>   #define as_int(x) __builtin_astype((x), int)
>                     ^                ~~~
>


Why do you think this is a bug? It seems to follow standard behavior in C to promote char to int if required. Just like if you would have a C code:

  int as_int(int i);
  void foo() {
      char src = 1;
      int dst = as_int(src);
  } 

This code would complie and the same exactly IR would be generated.


https://reviews.llvm.org/D28136