[AscendNPU-IR] Support CANN-9.0.0#843
[AscendNPU-IR] Support CANN-9.0.0#843happyhappyhappy-arch wants to merge 4 commits intotile-ai:npuirfrom
Conversation
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
There was a problem hiding this comment.
Code Review
This pull request introduces support for CANN 9.0.0.beta2 alongside the existing 8.5.0 version. Key changes include automated CANN version detection in CMake and shell scripts, version-specific MLIR codegen for the Fixpipe operation, and updated documentation for installation and environment variables. Feedback focuses on improving the robustness of version detection regex in CMake, ensuring absolute paths are used in build configurations to avoid path resolution issues, and properly initializing MLIR attributes in the C++ codegen to prevent potential verification failures or crashes.
| if(DEFINED _tilelang_ascend_home) | ||
| if(_tilelang_ascend_home MATCHES "cann-8\\.5\\.0(/|$)") | ||
| set(_tilelang_ascend_cann_version "8.5.0") | ||
| elseif(_tilelang_ascend_home MATCHES "cann-9\\.0\\.0-beta\\.2(/|$)") |
There was a problem hiding this comment.
The regex for detecting CANN 9.0.0-beta.2 is quite restrictive and might fail if the directory name differs slightly (e.g., cann-9.0.0-beta2 without the dot before 2). The Python utility and shell script use more flexible matching. Consider using a more inclusive regex to improve auto-detection robustness.
elseif(_tilelang_ascend_home MATCHES "cann-9\\.0\\.0.*beta.*2(/|$)")
| @@ -155,6 +245,7 @@ cd build | |||
|
|
|||
| echo "set(USE_NPUIR ON)" >> config.cmake | |||
| echo "set(BISHENGIR_ROOT_PATH $BISHENGIR_PATH)" >> config.cmake | |||
There was a problem hiding this comment.
The BISHENGIR_ROOT_PATH is written to config.cmake as a relative path and without quotes. This can cause issues if the path contains spaces or when CMake is run from the build directory, as it will look for the path relative to build/. It is recommended to use an absolute path and wrap it in quotes to ensure CMake can correctly locate the headers and libraries.
| echo "set(BISHENGIR_ROOT_PATH $BISHENGIR_PATH)" >> config.cmake | |
| BISHENGIR_PATH_ABS="$(cd "$BISHENGIR_PATH" 2>/dev/null && pwd || echo "$BISHENGIR_PATH")" | |
| echo "set(BISHENGIR_ROOT_PATH \"$BISHENGIR_PATH_ABS\")" >> config.cmake |
| if [ -z "$current_home" ]; then | ||
| echo "Error: neither --cann-version nor ASCEND_HOME_PATH is set." >&2 |
There was a problem hiding this comment.
Please use version 8.5.0 by default, as it is the version currently recommended by the community.
There was a problem hiding this comment.
Checked
|
|
||
| [tool.codespell] | ||
| ignore-words-list = "nd, te, ist, LOD, offen, NotIn, HSA, cann" | ||
| ignore-words-list = "nd, te, ist, LOD, offen, NotIn, HSA, cann, VOr" |
There was a problem hiding this comment.
The Vector Or opration, which in README.md as VOr.
In format CI, that op considered as a misspelling of "for".
3b733b5 to
eefbba5
Compare
This MR will be suspended until CANN releases version 9.0.0.