Skip to content

refactor: simplify xllm_ops pre-build checks and marker-based rebuild logic.#1111

Open
LMX-xin wants to merge 1 commit intojd-opensource:mainfrom
LMX-xin:bugfix/fix_rec
Open

refactor: simplify xllm_ops pre-build checks and marker-based rebuild logic.#1111
LMX-xin wants to merge 1 commit intojd-opensource:mainfrom
LMX-xin:bugfix/fix_rec

Conversation

@LMX-xin
Copy link
Copy Markdown
Collaborator

@LMX-xin LMX-xin commented Mar 25, 2026

Summary

  • skip xllm_ops precompilation when the installed marker matches the source git HEAD
  • trigger xllm_ops precompilation only when the marker is missing or stale
  • add a CANN version guard before rebuilding xllm_ops, and require CANN 8.5 for compilation

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the pre-compilation logic for xllm_ops in CMakeLists.txt and utils.py. The changes introduce a new mechanism using a .xllm_ops_git_head marker file to track the third_party/xllm_ops git HEAD, replacing the previous CMake cache variable approach. This ensures xllm_ops is rebuilt only when its source code changes or the marker is missing. Additionally, the Python script now includes robust error handling for git operations and enforces a CANN version 8.5 requirement for compilation. The review comments point out a potential inconsistency in both CMakeLists.txt and utils.py regarding the precedence of ASCEND_HOME_PATH and ASCEND_OPP_PATH when determining the opp root, suggesting that ASCEND_OPP_PATH might be expected to have higher priority.

DongheJin
DongheJin previously approved these changes Mar 26, 2026
RobbieLeung
RobbieLeung previously approved these changes Mar 26, 2026
@LMX-xin LMX-xin dismissed stale reviews from RobbieLeung and DongheJin via 1d987a1 March 30, 2026 08:20
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.

4 participants