Skip to content

Conversation

guangyey
Copy link
Contributor

Motivation

Clean up getDeviceIndexOfCurrentQueue since it duplicates current_device and provides no additional functionality.

@Copilot Copilot AI review requested due to automatic review settings September 17, 2025 09:18
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 removes the getDeviceIndexOfCurrentQueue() function and replaces its usage with the existing current_device() function to eliminate code duplication.

  • Removes duplicate function getDeviceIndexOfCurrentQueue() from Runtime.h
  • Updates function signatures to use current_device() and improved type consistency
  • Modernizes template code using if constexpr and std::is_same_v

Reviewed Changes

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

Show a summary per file
File Description
src/comm/Runtime.h Removes the duplicate getDeviceIndexOfCurrentQueue() function
src/comm/DeviceProperties.h Updates all function signatures to use current_device() and improves type consistency
src/ATen/native/xpu/sycl/TensorShapeKernels.cpp Simplifies device property access by removing explicit device ID retrieval
src/ATen/native/xpu/sycl/SoftMaxKernels.cpp Replaces device property access with getCurrentDeviceProperties()
src/ATen/native/xpu/sycl/Norm.h Simplifies device property access by removing explicit device ID retrieval
src/ATen/native/xpu/sycl/BatchNormKernels.cpp Updates to use current_device() instead of removed function

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@guangyey guangyey force-pushed the guangyey/clean branch 3 times, most recently from cf2f862 to c5e1422 Compare September 17, 2025 11:22
Copy link
Contributor

@EikanWang EikanWang left a comment

Choose a reason for hiding this comment

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

LGTM.

nit: syclMaxWorkItemsPerTile - Tile is not a good name, as we don't work on Title. The Tile here means each device.

@guangyey guangyey added this pull request to the merge queue Sep 18, 2025
Merged via the queue into main with commit 24fab67 Sep 18, 2025
25 checks passed
@guangyey guangyey deleted the guangyey/clean branch September 18, 2025 01:44
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.

2 participants