Fznamznon added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:12248
+ /// SYCLDiagIfDeviceCode(Loc, diag::err_type_unsupported) << "__float128";
+ DeviceDiagBuilder SYCLDiagIfDeviceCode(SourceLocation Loc, unsigned DiagID);
+
----------------
rjmccall wrote:
> Will this collect notes associated with the diagnostic correctly?
Could you please make your question a bit more concrete?
This function is supposed to work in the same way as
`Sema::CUDADiagIfDeviceCode` and `Sema::diagIfOpenMPDeviceCode` . It emits
given diagnostic if the current context is known as "device code" and makes
this diagnostic deferred otherwise. It uses the `DeviceDiagBuilder` which was
implemented earlier. This `DeviceDiagBuilder` also tries to emit callstack
notes for the given diagnostics. Do you mean these callstack notes or something
else?
================
Comment at: clang/lib/Sema/SemaAvailability.cpp:479
+ case UnavailableAttr::IR_SYCLForbiddenType:
+ diag_available_here = diag::err_type_unsupported;
+ break;
----------------
rjmccall wrote:
> All of the other cases are setting this to a note, not an error, so I suspect
> this will read wrong.
Yes, this is not a note. For such samples:
```
int main() {
__float128 CapturedToDevice = 1;
kernel<class variables>([=]() {
decltype(CapturedToDevice) D;
});
}
```
It looks like this:
```
float128.cpp:63:14: error: 'CapturedToDevice' is unavailable
decltype(CapturedToDevice) D;
^
float128.cpp:59:14: error: '__float128' is not supported on this target ///
This emitted instead of note
__float128 CapturedToDevice = 1;
^
```
I had feeling that it should probably be a note. But there is no implemented
note for unsupported types. I think I can add a new one if it will make it
better. Should I?
================
Comment at: clang/lib/Sema/SemaAvailability.cpp:534
+ if (S.getLangOpts().SYCLIsDevice)
+ S.SYCLDiagIfDeviceCode(Loc, diag) << ReferringDecl;
+ else
----------------
rjmccall wrote:
> Are you sure you want to be applying this to all of the possible diagnostics
> here, rather than just for SYCLForbiddenType unavailable attributes?
I suppose it is reasonable if we want to reuse unavaliable attribute for other
SYCL use cases. Plus, In SYCL we don't know where is device code until we
instantiate templates, it happens late, so we have to defer any diagnostic
while compiling for device, otherwise we can point to host code where much more
is allowed.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:18030
+ if (LangOpts.SYCLIsDevice && FD->hasAttr<SYCLKernelAttr>())
+ return FunctionEmissionStatus::Emitted;
+
----------------
rjmccall wrote:
> So you want to emit it for the definition in addition to emitting it for
> specific specializations?
Somehow diagnostics are emitted only for the definitions.
Without this change diagnostics aren't emitted at all.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7771
+ return true;
+ }
+
----------------
rjmccall wrote:
> I wonder if it's reasonable to treat all forbidden types the same here or if
> we want different functions for the ARC and SYCL use cases.
I think it could be reasonable if we will have forbidden type cases for SYCL
sometime. For now, I don't see the purpose in a separate function for SYCL.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74387/new/
https://reviews.llvm.org/D74387
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
