Conversation
jxt1234
commented
Feb 23, 2026
- LLM QNN 后端支持设置 max_history_token ,此时 Attention 使用 QNN 实现
- 部分 QNN 算子 bug 修正
QNN:Refractor: Use op name as qnn's graph op name as default QNN:Bugfix: Fix bug for reshape with common nc4hw4 case QNN:Bugfix: When output quant is nullptr, don't support convolution quant QNN:Refractor: Opt the log when can't find input / output QNN:Refractor: Transpose Q, K, V's matmul axis for easy to use kvcache QNN:Feature: Support kv_cache for QNNAttention
26f7cc0 to
c5d5bad
Compare
LLM:Feature: Support max_history_token for qnn
c5d5bad to
32f9072
Compare
There was a problem hiding this comment.
PR #4174: Feature/qnnfusesupport 代码审查
总体评估
本 PR 为 QNN 后端的 LLM Attention 增加了 KV Cache 支持(通过 max_history_token 配置),并修正了若干 QNN 算子 Bug。改动涉及 17 个文件,+971/-278 行。整体方向正确,但存在若干正确性、安全性和代码质量问题需要关注。
具体问题
1. CMakeLists.txt: glob 模式过于宽泛
- 文件:
source/backend/qnn/CMakeLists.txt, 第 4-5 行 file(GLOB BACKEND_SRCS ${CMAKE_CURRENT_LIST_DIR}/backend/*)将匹配所有文件,包括.h、.md、.txt等非源文件。新增了dsprpc_interface.cc文件(非.cpp后缀),这才是改为*的原因,但更好的做法是:
file(GLOB BACKEND_SRCS ${CMAKE_CURRENT_LIST_DIR}/backend/*.cpp ${CMAKE_CURRENT_LIST_DIR}/backend/*.cc)2. 全局可变静态变量 gExtraIoPrefix 线程安全问题
- 文件:
source/backend/qnn/backend/QNNBackend.cpp, 第 25 行 static std::string gExtraIoPrefix = "_mnn";是一个非 const 的全局 staticstd::string。如果它不会被修改,应该声明为static const std::string或者使用static constexpr const char*。
3. RPCBuffer 析构函数中未检查 nullptr
- 文件:
source/backend/qnn/backend/QNNBackend.cpp,RPCBuffer::~RPCBuffer() - 析构函数直接调用
rpcmem_free(mPtr),但如果mPtr为 nullptr(虽然alloc失败时不会创建对象,但需考虑其他路径),行为取决于底层rpcmem_free实现。建议添加 nullptr 检查以增强防御性。
4. _findInput 和 _findOutput 中引用了未定义的变量 inputs[i]
- 文件:
source/backend/qnn/backend/QNNBackend.cpp,_findInput和_findOutput函数 - 在
#ifdef QNN_VERBOSE块中的MNN_PRINT("input name: %s %s\n", inputs[i].second.c_str(), dstT.v1.name);,此处inputs和i在函数作用域内不存在(函数参数是name和index)。虽然在QNN_VERBOSE未定义时不会编译,但一旦启用就会导致编译错误。应改为name.c_str()。
5. rpcmem_init() 调用缺少对应的 rpcmem_deinit()
- 文件:
source/backend/qnn/backend/QNNBackend.cpp,registerQNNRuntimeCreator中 rpcmem_init()在注册时调用,但在整个代码中没有看到对应的rpcmem_deinit()调用,存在资源泄漏。
6. setupState 中 mask->setToTensor 返回值未检查
- 文件:
source/backend/qnn/backend/QNNBackend.cpp,setupState函数 mask->setToTensor可能返回false(memRegister失败),但调用处未检查返回值。类似地,statesInputs[i]->setToTensor和statesOutput[i]->setToTensor的返回值也被忽略了。
7. _loadState 中缺少 mStateMaxSize 为 0 的边界检查
- 文件:
source/backend/qnn/backend/QNNBackend.cpp,_loadState函数 - 当
stateNumber > 0但mStateMaxSize == 0时,会执行RPCBuffer::alloc(0),可能产生未定义行为。
8. compute 函数中 KV Cache 状态更新可能越界
- 文件:
source/backend/qnn/backend/QNNBackend.cpp,PluginExecuteRaw::compute mStateCurrent += meta->add累加后,没有检查mStateCurrent是否超过mStateMaxSize。如果累计 token 数超过mStateMaxSize,maskPtr[i+mStateCurrent]将越界写入,导致内存损坏。
9. mStateCurrent -= meta->remove 可能导致负值
- 文件:
source/backend/qnn/backend/QNNBackend.cpp,PluginExecuteRaw::compute - 如果
meta->remove > mStateCurrent,mStateCurrent会下溢(类型为int,变为负数),后续maskPtr[i+mStateCurrent]访问将越界。
10. invokModel 函数名拼写错误
- 文件:
source/backend/qnn/backend/QNNBackend.cpp invokModel应为invokeModel。虽然原代码就存在此拼写错误,但既然重构了函数签名,这是修正的好时机。
11. createStaticFloatTensor 的 coef 维度与数据不匹配
- 文件:
source/backend/qnn/execution/QNNAttention.cpp coef的维度被改为{1, 1, 1, 1},但scaleVec大小为totalSize = batch * headNum * seqLenQ * headDim。这意味着只有第一个元素会被使用,其余的scaleVec数据被忽略。如果 QNN 支持广播,这可能是有意的优化,但值得在注释中说明。
12. dsprpc_interface.cc 中 load_fn 未处理 lib == nullptr 的情况
- 文件:
source/backend/qnn/backend/dsprpc_interface.cc,load_fn函数 - 当
load_lib()返回nullptr时(dlopen失败),load_fn仍然调用dlsym(lib, fn_name),此时dlsym(nullptr, ...)的行为是搜索默认符号表,这可能不是预期行为。
13. llm.cpp 缩进不一致
- 文件:
transformers/llm/engine/src/llm.cpp - 修改后的代码块使用了 3 空格缩进(
if (mConfig->backend_type() == "cpu" && mValidBlockSize.empty())),而周围代码使用 4 空格缩进或 tab。这可能是因为在 diff 中混合了制表符和空格。
14. positionIds 维度变更可能引入兼容性问题
- 文件:
transformers/llm/engine/src/llm.cpp和transformers/llm/engine/tools/generateLlmIO.cpp positionIds从{seqLen}改为{1, seqLen},这是一个影响模型输入格式的变更。需确认所有使用 positionIds 的模型和前处理代码都兼容新的 2D 格式。
15. ModuleBasic.cpp 中 KVMeta 结构体重复定义
- 文件:
tools/cpp/ModuleBasic.cpp KVMeta结构体在此文件中重新定义。如果QNNBackend.cpp中的KVMeta也存在(通过meta = (KVMeta*)(ctx->backend()->getMetaPtr())),说明这个结构体需要在公共头文件中定义一次,而非在多处重复定义。结构体不一致将导致内存布局错误。
16. addStaticTensorToGraph 和 addStageTensorToGraph 合并后丢失断言
- 文件:
source/backend/qnn/backend/QNNBackend.hpp和.cpp - 原来的
addStaticTensorToGraph有断言MNN_ASSERT(staticTensor->v1.type == QNN_TENSOR_TYPE_STATIC),addStageTensorToGraph有断言MNN_ASSERT(stageTensor->v1.type == QNN_TENSOR_TYPE_NATIVE)。合并为addTensor后这些断言被移除,降低了调试时发现类型错误的能力。
17. 注释中残留的调试代码
- 文件:
source/backend/qnn/backend/QNNBackend.cpp - 多处保留了注释掉的
MNN_PRINT调试语句(如// MNN_PRINT("All Inputs Begin\n"),// inputs[i]->printShape())。提交前应清理。
亮点
- 将
QNNAttention中基于索引的mTempTensorWrappers[N]访问方式重构为命名变量(如Query_perm,scaleQ,QK等),大幅提升了可读性。 dsprpc_interface使用dlopen/dlsym动态加载方式,避免了对 QNN SDK 库的硬链接依赖。QNNCommonExecution的create*方法改为返回shared_ptr,使调用方无需依赖全局索引数组。setNodeName增加了对op->name()的支持,优先使用算子的原始名称。
总结
问题 8,9(越界)是 blocker,必须修;4 和 15 修复成本低但隐患大,强烈建议一并修复
Reviewed by claude-opus-4-6
@jxt1234 8,9要不增加一下判断保护一下吧 |
…d fix QNN op bugs (alibaba#4174)