Skip to content

Refactor SYCL Build Flags #1822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Refactor SYCL Build Flags #1822

wants to merge 2 commits into from

Conversation

chunhuanMeng
Copy link
Contributor

SYCL Build Flag Updates

Added SYCL Target Options for Kernel Compilation

Included -fsycl-targets=spir64_gen,spir64 in SYCL_KERNEL_OPTIONS to explicitly specify SYCL targets during kernel compilation, regardless of whether AOT (Ahead-of-Time) compilation is enabled.

Refactored SYCL Flags for Device Linking

Consolidated device linking flags into SYCL_LINK_FLAGS instead of using SYCL_FLAGS, ensuring proper flag usage during the device linking stage.

Removed Redundant SYCL Target Options in AOT Compilation

Eliminated duplicate definitions of SYCL_TARGETS_OPTION and its inclusion in SYCL_KERNEL_OPTIONS under the AOT compilation path to avoid redundancy.

@Copilot Copilot AI review requested due to automatic review settings July 8, 2025 08:38
@chunhuanMeng chunhuanMeng changed the title Meng fix aot none Refactor SYCL Build Flags Jul 8, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances SYCL build configuration by explicitly specifying compilation targets, refactoring link flags, and removing redundant Ahead-of-Time (AOT) options.

  • Add -fsycl-targets=spir64_gen,spir64 to kernel compilation flags
  • Use a dedicated SYCL_LINK_FLAGS for device linking instead of SYCL_FLAGS
  • Remove duplicate target-option definitions in the AOT branch

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
cmake/Modules/FindSYCL.cmake Swapped out SYCL_FLAGS for SYCL_LINK_FLAGS in SYCL_link_device_objects
cmake/BuildFlags.cmake Introduced SYCL_TARGETS_OPTION, updated SYCL_KERNEL_OPTIONS and SYCL_FLAGS, and cleaned up AOT-specific duplication
Comments suppressed due to low confidence (2)

cmake/BuildFlags.cmake:75

  • [nitpick] The variable name SYCL_TARGETS_OPTION is singular but holds multiple targets. Renaming it to SYCL_TARGETS_OPTIONS could improve clarity and consistency.
  set(SYCL_TARGETS_OPTION -fsycl-targets=spir64_gen,spir64)

cmake/Modules/FindSYCL.cmake:386

  • SYCL_LINK_FLAGS is referenced here but not defined in BuildFlags.cmake. Consider initializing or defining SYCL_LINK_FLAGS before use to ensure device linking flags are correctly applied.
        ${SYCL_LINK_FLAGS}

@chunhuanMeng chunhuanMeng requested a review from fengyuan14 July 9, 2025 01:49
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.

1 participant