feat: remove redundant fusedQKV bias transpose kernel in vision bert … - #1361
feat: remove redundant fusedQKV bias transpose kernel in vision bert …#1361junna2016 wants to merge 2 commits into
Conversation
PR #1361 第 2 轮评审 — LGTM
无阻塞项非阻塞发现（并集报告：单票也保留，降级不丢弃）
本轮 KPI{
"reps": 5,
"agentic_reps": 2,
"diff_truncated": false,
"patch_bytes": 3097,
"coverage": 1.0,
"shards": 1,
"discover_ok": 5,
"discover_fail": 0,
"discover_attempts": 5,
"clusters": 3,
"synth_fail": 0,
"confirm": {
"candidates": 0,
"to_block": 0,
"confirm_fail": 0
},
"blocking_dedup_merged": 0,
"fact_check": {
"checked": 0,
"hold": 0,
"refuted": 0,
"parse_fail": 0
},
"model": "whale/DeepSeek-V4-Pro-0813",
"round_index": 2,
"delta_files": 3,
"full_files": 4,
"new_findings": 3,
"merged": 0,
"recheck": {
"fixed": 0,
"still_open": 2,
"cannot_substantiate": 0,
"fail": 0
},
"blocking": 0,
"blocking_on_unchanged": 0
}本报告由 rtp-llm-agent-platform 自动生成，同一 PR 的后续轮次就地更新同一条评论。 rtp-llm-agent-platform review · 第 2 轮 · head |
LLLLKKKK
left a comment
LLLLKKKK
left a comment
There was a problem hiding this comment.
AI Code Review - PR #1361
Status: BLOCKING
Summary: P0/0 · P1/1 · P2/2 · P3/2
Reviewed: commit d103edda15c6 · 2026-08-31 20:26 UTC+8
Blocking Issues
P1
- 新增派生分支缺少单测，且仓内 smoke 对该分支为零差异覆盖 @
rtp_llm/cpp/config/ModelConfig.cc:191- 建议：补一个聚焦测试固定真值表：
(rope=No, use_kvcache=false)→ false；(rope=No, use_kvcache=true)→ 保持入参；(rope=Base, use_kvcache=false)→ 保持入参；入参显式为 true 时的覆盖行为也需断言。getAttentionConfigs已在pybind/ConfigInit.cc:2009导出，写 py_test 成本最低，也可新建rtp_llm/cpp/config/test/用cc_test_wrapper。端到端侧建议在 CUDA suite 补一条 bert/roberta embedding 用例（golden 资产data/model/bert/expect.pt、roberta_expect.pt已存在，目前仅被 ROCm 消费），或在 PR 描述中说明目标模型在哪条用例/平台做过前后对比及耗时数据。
- 建议：补一个聚焦测试固定真值表：
Non-blocking Suggestions
P2
- 判定条件与注释宣称的不变量不等价，use_logn_attn 的 q 缩放会被静默丢弃 @
rtp_llm/cpp/config/ModelConfig.cc:191- 建议：把条件收紧为显式表达前提，例如追加
&& !config.use_logn_attn && !config.use_mla（两字段均在AttentionConfig.h:38/42），或对use_logn_attn && style == No && !use_kvcache这类矛盾组合 fail-fast 抛错，避免精度问题只能靠离线比对发现；同时把注释改为描述实际条件（「无通用 RoPE kernel 且无 KV cache」），不要让读者按更强的不变量去信任该分支。
- 建议：把条件收紧为显式表达前提，例如追加
- 中心派生无条件覆盖模型侧显式配置，生效面大于注释所述且无日志与回滚手段 @
rtp_llm/cpp/config/ModelConfig.cc:192- 建议：三选一收口：(1) 优先使用已有本地扩展点，在目标模型
_create_config中设置（与models/bert.py:38一致），中心逻辑保持不动；(2) 保留「显式设置优先」语义，仅在字段仍为默认值时才派生；(3) 若坚持中心化，请在 PR 描述中列出受影响 task_type 与模型清单，并在覆盖生效时打一条一次性 INFO 日志（含 rope style、use_kvcache、覆盖前后取值，放初始化路径而非 per-forward），必要时提供一个可关闭该优化的开关作为运维回滚手段。同时把注释中的 "embedding models" 改为准确表述（非 LANGUAGE_MODEL 任务且无 RoPE）。
- 建议：三选一收口：(1) 优先使用已有本地扩展点，在目标模型
P3
- 同一决策存在三处独立编码，谓词重复、注释表述过宽并混入空行删除 @
rtp_llm/cpp/config/ModelConfig.cc:188- 建议：提取局部常量（如
const bool no_rope_no_kvcache = config.rope_config.style == RopeStyle::No && !use_kvcache;）供 188/191 两处复用；注释改为按后端限定表述；恢复被删除的空行以保持分段风格。真源建议二选一收敛：以need_rope_kv_cache为唯一权威来源（删除bert.py:38，并让trt.py改读该字段），或在AttentionConfig.h:52注明该字段的权威来源与各后端遵循情况（明确 trt.py 使用独立判据）。
- 建议：提取局部常量（如
- flag 语义过载，跳过 KV 写入依赖未被断言保护的隐式不变量 @
rtp_llm/cpp/config/ModelConfig.cc:192- 建议：在
AttentionConfig.h:52补一行注释说明该字段同时决定是否写 KV cache，避免后续调用方误用；或在读取该 flag 的 impl 构造处对need_rope_kv_cache == False and kv_cache is not None做显式 assert/raise，把当前的隐式不变量固化为 fail-fast 检查。
- 建议：在
Checklist Findings (12 fail / 26 total)
General Principles Checklist
- [6.1] Architecture — 兼容性：外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue
中心派生无条件覆盖模型侧显式配置，生效面大于注释所述且无日志与回滚手段
use_kvcache由rtp_llm/config/model_config.py:912推导为task_type == LANGUAGE_MODEL，RopeStyle::No又是RopeConfig.h:22的默认值，因此该分支覆盖全部非 LM 任务（DENSE/SPARSE/COLBERT/BGE_M3/SEQ_CLASSIFICATION/RERANKER/LINEAR_SOFTMAX 等）下所有未显式设 rope 的模型，包含 Baichuan-13B（models/llama.py:153，ALiBi）、Cohere（llama.py:220）被部署为 embedding/reranker 的情形，远超标题声明的 vision bert。赋值为无条件覆盖：经 pybind 显式设 true 的配置（pybind/ConfigInit.cc:1657）被静默丢弃；to_string()（ModelConfig.cc:213）打印的是派生前成员，日志看不到实际生效值，也无 env/参数开关可回滚。 - [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue
中心派生无条件覆盖模型侧显式配置，生效面大于注释所述且无日志与回滚手段
use_kvcache由rtp_llm/config/model_config.py:912推导为task_type == LANGUAGE_MODEL，RopeStyle::No又是RopeConfig.h:22的默认值，因此该分支覆盖全部非 LM 任务（DENSE/SPARSE/COLBERT/BGE_M3/SEQ_CLASSIFICATION/RERANKER/LINEAR_SOFTMAX 等）下所有未显式设 rope 的模型，包含 Baichuan-13B（models/llama.py:153，ALiBi）、Cohere（llama.py:220）被部署为 embedding/reranker 的情形，远超标题声明的 vision bert。赋值为无条件覆盖：经 pybind 显式设 true 的配置（pybind/ConfigInit.cc:1657）被静默丢弃；to_string()（ModelConfig.cc:213）打印的是派生前成员，日志看不到实际生效值，也无 env/参数开关可回滚。 - [6.1] Architecture — 回滚路径：风险行为存在运维回滚手段 → issue
中心派生无条件覆盖模型侧显式配置，生效面大于注释所述且无日志与回滚手段
use_kvcache由rtp_llm/config/model_config.py:912推导为task_type == LANGUAGE_MODEL，RopeStyle::No又是RopeConfig.h:22的默认值，因此该分支覆盖全部非 LM 任务（DENSE/SPARSE/COLBERT/BGE_M3/SEQ_CLASSIFICATION/RERANKER/LINEAR_SOFTMAX 等）下所有未显式设 rope 的模型，包含 Baichuan-13B（models/llama.py:153，ALiBi）、Cohere（llama.py:220）被部署为 embedding/reranker 的情形，远超标题声明的 vision bert。赋值为无条件覆盖：经 pybind 显式设 true 的配置（pybind/ConfigInit.cc:1657）被静默丢弃；to_string()（ModelConfig.cc:213）打印的是派生前成员，日志看不到实际生效值，也无 env/参数开关可回滚。 - [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue
flag 语义过载，跳过 KV 写入依赖未被断言保护的隐式不变量
need_rope_kv_cache=false在下游不只跳过 rope 融合 kernel，还同时跳过 KV cache 写入：py_flashinfer_mha.py:821-822不再调用kv_cache_write_op，而PyFlashinferPagedPrefillImpl._prepare_fmha_input（同文件 859-863）只回传(query,)，K/V 必须从 paged cache 读出；rocm_impl/aiter.py:1610-1613同理直接fmha_input = qkv且不写 cache。当前不出错仅因新条件含!use_kvcache，从而保证kv_cache is None（models_py/model_desc/bert.py:153）。一旦后续出现use_kvcache=false但仍挂载 cache 的组合，attention 会静默读到未写入的 KV，属无报错的错误输出。 - [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue
flag 语义过载，跳过 KV 写入依赖未被断言保护的隐式不变量
need_rope_kv_cache=false在下游不只跳过 rope 融合 kernel，还同时跳过 KV cache 写入：py_flashinfer_mha.py:821-822不再调用kv_cache_write_op，而PyFlashinferPagedPrefillImpl._prepare_fmha_input（同文件 859-863）只回传(query,)，K/V 必须从 paged cache 读出；rocm_impl/aiter.py:1610-1613同理直接fmha_input = qkv且不写 cache。当前不出错仅因新条件含!use_kvcache，从而保证kv_cache is None（models_py/model_desc/bert.py:153）。一旦后续出现use_kvcache=false但仍挂载 cache 的组合，attention 会静默读到未写入的 KV，属无报错的错误输出。 - [6.1] Quality — 逻辑变更未混入无关格式化 → issue
同一决策存在三处独立编码，谓词重复、注释表述过宽并混入空行删除
config.rope_config.style == RopeStyle::No && !use_kvcache在 188 行与 191 行逐字重复，后续调整口径易漏改。同一决策另有两处编码：models/bert.py:38的模型级赋值（本 PR 未清理），以及cuda_impl/trt.py:377/397-401用自有运行时判据need_rope or kv_cache is not None跳过同一 fused op、完全不读该 flag。注释「Skip rope+bias+transpose kernel entirely」「PACKED_QKV layout matches FMHA input directly」只对 headwise/aiter/trtllm_gen 成立：py_flashinfer_mha.py:855/902在 style=No 时 rope_impl 本就是 None，该路径走_split_qkv，唯一变化是跳过 821 行的 KV 写入。diff 还删掉了 188 行后的空行，与函数内其他派生段落风格不一 - [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue
同一决策存在三处独立编码，谓词重复、注释表述过宽并混入空行删除
config.rope_config.style == RopeStyle::No && !use_kvcache在 188 行与 191 行逐字重复，后续调整口径易漏改。同一决策另有两处编码：models/bert.py:38的模型级赋值（本 PR 未清理），以及cuda_impl/trt.py:377/397-401用自有运行时判据need_rope or kv_cache is not None跳过同一 fused op、完全不读该 flag。注释「Skip rope+bias+transpose kernel entirely」「PACKED_QKV layout matches FMHA input directly」只对 headwise/aiter/trtllm_gen 成立：py_flashinfer_mha.py:855/902在 style=No 时 rope_impl 本就是 None，该路径走_split_qkv，唯一变化是跳过 821 行的 KV 写入。diff 还删掉了 188 行后的空行，与函数内其他派生段落风格不一 - [6.1] Software Engineering — OCP：本地扩展点优先于修改中心逻辑 → issue
中心派生无条件覆盖模型侧显式配置，生效面大于注释所述且无日志与回滚手段
use_kvcache由rtp_llm/config/model_config.py:912推导为task_type == LANGUAGE_MODEL，RopeStyle::No又是RopeConfig.h:22的默认值，因此该分支覆盖全部非 LM 任务（DENSE/SPARSE/COLBERT/BGE_M3/SEQ_CLASSIFICATION/RERANKER/LINEAR_SOFTMAX 等）下所有未显式设 rope 的模型，包含 Baichuan-13B（models/llama.py:153，ALiBi）、Cohere（llama.py:220）被部署为 embedding/reranker 的情形，远超标题声明的 vision bert。赋值为无条件覆盖：经 pybind 显式设 true 的配置（pybind/ConfigInit.cc:1657）被静默丢弃；to_string()（ModelConfig.cc:213）打印的是派生前成员，日志看不到实际生效值，也无 env/参数开关可回滚。 - [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue
新增派生分支缺少单测，且仓内 smoke 对该分支为零差异覆盖
本 PR 仅 5 行新增、无任何测试。rtp_llm/cpp/config/下不存在 test 目录，全仓无getAttentionConfigs单测。唯一覆盖 rope=No embedding 的 smoke 是rtp_llm/test/smoke/suites_rocm_oss.bzl:186-231（bert/roberta/reranker/classifier），但rtp_llm/models/bert.py:38早已把该字段置 false，这些用例前后走完全相同路径，属零差异验证；CUDA 侧唯一 embedding 用例suites_sm8x.bzl:65为 qwen2 gte（rope=Base），根本不进入新分支。该分支同时决定是否跳过 KV 写入（py_flashinfer_mha.py:821、rocm_impl/aiter.py:1610），条件写错只会静默产出错误结果，当前无任何断言可拦截。 - [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue
新增派生分支缺少单测，且仓内 smoke 对该分支为零差异覆盖
本 PR 仅 5 行新增、无任何测试。rtp_llm/cpp/config/下不存在 test 目录，全仓无getAttentionConfigs单测。唯一覆盖 rope=No embedding 的 smoke 是rtp_llm/test/smoke/suites_rocm_oss.bzl:186-231（bert/roberta/reranker/classifier），但rtp_llm/models/bert.py:38早已把该字段置 false，这些用例前后走完全相同路径，属零差异验证；CUDA 侧唯一 embedding 用例suites_sm8x.bzl:65为 qwen2 gte（rope=Base），根本不进入新分支。该分支同时决定是否跳过 KV 写入（py_flashinfer_mha.py:821、rocm_impl/aiter.py:1610），条件写错只会静默产出错误结果，当前无任何断言可拦截。 - [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue
新增派生分支缺少单测，且仓内 smoke 对该分支为零差异覆盖
本 PR 仅 5 行新增、无任何测试。rtp_llm/cpp/config/下不存在 test 目录，全仓无getAttentionConfigs单测。唯一覆盖 rope=No embedding 的 smoke 是rtp_llm/test/smoke/suites_rocm_oss.bzl:186-231（bert/roberta/reranker/classifier），但rtp_llm/models/bert.py:38早已把该字段置 false，这些用例前后走完全相同路径，属零差异验证；CUDA 侧唯一 embedding 用例suites_sm8x.bzl:65为 qwen2 gte（rope=Base），根本不进入新分支。该分支同时决定是否跳过 KV 写入（py_flashinfer_mha.py:821、rocm_impl/aiter.py:1610），条件写错只会静默产出错误结果，当前无任何断言可拦截。
RTP-LLM Checklist
- [I] 代码质量 — 同一功能用统一工具函数 → issue
同一决策存在三处独立编码，谓词重复、注释表述过宽并混入空行删除
config.rope_config.style == RopeStyle::No && !use_kvcache在 188 行与 191 行逐字重复，后续调整口径易漏改。同一决策另有两处编码：models/bert.py:38的模型级赋值（本 PR 未清理），以及cuda_impl/trt.py:377/397-401用自有运行时判据need_rope or kv_cache is not None跳过同一 fused op、完全不读该 flag。注释「Skip rope+bias+transpose kernel entirely」「PACKED_QKV layout matches FMHA input directly」只对 headwise/aiter/trtllm_gen 成立：py_flashinfer_mha.py:855/902在 style=No 时 rope_impl 本就是 None，该路径走_split_qkv，唯一变化是跳过 821 行的 KV 写入。diff 还删掉了 188 行后的空行，与函数内其他派生段落风格不一
Strengths
- 消除的是真实的 per-forward 浪费：
kv_cache为 None 时旧代码进入kv_cache_write_op.py:87-131，每层每次 forward 都torch.empty两个 dummy KV cache 加索引张量并启动一次写入即弃的append_paged_kv_cache，跳过后开销按num_layers倍数消失，也降低了 CUDA graph 捕获期的分配风险。 - bias 的正确性论证成立：
models_py/bindings/rocm/FusedRopeKVCacheOp.cc:400/590/622向 kernel 传入的qkv_bias恒为 nullptr，bias 一直由 qkv linear/GEMM epilogue 承担，跳过该 kernel 不会丢 bias。 - 复用既有约定而非新造路径：判定口径与相邻
fuse_qkv_add_bias（ModelConfig.cc:188）一致，且各后端已有对称的 false 分支（headwise.py:419、rocm_impl/aiter.py:1610、trtllm_gen.py:592、xqa.py:102），未引入新 kernel 代码路径。 - 安全默认成立：
use_kvcache在 C++ 侧默认 true（ModelConfig.h:120），条件同时要求 rope=No，不会误伤有 RoPE 的 embedding 模型，与rocm_impl/aiter.py:1600注释的约束方向一致。 - 爆炸半径受控：只修改
getAttentionConfigs()返回的副本，函数保持 const、无共享状态改写；C++ 引擎侧对该字段只有 debug 打印（AttentionConfig.cc:36），影响限制在 Python 模型执行层，且派生只在建 impl 阶段执行一次，无 per-forward 开销。
|
|
||
| // Skip rope+bias+transpose kernel entirely for embedding models (no RoPE, no KV cache). | ||
| // The GEMM epilogue already handles bias, and PACKED_QKV layout matches FMHA input directly. | ||
| if (config.rope_config.style == RopeStyle::No && !use_kvcache) { |
There was a problem hiding this comment.
[P1] 新增派生分支缺少单测，且仓内 smoke 对该分支为零差异覆盖
本 PR 仅 5 行新增、无任何测试。rtp_llm/cpp/config/ 下不存在 test 目录，全仓无 getAttentionConfigs 单测。唯一覆盖 rope=No embedding 的 smoke 是 rtp_llm/test/smoke/suites_rocm_oss.bzl:186-231（bert/roberta/reranker/classifier），但 rtp_llm/models/bert.py:38 早已把该字段置 false，这些用例前后走完全相同路径，属零差异验证；CUDA 侧唯一 embedding 用例 suites_sm8x.bzl:65 为 qwen2 gte（rope=Base），根本不进入新分支。该分支同时决定是否跳过 KV 写入（py_flashinfer_mha.py:821、rocm_impl/aiter.py:1610），条件写错只会静默产出错误结果，当前无任何断言可拦截。
建议: 补一个聚焦测试固定真值表：(rope=No, use_kvcache=false) → false；(rope=No, use_kvcache=true) → 保持入参；(rope=Base, use_kvcache=false) → 保持入参；入参显式为 true 时的覆盖行为也需断言。getAttentionConfigs 已在 pybind/ConfigInit.cc:2009 导出，写 py_test 成本最低，也可新建 rtp_llm/cpp/config/test/ 用 cc_test_wrapper。端到端侧建议在 CUDA suite 补一条 bert/roberta embedding 用例（golden 资产 data/model/bert/expect.pt、roberta_expect.pt 已存在，目前仅被 ROCm 消费），或在 PR 描述中说明目标模型在哪条用例/平台做过前后对比及耗时数据。
Checklist: [6.1] 分布式/跨平台变更有对应覆盖；[6.1] 新逻辑有聚焦单测 + 相关集成/smoke 测试；[6.1] 边界 case 覆盖（空、单元素、最大值）
|
|
||
| // Skip rope+bias+transpose kernel entirely for embedding models (no RoPE, no KV cache). | ||
| // The GEMM epilogue already handles bias, and PACKED_QKV layout matches FMHA input directly. | ||
| if (config.rope_config.style == RopeStyle::No && !use_kvcache) { |
There was a problem hiding this comment.
[P2] 判定条件与注释宣称的不变量不等价，use_logn_attn 的 q 缩放会被静默丢弃
被跳过的 fused op 并非只做 RoPE：models_py/bindings/rocm/kernels/fused_rope_kvcache_kernel.cu:262/897/1239 在 use_logn_attn 为真时对 q 施加 logn 缩放，而该开关由 config.json 直接读入（models/llama.py:123、models/qwen.py:296），与 rope style 无耦合。另 models/deepseek_v2.py:635-636 在 mla_ops_type != MHA 时把 style 置 0，此处 No 表示「rope 由 MLA 算子内部完成」而非「无 rope」。这两类配置一旦以非 LANGUAGE_MODEL 任务部署即命中新分支，会被静默降级且无告警（目前 MLA impl 未读该字段，MLA 一侧属防御性加固）。
建议: 把条件收紧为显式表达前提，例如追加 && !config.use_logn_attn && !config.use_mla（两字段均在 AttentionConfig.h:38/42），或对 use_logn_attn && style == No && !use_kvcache 这类矛盾组合 fail-fast 抛错，避免精度问题只能靠离线比对发现；同时把注释改为描述实际条件（「无通用 RoPE kernel 且无 KV cache」），不要让读者按更强的不变量去信任该分支。
| // Skip rope+bias+transpose kernel entirely for embedding models (no RoPE, no KV cache). | ||
| // The GEMM epilogue already handles bias, and PACKED_QKV layout matches FMHA input directly. | ||
| if (config.rope_config.style == RopeStyle::No && !use_kvcache) { | ||
| config.need_rope_kv_cache = false; |
There was a problem hiding this comment.
[P2] 中心派生无条件覆盖模型侧显式配置，生效面大于注释所述且无日志与回滚手段
use_kvcache 由 rtp_llm/config/model_config.py:912 推导为 task_type == LANGUAGE_MODEL，RopeStyle::No 又是 RopeConfig.h:22 的默认值，因此该分支覆盖全部非 LM 任务（DENSE/SPARSE/COLBERT/BGE_M3/SEQ_CLASSIFICATION/RERANKER/LINEAR_SOFTMAX 等）下所有未显式设 rope 的模型，包含 Baichuan-13B（models/llama.py:153，ALiBi）、Cohere（llama.py:220）被部署为 embedding/reranker 的情形，远超标题声明的 vision bert。赋值为无条件覆盖：经 pybind 显式设 true 的配置（pybind/ConfigInit.cc:1657）被静默丢弃；to_string()（ModelConfig.cc:213）打印的是派生前成员，日志看不到实际生效值，也无 env/参数开关可回滚。
建议: 三选一收口：(1) 优先使用已有本地扩展点，在目标模型 _create_config 中设置（与 models/bert.py:38 一致），中心逻辑保持不动；(2) 保留「显式设置优先」语义，仅在字段仍为默认值时才派生；(3) 若坚持中心化，请在 PR 描述中列出受影响 task_type 与模型清单，并在覆盖生效时打一条一次性 INFO 日志（含 rope style、use_kvcache、覆盖前后取值，放初始化路径而非 per-forward），必要时提供一个可关闭该优化的开关作为运维回滚手段。同时把注释中的 "embedding models" 改为准确表述（非 LANGUAGE_MODEL 任务且无 RoPE）。
Checklist: [6.1] 兼容性：外部 HTTP/RPC API、持久数据、配置、环境迁移安全；[6.1] 可观测性：日志/指标/超时可操作、非噪声；[6.1] 回滚路径：风险行为存在运维回滚手段；[6.1] OCP：本地扩展点优先于修改中心逻辑
| @@ -186,7 +186,11 @@ AttentionConfigs ModelConfig::getAttentionConfigs(int64_t tp_size) const { | |||
|
|
|||
| // if qk_norm or use embedding model, fuse add bias in gemm | |||
| config.fuse_qkv_add_bias = qk_norm || (config.rope_config.style == RopeStyle::No && !use_kvcache) ? false : true; | |||
There was a problem hiding this comment.
[P3] 同一决策存在三处独立编码，谓词重复、注释表述过宽并混入空行删除
config.rope_config.style == RopeStyle::No && !use_kvcache 在 188 行与 191 行逐字重复，后续调整口径易漏改。同一决策另有两处编码：models/bert.py:38 的模型级赋值（本 PR 未清理），以及 cuda_impl/trt.py:377/397-401 用自有运行时判据 need_rope or kv_cache is not None 跳过同一 fused op、完全不读该 flag。注释「Skip rope+bias+transpose kernel entirely」「PACKED_QKV layout matches FMHA input directly」只对 headwise/aiter/trtllm_gen 成立：py_flashinfer_mha.py:855/902 在 style=No 时 rope_impl 本就是 None，该路径走 _split_qkv，唯一变化是跳过 821 行的 KV 写入。diff 还删掉了 188 行后的空行，与函数内其他派生段落风...
建议: 提取局部常量（如 const bool no_rope_no_kvcache = config.rope_config.style == RopeStyle::No && !use_kvcache;）供 188/191 两处复用；注释改为按后端限定表述；恢复被删除的空行以保持分段风格。真源建议二选一收敛：以 need_rope_kv_cache 为唯一权威来源（删除 bert.py:38，并让 trt.py 改读该字段），或在 AttentionConfig.h:52 注明该字段的权威来源与各后端遵循情况（明确 trt.py 使用独立判据）。
Checklist: [6.1] 逻辑变更未混入无关格式化；[6.1] DRY：重复非平凡逻辑被抽取或显式复用；[I] 同一功能用统一工具函数
| // Skip rope+bias+transpose kernel entirely for embedding models (no RoPE, no KV cache). | ||
| // The GEMM epilogue already handles bias, and PACKED_QKV layout matches FMHA input directly. | ||
| if (config.rope_config.style == RopeStyle::No && !use_kvcache) { | ||
| config.need_rope_kv_cache = false; |
There was a problem hiding this comment.
[P3] flag 语义过载，跳过 KV 写入依赖未被断言保护的隐式不变量
need_rope_kv_cache=false 在下游不只跳过 rope 融合 kernel，还同时跳过 KV cache 写入：py_flashinfer_mha.py:821-822 不再调用 kv_cache_write_op，而 PyFlashinferPagedPrefillImpl._prepare_fmha_input（同文件 859-863）只回传 (query,)，K/V 必须从 paged cache 读出；rocm_impl/aiter.py:1610-1613 同理直接 fmha_input = qkv 且不写 cache。当前不出错仅因新条件含 !use_kvcache，从而保证 kv_cache is None（models_py/model_desc/bert.py:153）。一旦后续出现 use_kvcache=false 但仍挂载 cache 的组合，attention 会静默读到未写入的 KV，属无报错的错误输出。
建议: 在 AttentionConfig.h:52 补一行注释说明该字段同时决定是否写 KV cache，避免后续调用方误用；或在读取该 flag 的 impl 构造处对 need_rope_kv_cache == False and kv_cache is not None 做显式 assert/raise，把当前的隐式不变量固化为 fail-fast 检查。
Checklist: [6.1] 状态不变量：创建/更新/失败/重试/回滚路径有效；[6.1] 错误语义：fail-fast/retry/fallback/silent 行为显式
LLLLKKKK
left a comment
LLLLKKKK
left a comment
There was a problem hiding this comment.
AI Code Review - PR #1361
Status: LGTM
Summary: P0/0 · P1/0 · P2/2 · P3/0
Reviewed: commit 9a84037bee09 · 2026-09-01 11:46 UTC+8
lgtm ready to ci
Non-blocking Suggestions
P2
- 判定条件与注释宣称的不变量不等价，use_logn_attn 的 Q 缩放会被静默丢弃 @
rtp_llm/cpp/config/ModelConfig.cc:191- 建议：仅在
!config.use_logn_attn时关闭该标志，或将 logn 缩放拆为独立步骤，并增加该组合的回归测试。
- 建议：仅在
- 新增 BERT smoke 未覆盖配置覆盖分支 @
rtp_llm/test/smoke/suites_sm120.bzl:44- 建议：增加以初始值 true 进入
getAttentionConfigs的端到端用例，确保回退该分支时测试失败。
- 建议：增加以初始值 true 进入
Checklist Findings (2 fail / 81 total)
General Principles Checklist
- [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue
判定条件与注释宣称的不变量不等价，use_logn_attn 的 Q 缩放会被静默丢弃
当RopeStyle::No、use_kvcache=false、use_logn_attn=true且初始标志为 true 时，此处分支仍强制关闭预处理。Aiter 的无缓存 prefill 随后直接传递 QKV，绕过 fused kernel 中独立执行的logn_attention，长序列 Q 缩放及输出语义因此改变。 - [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue
新增 BERT smoke 未覆盖配置覆盖分支
新逻辑仅在初始need_rope_kv_cache=true、无 RoPE 且禁用 KV cache 时改变结果；该 smoke 使用的Bert._create_config已提前将该字段设为 false，因此删除本次派生分支后 smoke 仍会通过。
Strengths
- 真值表覆盖 8 种核心组合，并验证源配置保持不变。
- 单测经过真实 pybind 边界，BUILD 依赖与 SM120 smoke 注册正确。
|
|
||
| // Skip rope+bias+transpose kernel entirely for embedding models (no RoPE, no KV cache). | ||
| // The GEMM epilogue already handles bias, and PACKED_QKV layout matches FMHA input directly. | ||
| if (config.rope_config.style == RopeStyle::No && !use_kvcache) { |
There was a problem hiding this comment.
[P2] 判定条件与注释宣称的不变量不等价，use_logn_attn 的 Q 缩放会被静默丢弃
当 RopeStyle::No、use_kvcache=false、use_logn_attn=true 且初始标志为 true 时，此处分支仍强制关闭预处理。Aiter 的无缓存 prefill 随后直接传递 QKV，绕过 fused kernel 中独立执行的 logn_attention，长序列 Q 缩放及输出语义因此改变。
建议: 仅在 !config.use_logn_attn 时关闭该标志，或将 logn 缩放拆为独立步骤，并增加该组合的回归测试。
Checklist: [6.1] 状态不变量：创建/更新/失败/重试/回滚路径有效
| gpu_type = ["RTX_5000_PRO"], | ||
| ), | ||
| smoke_test( | ||
| name = "embedding_bert_sm120", |
There was a problem hiding this comment.
[P2] 新增 BERT smoke 未覆盖配置覆盖分支
新逻辑仅在初始 need_rope_kv_cache=true、无 RoPE 且禁用 KV cache 时改变结果；该 smoke 使用的 Bert._create_config 已提前将该字段设为 false，因此删除本次派生分支后 smoke 仍会通过。
建议: 增加以初始值 true 进入 getAttentionConfigs 的端到端用例，确保回退该分支时测试失败。
Checklist: [6.1] 新逻辑有聚焦单测 + 相关集成/smoke 测试
3 participants
…on pro5000