Skip to content

[SYCL] Unify work-group scope variable checks - #23644

Open
inteltdelisle wants to merge 2 commits into
intel:syclfrom
inteltdelisle:work/work_group_scope_helper
Open

inteltdelisle wants to merge 2 commits into
intel:syclfrom
inteltdelisle:work/work_group_scope_helper

Conversation

@inteltdelisle

Copy link
Copy Markdown
Contributor

Four places in CodeGen checked whether a variable is a work-group local by looking for SYCLScopeAttr on the variable or on its type, with slight differences between them. Replace them with CGSYCLRuntime::getWorkGroupVarKind, which distinguishes hierarchical parallelism locals (attribute on the variable) from wg_scope types such as work_group_static (attribute on the type).

Generated code is unchanged.

Add CodeGen tests for wg_scope types, and extend the hierarchical parallelism tests to cover private_memory, wg_scope types and const locals.

Fixes #23626

Co-Authored-By: Claude Opus 5.5

Four places in CodeGen checked whether a variable is a work-group local
by looking for SYCLScopeAttr on the variable or on its type, with slight
differences between them. Replace them with CGSYCLRuntime::getWorkGroupVarKind,
which distinguishes hierarchical parallelism locals (attribute on the variable)
from wg_scope types such as work_group_static (attribute on the type).

Generated code is unchanged.

Add CodeGen tests for wg_scope types, and extend the hierarchical
parallelism tests to cover private_memory, wg_scope types and const locals.

Fixes intel#23626

Co-Authored-By: Claude Opus 5.5

Signed-off-by: Delisle, Thierry <thierry.delisle@intel.com>
@inteltdelisle
inteltdelisle requested a review from a team as a code owner October 27, 2026 17:53
@inteltdelisle
inteltdelisle requested a balanced review from Copilot October 27, 2026 17:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues
What changed in this PR

Unifies SYCL work-group scope variable detection in CodeGen by routing all checks through a single helper that distinguishes hierarchical hoisted locals from wg_scope types, and adds/extends CodeGenSYCL tests to cover these cases.

Changes:

  • Introduce CGSYCLRuntime::getWorkGroupVarKind() and use it in multiple CodeGen locations to consistently classify work-group variables.
  • Adjust initialization handling so only hierarchical hoisted locals get the special “no guarded init + store-based init” path.
  • Add/extend CodeGenSYCL tests for wg_scope-decorated types, private_memory, and const locals in hierarchical kernels.
File Description
clang/​test/​CodeGenSYCL/​wg_scope_class.cpp New test validating wg_scope-type variables are emitted in local AS and are zero-initialized like statics.
clang/​test/​CodeGenSYCL/​wg_init.cpp Extends hierarchical WG init test to cover private_memory and wg_scope types.
clang/​test/​CodeGenSYCL/​const-wg-init.cpp Extends const hierarchical WG local test with an explicit global undef check.
clang/​lib/​CodeGen/​CodeGenModule.cpp Use unified work-group variable classification to choose local address space for globals.
clang/​lib/​CodeGen/​CGSYCLRuntime.h Adds WorkGroupVarKind enum and getWorkGroupVarKind() API.
clang/​lib/​CodeGen/​CGSYCLRuntime.cpp Implements getWorkGroupVarKind() and uses it to validate WG local emission.
clang/​lib/​CodeGen/​CGDecl.cpp Replace ad-hoc attribute checks with unified kind query and refine WG init handling.

Review effort: Lite

Comment thread clang/test/CodeGenSYCL/wg_scope_class.cpp Outdated
Comment thread clang/test/CodeGenSYCL/wg_scope_class.cpp
Comment on lines +6 to +7
// scope alloca, and that a variable of a type with the wg_scope attribute is
// treated as a hierarchical work group local (no initializer).
According to copilot:
These checks hardcode Itanium-mangled internal symbol names, which makes the test more brittle to minor frontend/mangling changes. Prefer matching with a looser symbol regex (e.g. @{{.*}}AutoWGS{{.*}}) or anchoring on the IR properties being validated (addrspace(3), type, initializer) without requiring the exact mangled spelling.

Seems reasonnable.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SYCL] Add helper to check variable work group scope.

2 participants