[SYCL] Add property-based device_has diagnostics #7297
Conversation
The device_has property should have behavior similar to that of the sycl::device_has attribute. This commit makes the aspect propagation pass issue the same diagnostic as done for when an application uses aspects that are not in sycl::device_has, albeit with a small change to the diagnostics message to differentiate the origin of the device_has, be it property or attribute. Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
elizabethandrews
left a comment
elizabethandrews
left a comment
There was a problem hiding this comment.
FE changes LGTM
smanna12
left a comment
smanna12
left a comment
There was a problem hiding this comment.
FE changes look good to me.
aelovikov-intel
left a comment
aelovikov-intel
left a comment
There was a problem hiding this comment.
Some NITs inline, could be addressed post-commit.
| using namespace sycl::ext::oneapi::experimental; | ||
|
|
||
| // expected-note-re@+1 4 {{propagated from call to function '{{.*}}Struct1::Struct1({{.*}})'}} | ||
| struct [[__sycl_detail__::__uses_aspects__(aspect::fp16)]] Struct1 { |
There was a problem hiding this comment.
nit: I'd suggest renaming structs/funcs to something like StructWithFP16, use_fp16, use_cpu, etc. Not sure how much help it would be though.
There was a problem hiding this comment.
They have been renamed to something more informative. Is this along the lines of what you were thinking?
sycl/test/extensions/properties/properties_kernel_device_has_warning.cpp
Outdated
Show resolved
Hide resolved
sycl/test/extensions/properties/properties_kernel_device_has_warning.cpp
Show resolved
Hide resolved
| struct KernelFunctorWithOnlyWGSizeHintAttr { | ||
| // expected-warning@+1 {{kernel has both attribute 'work_group_size_hint' and kernel properties; conflicting properties are ignored}} | ||
| void operator() [[sycl::work_group_size_hint(32)]] () const {} | ||
| }; |
There was a problem hiding this comment.
Is that based on the place where the kernel is submitted?
There was a problem hiding this comment.
I agree, technically it would be better to do it at submission, but to my knowledge there's no good way to get back to the submission from the level where we can detect this mismatch. If I am wrong I think it's something we should investigate in a separate patch as this diagnostic isn't new to this patch but is just added for device_has and cases are added for work_group_size_hint because they were affected by a bug when the diagnostic was first introduced.
Signed-off-by: Larsen, Steffen <[email protected]>
|
Opened #8798 for timeout issue with more details. |
|
extensions/properties/properties_kernel_device_has_warning.cpp is failing in post-commit for Linux (no assert). I am investigating. |
|
Post-commit fix here: #7329 |
7 participants
The device_has property should have behavior similar to that of the sycl::device_has attribute. This commit makes the aspect propagation pass issue the same diagnostic as done for when an application uses aspects that are not in sycl::device_has, albeit with a small change to the diagnostics message to differentiate the origin of the device_has, be it property or attribute.