[SYCL] Fix build issue that appeared in self-build

[romanovvlad]

Define __SYCL_TYPE to empty unless compiling in SYCL device mode

[romanovvlad]

The same as #7811 , but for SYCL_TYPE

[bader]

The same as #7811 , but for SYCL_TYPE

@romanovvlad, could you check if there are any other attributes which require this fix, please?

[AlexeySachkov]

I wonder, can we update our attributes so __has_cpp_attribute returns true only for device compilation? Tagging @intel/dpcpp-cfe-reviewers

[bader]

I think you mean that some SYCL specific attributes can be enabled by the compiler only in SYCL device compilation mode and reported as unsupported in other modes (SYCL host or non-SYCL). Right?
I don't think we can change the semantic of __has_cpp_attribute.

[AlexeySachkov]

I think you mean that some SYCL specific attributes can be enabled by the compiler only in SYCL device compilation mode and reported as unsupported in other modes (SYCL host or non-SYCL). Right?

Yes, thanks, that's what I meant. I would like us to simply rely on __has_cpp_attibute alone, without need to check for __SYCL_DEVICE_ONLY__.

[elizabethandrews]

You can modify attribute definition to enable it only on device. You can silence the ignored attribute on host which is how we handle other device only attributes

[romanovvlad]

We will still have checks for attribute arg validity, see #7881 (comment)

[elizabethandrews]

Hmm...technically that means attribute semantics is incorrect i.e. it is invalid code. I cannot recall off the top of my head how we handle this usually. @smanna12 @AaronBallman should we diagnose invalid attribute usage i.e. invalid argument when it is ignored on device? I vaguely remember a discussion about this once but I cannot recall the resolution

[AaronBallman]

Typically, when an attribute is ignored, we warn about the "unknown" attribute and that is it. e.g., [[no_unique_address]] is a standard attribute that accepts no arguments, but we don't support it on Windows because of , and so: https://godbolt.org/z/6Yh1jWx89

[elizabethandrews]

Ok Thank you for confirming.

[elizabethandrews]

@romanovvlad if this attribute is not required on host, you can modify the attribute definition as well as silence other diagnostics by not handling the attribute on host as you have already done. I think we can then remove SYCL_DEVICE_ONLY check as @AlexeySachkov suggested.

[elizabethandrews]

I spend a bit of time looking into this. SYCL_DEVICE_ONLY check cannot be removed at the moment because __has_attribute does not consider LangOpts when generating attributes in TableGen. This should be fixed in upstream clang if possible. We can do this as a separate task IMO.

[bader]

@romanovvlad, I've fixed formatting, but I'd you to take a look at llvm-test-suite regressions.

[romanovvlad]

The same as #7811 , but for SYCL_TYPE

@romanovvlad, could you check if there are any other attributes which require this fix, please?

Do not see any other attributes that require the fix.

[romanovvlad]

@bader
The test failures happen because of our diagnostics for [[sycl::device_has(...)]], they check that the argument is a type marked by [[__sycl_detail__::sycl_type(x)]]. But since in this patch we do not mark enum class aspect with this attribute, the FE says that the type is invalid.

Those checks currently happen during both host and device compilations.

As a possible quick solution we can ignore the attribute completely for SYCL host compilation(current version of the patch).

Alternatively, I see that we have SilentlyIgnoreSYCLIsHost flag in Attr.td which is likely supposed to address this, but currently it silences <<>> attribute ignored warning only while other diagnostics take place. We could try to adjust FE to skip all diagnostics if this flag is enabled.

[elizabethandrews]

Alternatively, I see that we have SilentlyIgnoreSYCLIsHost flag in Attr.td which is likely supposed to address this, but currently it silences <<>> attribute ignored warning only while other diagnostics take place. We could try to adjust FE to skip all diagnostics if this flag is enabled.

SilentlyIgnoreSYCLIsHost was implemented to avoid diagnosing "unknown attribute" error only. Earlier, host compilation pass would throw this warning confusing users compiling their SYCL program.

If we want to avoid attribute semantic checks as well using tablegen I think we will need bigger change where SYCL attributes are handled like we handle target specific attributes maybe. I don't know how to do this off the top off my head and if its worth it.

The easy way to avoid semantic checks for attributes is what you have done in this PR - return early from handlers. IIRC we don't really do this for most SYCL device attributes. So at the moment, we are doing validity checks on host for most other device attributes as well I think.