Skip to content

Enhance debug build[DRAFT] #1718

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Enhance debug build[DRAFT] #1718

wants to merge 3 commits into from

Conversation

chunhuanMeng
Copy link
Contributor

Introduces a new DEBUG_XPU environment variable to enhance configurability and debugging capabilities for XPU-related builds. The changes primarily focus on conditional logic adjustments and library setup modifications to support this new variable.

Debugging and Build Configuration Enhancements:

  • Addition of DEBUG_XPU Environment Variable: Introduced DEBUG_XPU to toggle debugging-specific behaviors during the build process. This replaces certain usages of BUILD_SEPARATE_OPS for debugging purposes.

  • Conditional Debugging Options for SYCL Kernel Compilation: Added logic to apply debugging flags (-g, -O0, etc.) to SYCL kernel compilation when DEBUG_XPU is set and the build type matches Debug or RelWithDebInfo.

Library Setup Modifications:

  • Linux Build Adjustments: When the DEBUG_XPU flag is enabled, the build system dynamically compiles and links each SYCL source file into its own separate static library, and then links all of them into the main torch_xpu_ops target

  • Windows Build Adjustments: Similar changes as for Linux.

@Copilot Copilot AI review requested due to automatic review settings June 4, 2025 06:09
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

Introduces a new DEBUG_XPU flag to control debug-specific build behavior, replacing debug uses of BUILD_SEPARATE_OPS and splitting SYCL sources into per-file static libraries on both Windows and Linux.

  • Add DEBUG_XPU variable in CMakeLists.txt and guard debug compiler flags in BuildFlags.cmake
  • Create individual SYCL static libraries for each source and link them with whole-archive options under DEBUG_XPU on Windows and Linux
  • Retain BUILD_SEPARATE_OPS as a fallback but remove its previous debug shortcut

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/BuildOnWindows.cmake New if(DEBUG_XPU) block to build and link per‐source SYCL libraries
src/BuildOnLinux.cmake Parallel changes for Linux with --whole-archive linker flags
cmake/BuildFlags.cmake Wrapped debug SYCL compilation flags (-g, -O0, etc.) in DEBUG_XPU guard
CMakeLists.txt Set DEBUG_XPU from environment and remove unconditional BUILD_SEPARATE_OPS
Comments suppressed due to low confidence (4)

cmake/BuildFlags.cmake:88

  • [nitpick] Add a brief comment explaining the DEBUG_XPU guard’s purpose and its relationship with CMAKE_BUILD_TYPE to help future maintainers understand why these flags are conditional.
if(DEBUG_XPU)

src/BuildOnWindows.cmake:36

  • CMake’s get_filename_component only accepts one mode per call. Split this into two calls—one for REALPATH and another for NAME_WLE—to extract both the absolute path and filename without extension.
get_filename_component(name ${sycl_src} NAME_WLE REALPATH)

src/BuildOnLinux.cmake:26

  • As with Windows, you must not combine NAME_WLE and REALPATH in one call. Use two separate get_filename_component calls to properly extract each property.
get_filename_component(name ${sycl_src} NAME_WLE REALPATH)

CMakeLists.txt:79

  • [nitpick] The old BUILD_SEPARATE_OPS logic for debug builds is commented out earlier in this file. Consider removing or cleaning up that obsolete block to improve readability.
set(DEBUG_XPU $ENV{DEBUG_XPU})

target_link_options(torch_xpu_ops PUBLIC
"-WHOLEARCHIVE:$<TARGET_FILE:${sycl_lib}>"
)
list(APPEND TORCH_XPU_OPS_LIBRARIES torch_xpu_ops)
Copy link
Preview

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

Appending torch_xpu_ops inside the loop adds duplicate entries for each SYCL source. Move this line outside the loop so the main library is appended only once.

Copilot uses AI. Check for mistakes.

@chunhuanMeng chunhuanMeng changed the title Enhance debug build Enhance debug build[DRAFT] Jun 4, 2025
@chunhuanMeng chunhuanMeng marked this pull request as draft June 4, 2025 07:25
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