[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