llvm icon indicating copy to clipboard operation
llvm copied to clipboard

Published 20 hours ago • verified publisher icon intel

[SYCL] Add warning for attributes applied to non-kernel functions

Open dyniols opened this issue 1 year ago • 2 comments

According to 5.8.1. Kernel attributes, kernel attributes can only be applied to SYCL kernel functions, but not to a regular device functions.

Based on this information, this pull request introduces a warning diagnostic whenever specified attributes are applied to regular device functions.

dyniols avatar Aug 21 '24 15:08 dyniols

@intel/dpcpp-cfe-reviewers Can you review?

dyniols avatar Aug 26 '24 14:08 dyniols

@premanandrao I did requested changes can you look over them? Thanks

dyniols avatar Aug 28 '24 09:08 dyniols

@elizabethandrews Do you know how to check if function is a kernel in CollectSyclExternalFuncs?

dyniols avatar Sep 02 '24 09:09 dyniols

@elizabethandrews Do you know how to check if function is a kernel in CollectSyclExternalFuncs?

I think CollectSyclExternalFuncs only collects SYCL_EXTERNAL functions. So that is a dead end. Since you want the diagnostic emitted for non-device functions as well, I think it needs to be handled the way you currently do in the deferred diagnostics framework. I am not sure if I am missing something though. @premanandrao do you have insight here? I also noticed the diagnostics are not emitted on declarations which is a bit odd but I am not sure what the solution is

elizabethandrews avatar Sep 03 '24 23:09 elizabethandrews

Okay, I investigated it, and it seems that dependency checks on attributes were unnecessary.

dyniols avatar Sep 04 '24 12:09 dyniols

@elizabethandrews Could you look at my changes?

dyniols avatar Sep 09 '24 14:09 dyniols

@mdtoguchi - Driver/sycl.c on Windows has been failing in post-commit as of yesterday for unrelated changes (https://github.com/intel/llvm/pull/15297) and this PR is the first case I've seen it fail in pre-commit. Do you have any idea of how to address it? Can we disable it temporarily?

steffenlarsen avatar Sep 17 '24 11:09 steffenlarsen

@mdtoguchi - Driver/sycl.c on Windows has been failing in post-commit as of yesterday for unrelated changes (#15297) and this PR is the first case I've seen it fail in pre-commit. Do you have any idea of how to address it? Can we disable it temporarily?

@steffenlarsen, it should be OK to disable temporarily (maybe just use REQUIRES: system-linux) until I can figure out what is going on.

mdtoguchi avatar Sep 17 '24 13:09 mdtoguchi

Failure in Driver/sycl.c for Windows is known and will be disabled in https://github.com/intel/llvm/pull/15416.

steffenlarsen avatar Sep 17 '24 15:09 steffenlarsen