Skip to content

Conversation

@jikunshang
Copy link
Collaborator

@jikunshang jikunshang commented Sep 5, 2025

Essential Elements of an Effective PR Description Checklist

  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS ABOVE HAVE BEEN CONSIDERED.

Purpose

add pvc,bmg as default AOT build target. currently, we only enable bmg and pvc.
will add more xe3 aot options in the future, limited xe3 arch supported in ocloc 1146.
how to check supported aot option:
ocloc ids -h

Test Plan

Test Result

(Optional) Documentation Update

BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)

@Copilot Copilot AI review requested due to automatic review settings September 5, 2025 00:13
Copy link

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 enables Ahead-of-Time (AOT) compilation for SYCL GPU builds by adding configurable device target support with pvc and bmg as default targets.

  • Adds AOT compilation configuration with performance optimization flags
  • Replaces hardcoded SYCL link flags with configurable SYCL_LINK_OPTION variable
  • Sets pvc and bmg as default AOT device targets

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

set(PERF_LINK_OPTS "${PERF_LINK_OPTS} -Xfinalizer -enableBCR")
set(PERF_LINK_OPTS "${PERF_LINK_OPTS} -Xfinalizer -DPASTokenReduction")

set(LINKE_OPTS ${LINKE_OPTS}
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

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

Variable name 'LINKE_OPTS' contains a typo. It should be 'LINK_OPTS' for consistency and clarity.

Copilot uses AI. Check for mistakes.
set(LINKE_OPTS ${LINKE_OPTS} -fsycl-targets=spir64_gen)
endif()

set(LINKE_OPTS ${LINKE_OPTS} -Xs )
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

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

There's a trailing space after '-Xs' which could cause issues. Remove the trailing space or add a comment explaining why it's needed.

Suggested change
set(LINKE_OPTS ${LINKE_OPTS} -Xs )
set(LINKE_OPTS ${LINKE_OPTS} -Xs)

Copilot uses AI. Check for mistakes.
CMakeLists.txt Outdated
set(VLLM_EXTRA_INCLUDE_DIRECTORIES ${CMPLR_ROOT}/include/sycl)
list(APPEND VLLM_GPU_FLAGS "-DVLLM_BUILD_XPU_OPS" )
list(APPEND VLLM_GPU_LINK_FLAGS "-fsycl" "-fsycl-targets=spir64")
list(APPEND VLLM_GPU_FLAGS "-DVLLM_BUILD_XPU_OPS" -fsycl-targets=spir64_gen,spir64 )
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

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

The compiler flag '-fsycl-targets=spir64_gen,spir64' is hardcoded here but AOT configuration is handled elsewhere. Consider using the same configurable approach for consistency.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@rogerxfeng8 rogerxfeng8 left a comment

Choose a reason for hiding this comment

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

pls update the pr descriptions

@jikunshang
Copy link
Collaborator Author

pls update the pr descriptions

updated.

@jikunshang jikunshang changed the title Enable AOT build Enable AOT build for BMG, PVC Sep 15, 2025
Signed-off-by: Kunshang Ji <[email protected]>
Signed-off-by: Kunshang Ji <[email protected]>
Signed-off-by: Kunshang Ji <[email protected]>
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