Repository navigation
sycl: fix compiler warnings - missing prototypes, dead code, extra semicolons - #30271
Merged
Merged
Conversation
Contributor
Author
github-actions
Bot
added
build
mctylr-gh
force-pushed
the
sycl-cmake
branch
2 times, most recently
from
October 10, 2026 09:14
a9153b2 to
62fd9a0
Compare
arthw
reviewed
Oct 12, 2026
The Intel fork of LLVM for SYCL support, and Intel oneAPI/DPC++ identifies itself as IntelLLVM, which cmake recongises. So treat it much like LLVM. Tell IntelLLVM to not disable optimizations when debugging information is includes, mostly of use for build type RelWithDebInfo, and Debug. I don't believe it is supported by mainstream versions of LLVM/Clang/Clang++. Treat oneAPI and LevelZero headers as system headers (suppress warnings).
- mark file-local functions `static` (only referenced in their own TU, fixes `-Wmissing-prototypes`): `ggml_sycl_op_swiglu_oai`, `ggml_sycl_op_upscale`, `get_zes_init_res`, `initialize_sycl_begining`, `is_bmg_g31_arch`, `sycl_get_mem_info`, `query_free_memory_by_ze`, `get_memory_size_by_sycl_api` - include the declaration header `gla.hpp` in `gla.cpp` so the prototype of `ggml_sycl_op_gated_linear_attn` is visible at its definition (called from `ggml-sycl.cpp`) - remove extra ';' after function / member definitions: `upscale` (x3), `vecdotq`, `dpct/helper.hpp`
- remove break / return unreachable after the `noreturn GGML_ABORT` / `GGML_ASSERT(false)` (`ggml_abort` is `GGML_NORETURN`): `concat`, `count-equal`, `fattn-tile`, `getrows`, `topk-moe`, `ggml-sycl` (x2), `convert` (x3) If the `GGML_ASSERT` definition was modified or redefined to be empty, similar to how C `assert()` acts when `NDEBUG` is defined, these changes could generate errors.
- remove extra ';' after function / member definitions: `dpct/helper.hpp`
…load `parallel_for(range, properties, kernel)`, used to advertise the Intel `grf_size_automatic` property for the flash-attention kernels, is deprecated in SYCL 2020. The recommended replacements are a `launch_config` object or a kernel *functor* with a `get(properties_tag)` member. `launch_config` cannot carry `grf_size` (it static_asserts against properties that have a compile-time kernel effect), so the kernel is wrapped in a functor that advertises the property through the properties_tag getter. - add `grf_size_kernel<KernelFunc, warp_size>`: a kernel functor that forwards to the kernel lambda and returns `grf_size_automatic` from a `const get(properties_tag)` member - `lauch_kernel`: use `grf_size_kernel` in the `use_large_grf` branch instead of the deprecated overload; the `use_large_grf = false` branch is unchanged - the `reqd_sub_group_size` attribute is also placed on the functor operator() it is written at the start of the declaration because icpx rejects it after the const qualifier Assisted-by: pi (unsloth/Qwen3.8-27B-GGUF:Q4_K_M via llama.cpp)
The kernel carried both the `reqd_sub_group_size` attribute and a kernel
property (`grf_size_automatic`), which icpx flags with
`-Wignored-attributes`. The oneAPI kernel-properties framework already
provides `sub_group_size`, so move the sub-group size from the attribute
into the same get(properties_tag) object that carries
`grf_size_automatic`.
- rename `grf_size_kernel` to `props_kernel<KernelFunc, Size, UseGrf>;` the
sub-group size is now a property (`sub_group_size<Size>`) returned from
the `const get(properties_tag)` member, and `grf_size_automatic` is
added only when `UseGrf` is true
- drop the `reqd_sub_group_size` attribute from the kernel lambda and the
functor operator(); no attribute remains on the kernel
- `lauch_kernel`: collapse the `use_large_grf` branch into a single
`parallel_for(rng, props_kernel<..., use_large_grf>{kernel})`
No behaviour change: on-device (bmg-g31) the property applies the same
sub-group size as the attribute (16), versus 32 by default.
Assisted-by: pi (unsloth/Qwen3.8-27B-GGUF:Q4_K_M via llama.cpp)
Correct incorrect, potentially misleading arguments in cmake `find_path()`. This revert my change in 0bb287db0 to the original behaviour.
Titaniumtown
approved these changes
Oct 20, 2026
Titaniumtown
changed the title
Contributor
|
Thank you @mctylr-gh! |
edwardyoon
pushed a commit
to edwardyoon/focus-llama
that referenced
this pull request
Oct 24, 2026
auRose94
pushed a commit
to auRose94/llama.cpp
that referenced
this pull request
Oct 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Primary: The current Intel oneAPI DPC++ and I believe intel/llvm (sycl branch) identify as IntelLLVM not as Clang in CMake's CMAKE_COMPILER_ID, which means that the default warnings are not configured for some/many SYCL builds.
Secondary: Small, behaviour-preserving cleanups in the SYCL backend, found while building with the Intel oneAPI DPC++ compiler (IntelLLVM), these are pre-existing warnings, once the cmake build includes the usual warning flags for IntelLLVM:
static(only referenced in their own file,no prior declaration; -Wmissing-prototypes)
cross-TU prototype of ggml_sycl_op_gated_linear_attn is visible at the definition
noreturn GGML_ABORT/GGML_ASSERT(false)Verified: the SYCL backend recompiles clean under
-Wall -Wextra -Wpedantic -Wmissing-prototypes -Wextra-semi. No new warnings introduced.Issue: #30269: Feature Request: SYCL build: file-local functions missing static, unreachable code after abort, extra semicolons
Additional information
either way; they are separate commit if maintainers want to keep the defensive
break/return after the abort.
parallel_for(properties)overload:Requirements
warnings and drafted the minimal edits from the compiler output; I
reviewed each change, understand it, and take responsibility for all of
it.