Skip to content

Feature Request: SYCL build: file-local functions missing static, unreachable code after abort, extra semicolons #30269

Closed

Description

@mctylr-gh

Prerequisites

  • I am running the latest code. Mention the version if possible as well.
  • I carefully followed the README.md.
  • I searched using keywords relevant to my issue to make sure that I am creating a new issue that is not already open (or closed).
  • I reviewed the Discussions, and have a new and useful enhancement to share.

Feature Description

This isn't a such much a feature request, but an request to add an enhancement / update to the cmake build system. And the second part is a clean-up of warning messages.

Motivation

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.

Secondly, as a result of adding these warnings to the IntelLLVM based builds, the usual warning set (-Wall -Wextra -Wpedantic -Wmissing-prototypes -Wextra-semi) surfaces a number of small, pre-existing hygiene issues in ggml/src/ggml-sycl/. None are functional bugs; all are behaviour-preserving cleanups:

  • File-local helpers defined without static and with no prior
    declaration (flagged by -Wmissing-prototypes, which IntelLLVM applies to
    C++ and mainline Clang does not):
filename function
element_wise.cpp ggml_sycl_op_swiglu_oai
upscale.cpp ggml_sycl_op_upscale
ggml-sycl.cpp get_zes_init_res, initialize_sycl_begining, is_bmg_g31_arch, sycl_get_mem_info
mem.cpp query_free_memory_by_ze, get_memory_size_by_sycl_api

Each is only referenced in its own file, the surrounding helpers in the same files are already static, so this matches the existing pattern.

One exception is ggml_sycl_op_dated_linear_attn is called from ggml-sycl.cpp, so it is fixed by including the gla.hpp header that already declares it.

  • Unreachable break; / return; immediately after a noreturn call
    (GGML_ABORT / GGML_ASSERT(false); ggml_abort is GGML_NORETURN):
    concat, count-equal, fattn-tile, getrows, topk-moe, ggml-sycl (x2),
    convert (x3).
  • Extra semicolon ; after a function / member definition: upscale (x3),
    vecdotq (x1), and in dpct/helper.hpp.
  • fattn-common - replace deprecated parallel_for(properties) overload
  • fattn-common - express reqd_sub_group_size as a kernel property

Changes are split across files in case there any issues or concerns.

Possible Implementation

I will submit an PR of my draft for these issues. PR #30271

I welcome comments, constructive criticism, and feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions