[PATCH] D74387: [SYCL] Do not diagnose use of __float128

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

@rjmccall, Thank you very much for so detailed response, It really helps. I started working on implementation and I have a couple of questions/problems with this particular appoach.

> - If `foo` uses `__float128` (whether in its signature or internally), that is invalid in device mode, but the diagnostic will be delayed by the forbidden-type mechanism, meaning that it will become an `unavailable` attribute on `foo`.

So, for example if some variable is declared with `__float128` type, we are adding to parent function Unavaliable attribute, right?

> - If `bar` uses `foo`, that use is invalid in device mode (because of the `unavailable` attribute), but the diagnostic will be delayed via the standard CUDA/OMP mechanism because we don't know yet whether `bar` should be emitted as a device function.
> - If `kernel` uses `bar`, that will trigger the emission of the delayed diagnostics of `bar`, including the use-of-`unavailable`-function diagnostic where it uses `foo`.  It should be straightforward to specialize this diagnostic so that it reports the error by actually diagnosing the use of `__float128` at the original location (which is recorded in the `unavailable` attribute) and then just adding a note about how `foo` is used by `bar`.

Consider following example (this is absolutely valid SYCL code, except __float128 usage):

  // Host code:
  __float128 A;
  // Everything what lambda passed to `single_task` calls becomes device code. Capturing of host variables means that these variables will be passed to device by value, so using of A in this lambda is invalid.
  sycl_kernel<class kernel_name>([=]() {auto B = A});

In this case we add unavailable attribute to parent function for variable `A` declaration. But this function is not called from device code. Please correct me if I'm wrong but it seems that we need to diagnose not only functions, but also  usages of any declarations with unavailable attribute including variable declarations, right?

In addition, there are a couple of problems with this approach, for example with unevaluated `sizeof` context, i.e. code like this:

  sycl_kernel<class kernel_name>([=]() {int A = sizeof(__float128);});

is diagnosed too, which IMO shouldn't be diagnosed.

I can upload what I have now to this review if it will help better (or maybe we will understand that I'm doing something wrong).

I'm also thinking about a bit another approach:

- If some declaration uses `__float128` it will become an unavailable attribute on this declaration. That means we don't always add unavailable attribute to the function which uses `__float128` internally.
- In the place where clang actually emits `use-of-unavailable-declaration` diagnostics (somewhere in `DoEmitAvailabilityWarning` function, defined in `SemaAvailability.cpp`) for SYCL, we make these diagnostics deferred using CUDA/OMP deferred diagnostics mechanism (using SYCL-specific analog of function like `diagIfOpenMPDeviceCode`/`CUDADiagIfDeviceCode`).

But this won't emit diagnostics for simple declarations which has __float128 type, but is not used anywhere in code.

I'm also curious about OpenMP handling of this "unsupported type" problem. @ABataev , Am I right that in OpenMP such diagnostics are emitted only if forbidden type is used in some arithmetical operations? Is it enough to prevent problems on various GPU devices which don't support this type?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387