vllm-project / vllm-project/afd-plugin
[RFC]: Restructure csrc/npu into ascend_kernels / pybind / scripts around the CANN npu_op_* build system
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 228
- Forks
- 48
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 53
Description
Motivation
csrc/npu/ currently mixes three unrelated concerns in one flat namespace: the
ACLNN operator sources (a2e/, e2a/), the PyTorch extension
(torch_extension/ + aclnn_torch_adapter/), and the CANN build system
(build.sh, cmake/, build_aclnn.sh). The flat layout has four concrete
costs:
- Duplicated shared headers.
comm_args.h,data_copy.h, and
moe_distribute_base.hare vendored twice, once undera2e/op_kernel/and
once undere2a/op_kernel/. The two copies are byte-identical today, so any
fix to the MoE distribute base must be applied twice with nothing keeping
them in sync. - Operator selection is hard-coded shell control flow.
build_aclnn.sh
passes-n "a2e;e2a"and maps SOC to operator set throughcase
statements. Adding an operator or a SOC generation means editing shell, and
nothing validates that a requested operator is buildable for the requested
SOC. - A vendored generic CANN build wrapper with no owner.
build.shis a
~200-line "build open project" script carrying ccache/bisheng handling and
-DCUSTOM_ASCEND_CANN_PACKAGE_PATHplumbing that AFD never varies. It is
CANN-licensed boilerplate maintained in-tree, so every CANN bump is a
potential merge. - The A5 tiling gate is fragile.
AFD_TILING_HAS_COMM_ENGINEgates
Mc2CcTilingConfig::SetCommEngine(3)for Ascend 950 ina2e.cppand
e2a.cpp. In the current tree it is wired into the code-generation compile
options in a way that does not take effect (see Risks).
The operator half of this has an established upstream shape: the CANN
npu_op_* run-package build system, the same structure used by the
umdk/cam comm_operator ascend_kernels project. Aligning with it lets us
delete the vendored wrapper instead of maintaining it.
Proposed change
Split the directory along concern boundaries, and drive operator selection from
data instead of shell logic. Diff: 56 files, +1576/−3170.
csrc/npu/
ascend_kernels/ # ACLNN operator run package (npu_op_* build system)
CMakeLists.txt # npu_op_package(... TYPE RUN) driver
AddCustom.json # msopgen input template
operator_registry.json # SOC -> operators map + per-op build metadata
cmake_files/ # cmake/, op_host/, op_kernel/ build fragments
a2e/{op_api,op_host,op_kernel}
e2a/{op_api,op_host,op_kernel}
utils/op_kernel/ # shared headers, deduplicated
pybind/ # PyTorch extension (_C_ascend)
CMakeLists.txt
torch_binding.cpp, torch_binding_meta.cpp
pytorch_extension/ # NPUBridge, NPUStorageImpl, op_api_common.h
scripts/
compile_ascend_proj.sh # msopgen + cmake driver for one SOC
select_ops.py # registry-driven operator resolution
set_conf.py # patch generated CMakePresets.json
build_aclnn.sh # thin entry point, calls scripts/
Behavioral changes, called out because they are not pure moves:
- Operator selection becomes registry-driven.
operator_registry.jsonmaps
a SOC generation (ascend910_93,ascend950) to the operator directories it
supports, plus per-operator metadata.select_ops.pyvalidates the requested
selection against the registry, rejects unknown SOCs, unknown operators, and
empty entries, then applies the SHMEM availability filter. Adding a SOC
generation becomes a registry entry rather than acasebranch. - Op host sources consolidate.
a2e_def.cpp/a2e_proto.cpp/
a2e_tiling.cppfold intoa2e/op_host/a2e.cpp;e2a_tiling.cppfolds into
e2a/op_host/e2a.cpp.op_api/moves under each operator instead of being a
sibling ofop_host/. - Shared headers deduplicate into
ascend_kernels/utils/op_kernel/, copied
unconditionally by the build driver. - Run-package naming becomes deterministic.
build_aclnn.shpreviously
globbedoutput/CANN-custom_ops*.runand required exactly one match. The new
driver emits a fixedAFD_${SOC_VERSION}.runand the entry point asserts that
exact path. build.shis deleted. ItsASCEND_CANN_PACKAGE_PATHresolution moves
intocompile_ascend_proj.sh(tryingASCEND_HOME_PATH, then
ASCEND_TOOLKIT_HOME, then the default toolkit path). Parallelism becomes an
explicitAFD_BUILD_JOBS(default 8) instead of2 x nproc.setup.pyCMake source dir moves fromcsrc/npu/torch_extensionto
csrc/npu/pybind. The extension target nameafd_plugin._C_ascendand the
installed artifact layout are unchanged.vendor_nameis set toafd-pluginbyset_conf.py, so the run package
installs undervendors/afd-plugin/, preserving the existing
afd_plugin/_cann_ops_custom/vendors/afd-plugin/op_api/librpath contract.AFD_TILING_HAS_COMM_ENGINEordering fixed in
ascend_kernels/cmake_files/op_host/CMakeLists.txt— see Risks.
Plugin boundary
Everything here is plugin-owned in-tree C++/CMake; no vLLM source is touched.
- Plugin-owned: all of
csrc/npu/, the_C_ascendextension target, the
afd_ascendTorch library namespace, and the ACLNN operator build. - Compat helper: none.
- Compat patch: none. This change does not touch
afd_plugin/compat/. - Explicit class path: unchanged —
afd_plugin._C_ascendremains the import
surface, andafd_plugin/_cann_ops_custom/remains the install surface. - Build-time dependency change: the operator build now depends on the CANN
npu_op_*CMake package (find_package(ASC REQUIRED)) and onmsopgen,
rather than on the in-treecmake/+build.shpair. Reviewers should
confirm the minimum CANN version this implies.
Risks and alternatives
- A5 tiling define did not reach the code-generation path. In
cmake_files/op_host/CMakeLists.txt,list(APPEND CODE_GEN_COMPILE_OPTS -DAFD_TILING_HAS_COMM_ENGINE=1)was followed by an unconditional
set(CODE_GEN_COMPILE_OPTS -I${CMAKE_CURRENT_SOURCE_DIR}), which clobbered
the append beforenpu_op_code_gen(... COMPILE_OPTIONS ...)consumed it.
Confirmed withcmake -P: the 950 case produced[]instead of the expected
[-I...;-DAFD_TILING_HAS_COMM_ENGINE=1]. This is not a compile failure — the
#ifdefblock only callsSetCommEngine(3)at runtime on
SocVersion::ASCEND950— so the effect is a silently skipped A5 tiling
setting, which is exactly the kind of drift that is hard to attribute later.
Fixed by initialisingCODE_GEN_COMPILE_OPTSbefore the gate and appending
inside it; re-verified withcmake -Pthat 950 now carries the define and
ascend910_93does not. - Loss of build knobs.
build.shoffered ccache/bisheng acceleration,
CHECK_COMPATIBLE, and--cov/--verbose. The new driver drops them. If any
are load-bearing for CI or local NPU iteration, they need to be reintroduced
before the old script is deleted. - Behavior change for consumers of the run package. Anything globbing
CANN-custom_ops*.runbreaks. In-tree the only glob was inbuild_aclnn.sh
(updated), but downstream packaging scripts may assume the old name. - No functional change is intended to the a2e/e2a kernels or the pybind
surface. Reviewers should treat any semantic diff in the moved sources as a
bug, except the intentionalTWO_DIMSmagic-number extraction in
e2a/op_host/e2a.cpp. - Minor:
AddCustom.jsonandoperator_registry.jsoncarry no license
header. Pre-commit's SPDX check covers onlypy/pyi/rs/proto/sh, so this is
not CI-enforced; raising it only for reviewer awareness. - Alternatives considered: (a) keep the flat layout and only deduplicate the
three shared headers — smaller diff, but leaves shell-encoded operator
selection and the vendoredbuild.shin place; (b) keepbuild.shand layer
the registry on top — avoids the CANNnpu_op_*dependency, but keeps two
build systems alive. Option (a) is the lower-risk fallback if reviewers judge
the CANN-version coupling too costly.
Feedback period
Two weeks from posting, or until the affected code owners sign off, whichever
comes first.
CC list
@hsliuustc0106 @jiangkuaixue123 (per .github/CODEOWNERS).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by mapping csrc/npu/ against the proposed ascend_kernels, pybind, and scripts layout, then read build_aclnn.sh, cmake_files/op_host/CMakeLists.txt, and setup.py. Run the stated cmake -P check and inspect select_ops.py, set_conf.py, and operator_registry.json. Done means the build uses the registry-driven npu_op_* flow, preserves the pybind/install surface, and retains the documented A5 tiling behavior without the old build.sh.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp, python, shell
- Domain
- build-system, tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100