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

Mariya Podchishchaeva via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 11 09:35:56 PST 2020


Fznamznon added a comment.

In D74387#1869819 <https://reviews.llvm.org/D74387#1869819>, @ABataev wrote:

> I would add a check for the use of unsupported types in kernels. They should not be allowed to be used if target does not support it.


Yeah, I think so. We tried to make it using deferred diagnostics. Unfortunately there isn't a single 'create declaration' type place that we could diagnose this. This resulted to a lot of changes around Sema and probably unhanded cases. For example we need to diagnose each appearance of `__float128` type in device code. And `__float128` can appear in device code through so many ways, for example through auto types variable declaration and initializing it with __float128 value captured from the host, like this:

   // HOST CODE
    __float128 B = 1; // No errors
  ...
  // DEVICE CODE
    kernel<class some_kernel>([=]() {
            auto C = B; }); // Problem, C will actually have __float128 type!

And for example, we can't just trigger on __float128 type appearance in some code, like the diagnosic which I'm disabling does, because I believe that some unevaluated contexts shouldn't trigger errors, because they don't bring the unsupported type to the device code:

  template<typename t> void foo(){};
  __float128 nonemittedfunc();
  
  // DEVICE CODE
  foo<__float128>(); // This shouldn't bring errors
  std::conditional_t<SomeI < 1, decltype(nonemittedfunc()), int> SomeVar; // This shouldn't bring errors

The whole patch with test cases is available here https://github.com/intel/llvm/pull/1040 .
We decided to disable this until we figure out the way how to properly diagnose this.

In D74387#1869845 <https://reviews.llvm.org/D74387#1869845>, @rjmccall wrote:

> The right approach here is probably what we do in ObjC ARC when we see types that are illegal in ARC: in system headers, we allow the code but add a special `UnavailableAttr` to the declaration so that it can't be directly used.
>
> That is straightforward enough that I think you should just do it instead of leaving this as technical debt.


I haven't considered something like this, because I'm not familiar with ObjC at all... I will give it a try, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387





More information about the cfe-commits mailing list