[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D74387#2026844 <https://reviews.llvm.org/D74387#2026844>, @Fznamznon wrote:

> I'm not sure that I've discovered all examples and problems, but I have a couple of ones. I started with adapting current implementation for OpenMP and right now I'm analyzing corresponding OpenMP test fails (i.e. `clang/test/OpenMP/nvptx_unsupported_type_messages.cpp` and `clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp`). There are a lot of differences between the old approach and new one, which I'm working on. The new diagnostic triggers more errors than the old one, so I'd like to understand in which concrete cases we shouldn't emit diagnostic. For example you mentioned that memcopy-like operations should be ok in device code.


You can reach me here or via email to discuss more. We also can do it over openmp-dev if you like :)

> Right now the current implementation of the diagnostic also emits errors for sample like this:
> 
>   struct T {
>     char a;
>     __float128 f;
>     char c;
>   };
>   
>   #pragma omp declare target
>   T a;
>   T b = a; // The diagnostic is triggered here, because implicit copy constructor uses unavailable field
>   #pragma omp end declare target
>   
> 
> 
> Should we emit errors in such case too?

Preferably, I would allow the above case, or in general trivial copies of unavailable basic types. What I would like to happen is that we memcpy the unavailable field.
I guess if we could "simply" replace the unavailable types in the device code right away with the byte array replacement, most things should fall into place. Basically,
we could provide even provide the replacements in a header that we include automatically:

`clang/lib/Headers/OpenMP/typedefs.h`:

  #ifdef __DEFINE_FLOAT128__
  typedef char __float128[__FLOAT128_SIZE__] alignas(__FLOAT128_ALIGNMENT__);
  
  #undef __DEFINE_FLOAT128__
  #undef __FLOAT128_SIZE__
  #undef __FLOAT128_ALIGNMENT__
  #endif

Now copy constructors (and other "OK" uses) should work fine. If people use the member in anything that actually doesn't work on a char array, they get a (probably ugly) error.
Preferably we would intercept the diagnose messages or at least issue a note if we see __float128 used, maybe along the lines of:

  Note: The target does not support operations on `__float128` types. Values of this type are consequently represented by character arrays of appropriate size and alignment.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74387/new/

https://reviews.llvm.org/D74387