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

Egor Churaev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 10 06:40:34 PST 2017


echuraev added a comment.

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)
                    ^                ~~~


https://reviews.llvm.org/D28136





More information about the cfe-commits mailing list