Anastasia added a comment.
In https://reviews.llvm.org/D28136#697844, @bader wrote: > > 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. > > as_type is defined to be used for bit re-interpretation. (see 6.2.4.2 > Reinterpreting Types Using as_type() and as_typen()). In this sense, it's > exactly matches __bultin_astype built-in function. > > Here are a few relevant OpenCL C language specification quotes from 6.2.4 > section: > > > All data types described in tables 6.1 and 6.2 (except bool, half and void) > > may be also reinterpreted as another data type of **the same size** using > > the as_type() operator for scalar data types and the as_typen() operator > > for vector data types. > > > > > The usual type promotion for function arguments shall not be performed. > > > > > It is an error to use as_type() or as_typen() operator to reinterpret data > > to a type of a different number of bytes. > > So, aliasing as_type to __builtin_astype provides these checks, whereas we > can't do it for overloadable as_type function declarations. > > I also would like to address your original concerns: > > > The main issue is after preprocessing the header the original function name > > is no longer available in diagnostics reported. > > Actually diagnostics is able to provide a hint to exact code location in the > header file where as_<type> is defined as macro, so user gets correct name of > the operator in diagnostics as far as I know. > > > The spec defines as_type as a builtin function and not a macro. > > To be precise, spec defines as_type as an operator. So, the best way to > implement it would be to add a language support of such operator, but AFAIK > aliasing to __bulitin_astype via macro is sufficient enough for OpenCL use > cases. > > > Additionally your patch would allow as_type to be used with extra type (not > > only those defined in spec). > > Not sure I get this. We defined limited set of as_* functions - only for > types from tables 6.1 and 6.2 as specified by specification, so if OpenCL > developer will try to call as_<type1>(type2), which is not defined in the > header, compiler will report en error about calling undeclared function. > > > Also I don't see the problem to implement as_type with just simply calling > > a builtin. It should be inlined later anyways. > > Yes, but this solution will not give us error checking as pre-processor > solution. > > Does it make sense? From all the above arguments, I feel like the right approach would be to implement it as Clang builtin which closely matches the operator semantic in my opinion. We could of course reuse the implementation of __bultin_astype to avoid unnecessary extra work and code duplication. Using the macro seems to me more like a workaround solution and overloaded functions don't seem to be entirely the right thing either. What do you think about it? https://reviews.llvm.org/D28136 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
