feat(vpto): normalize address recurrences before soft post-update - #1244
feat(vpto): normalize address recurrences before soft post-update#1244yexiaosu wants to merge 9 commits into
Conversation
a017536 to
eb21dcd
Compare
mouliangyu
left a comment
There was a problem hiding this comment.
Review Findings\n\n### [P2] i16 recurrence 标准化的实现复杂度需要重新评估\n\n位置: \n\n独立的 normalizer pass 是合理的,但当前 i16 标准化引入了 shadow 、完整 重建、、后续 commit/rollback,以及 shadow/witness 清理。\n\n仓库已有 采用“先证明范围,成功后一次性改写,失败保持原 IR”的方式。当前 PR 还需要说明哪些 、signed/unsigned、多 use 场景必须依赖这套 shadow/witness 协议,以及为什么更简单的单次改写模式不能覆盖它们。\n\n这不一定是 correctness bug,但当前实现复杂度明显高于现有窄化 pass。建议在设计文档中补充覆盖场景和必要性,或缩小标准化改写范围。\n\n### [P2] ObjectEmission 的产物校验改动与本 PR 主题耦合不足\n\n位置: \n\n除 Bisheng post-update 开关外,本 PR 还加入了:\n\n- 删除预创建输出文件;\n- 非空文件检查;\n- device object/可执行 section 检查;\n- 检查;\n- 嵌入 device object 解析。\n\n这些属于独立的编译产物完整性校验,和地址递推标准化没有直接关系。建议拆成独立 commit/PR,或补充明确证据说明:\n\n- 哪个实际失败场景需要这些检查;\n- 的格式契约;\n- 各 CANN/Bisheng 版本兼容性;\n- 对应失败路径测试。\n\n另外, 和 仍主要依赖编译器返回码,而 LLVM IR 路径增加了更严格的 object 检查,校验策略并不一致。\n\n### [P2] 编译前删除输出文件的失败语义需要明确\n\n位置: \n\n 会在编译前删除 。如果调用者传入的路径不是始终由 新建的临时文件,那么编译失败会先丢失原有产物。\n\n请证明所有调用点都满足“输出路径必然是新建临时文件”;否则应改为输出到临时路径,校验成功后再替换目标文件。\n\n### [P2] changed-code 合规检查未通过\n\n对 PR 相对 merge-base 执行:\n\n\n\n结果:\n\n\n\n主要是 :新增或修改的控制语句缺少花括号,涉及:\n\n- \n- \n- \n- \n- \n\n这些属于当前主线的 changed-code 门禁,应在合并前修复;其余转换安全、 和 warning 也应逐项确认。\n\n## Non-blocking Suggestion\n\n 的 post-update op 共享表本身是合理的,因为两个 pass 都需要 base/stride operand 位置、地址单位、signedness 和 stride constraint。但建议在设计文档中明确区分:\n\n\n\n当前没有确认到 i16 recurrence 数学证明的具体错误,也没有发现 嵌套问题。\n\n总体建议:先让作者证明 i16 shadow/witness 方案的必要性,拆分或补齐 ObjectEmission 改动的证据与测试,并修复 changed-code 合规错误。
mouliangyu
left a comment
There was a problem hiding this comment.
Review Findings
[P2] i16 recurrence 标准化的实现复杂度需要重新评估
位置: lib/PTO/Transforms/VPTONormalizeAddressRecurrences.cpp
独立的 normalizer pass 是合理的,但当前 i16 标准化引入了 shadow iter_arg、完整 scf.for 重建、AddressRecurrenceWitnessOp、后续 commit/rollback,以及 shadow/witness 清理。
仓库已有 lib/PTO/Transforms/PTONarrowVPTOLoopCounters.cpp 采用“先证明范围,成功后一次性改写,失败保持原 IR”的方式。当前 PR 还需要说明哪些 iter_arg、signed/unsigned、多 use 场景必须依赖这套 shadow/witness 协议,以及为什么更简单的单次改写模式不能覆盖它们。
这不一定是 correctness bug,但当前实现复杂度明显高于现有窄化 pass。建议在设计文档中补充覆盖场景和必要性,或缩小标准化改写范围。
[P2] ObjectEmission 的产物校验改动与本 PR 主题耦合不足
位置: tools/ptoas/ObjectEmission.cpp
除 Bisheng post-update 开关外,本 PR 还加入了删除预创建输出文件、非空文件检查、device object/可执行 section 检查、__aicore_rel_binary 检查和嵌入 device object 解析。这些属于独立的编译产物完整性校验,和地址递推标准化没有直接关系。
建议拆成独立 commit/PR,或补充明确证据说明实际失败场景、__aicore_rel_binary 的格式契约、各 CANN/Bisheng 版本兼容性及失败路径测试。compileCppDeviceSourceToObject() 和 compileCppDeviceSourceToFatobj() 仍主要依赖编译器返回码,而 LLVM IR 路径增加了更严格的 object 检查,校验策略并不一致。
[P2] 编译前删除输出文件的失败语义需要明确
位置: tools/ptoas/ObjectEmission.cpp
removePrecreatedOutputFile() 会在编译前删除 outObjPath。如果调用者传入的路径不是始终由 TempFileRegistry 新建的临时文件,那么编译失败会先丢失原有产物。
请证明所有调用点都满足“输出路径必然是新建临时文件”;否则应改为输出到临时路径,校验成功后再替换目标文件。
[P2] changed-code 合规检查未通过
对 PR 相对 merge-base f71c72fe 执行:
python3 .codex/skills/enforce-ptoas-code-compliance/scripts/check_changed_code.py \
--repo . --base f71c72fe
结果:
checked_files=33 errors=292 warnings=6
主要是 G.FMT.11-CPP:新增或修改的控制语句缺少花括号,涉及 VPTONormalizeAddressRecurrences.cpp、VPTOPostUpdateUtils.cpp、VPTOSoftPostUpdate.cpp、ObjectEmission.cpp 和 test/vpto/npu_validation/common/two_buffer_main.h。这些属于当前主线的 changed-code 门禁,应在合并前修复;其余转换安全、atoi 和 goto warning 也应逐项确认。
Non-blocking Suggestion
VPTOPostUpdateUtils 的 post-update op 共享表本身是合理的,因为两个 pass 都需要 base/stride operand 位置、地址单位、signedness 和 stride constraint。但建议在设计文档中明确区分通用 recurrence 信息(初值、步长、trip count、wrap proof)和 post-update 专用信息(operand 位置、地址单位、最终 stride 限制、result 形式)。
当前没有确认到 i16 recurrence 数学证明的具体错误,也没有发现 vecscope 嵌套问题。
总体建议:先让作者证明 i16 shadow/witness 方案的必要性,拆分或补齐 ObjectEmission 改动的证据与测试,并修复 changed-code 合规错误。
mouliangyu
left a comment
There was a problem hiding this comment.
Review Findings
[P2] i16 recurrence 标准化的实现复杂度需要重新评估
位置: lib/PTO/Transforms/VPTONormalizeAddressRecurrences.cpp
独立的 normalizer pass 是合理的,但当前 i16 标准化引入了 shadow iter_arg、完整 scf.for 重建、AddressRecurrenceWitnessOp、后续 commit/rollback,以及 shadow/witness 清理。
仓库已有 lib/PTO/Transforms/PTONarrowVPTOLoopCounters.cpp 采用“先证明范围,成功后一次性改写,失败保持原 IR”的方式。当前 PR 还需要说明哪些 iter_arg、signed/unsigned、多 use 场景必须依赖这套 shadow/witness 协议,以及为什么更简单的单次改写模式不能覆盖它们。
这不一定是 correctness bug,但当前实现复杂度明显高于现有窄化 pass。建议在设计文档中补充覆盖场景和必要性,或缩小标准化改写范围。
[P2] ObjectEmission 的产物校验改动与本 PR 主题耦合不足
位置: tools/ptoas/ObjectEmission.cpp
除 Bisheng post-update 开关外,本 PR 还加入了删除预创建输出文件、非空文件检查、device object/可执行 section 检查、__aicore_rel_binary 检查和嵌入 device object 解析。这些属于独立的编译产物完整性校验,和地址递推标准化没有直接关系。
建议拆成独立 commit/PR,或补充明确证据说明实际失败场景、__aicore_rel_binary 的格式契约、各 CANN/Bisheng 版本兼容性及失败路径测试。compileCppDeviceSourceToObject() 和 compileCppDeviceSourceToFatobj() 仍主要依赖编译器返回码,而 LLVM IR 路径增加了更严格的 object 检查,校验策略并不一致。
[P2] 编译前删除输出文件的失败语义需要明确
位置: tools/ptoas/ObjectEmission.cpp
removePrecreatedOutputFile() 会在编译前删除 outObjPath。如果调用者传入的路径不是始终由 TempFileRegistry 新建的临时文件,那么编译失败会先丢失原有产物。
请证明所有调用点都满足“输出路径必然是新建临时文件”;否则应改为输出到临时路径,校验成功后再替换目标文件。
[P2] changed-code 合规检查未通过
对 PR 相对 merge-base f71c72fe 执行:
python3 .codex/skills/enforce-ptoas-code-compliance/scripts/check_changed_code.py \
--repo . --base f71c72fe
结果:
checked_files=33 errors=292 warnings=6
主要是 G.FMT.11-CPP:新增或修改的控制语句缺少花括号,涉及 VPTONormalizeAddressRecurrences.cpp、VPTOPostUpdateUtils.cpp、VPTOSoftPostUpdate.cpp、ObjectEmission.cpp 和 test/vpto/npu_validation/common/two_buffer_main.h。这些属于当前主线的 changed-code 门禁,应在合并前修复;其余转换安全、atoi 和 goto warning 也应逐项确认。
Non-blocking Suggestion
VPTOPostUpdateUtils 的 post-update op 共享表本身是合理的,因为两个 pass 都需要 base/stride operand 位置、地址单位、signedness 和 stride constraint。但建议在设计文档中明确区分通用 recurrence 信息(初值、步长、trip count、wrap proof)和 post-update 专用信息(operand 位置、地址单位、最终 stride 限制、result 形式)。
当前没有确认到 i16 recurrence 数学证明的具体错误,也没有发现 vecscope 嵌套问题。
总体建议:先让作者证明 i16 shadow/witness 方案的必要性,拆分或补齐 ObjectEmission 改动的证据与测试,并修复 changed-code 合规错误。
Summary
This PR adds
VPTONormalizeAddressRecurrencesPassbeforeVPTOSoftPostUpdatePassto safely normalize loop-varying VPTO address recurrences into an i16 recurrence domain.It also:
Motivation
Soft post-update needs to derive the per-iteration change of the complete effective address. Existing VPTO address operands may come from
index,i32, ori16induction variables and fixed-stepscf.foriter_args, potentially passing through signed or unsigned casts.A widening cast does not by itself prove that the loop delta is preserved. If the source recurrence wraps before it is widened, interpreting it as a mathematical recurrence in the wider address domain can generate an incorrect post-update pointer chain. This issue was identified by the review discussion in PTOAS PR #1018.
The new pass separates two responsibilities:
VPTONormalizeAddressRecurrencesPassproves that an address leaf can be represented safely as a canonical i16 recurrence.VPTOSoftPostUpdatePassanalyzes the complete effective-address delta, performs address-unit conversion, checks the final stride constraints, and commits the post-update rewrite.Implementation
Address recurrence normalization
The new pass recognizes address-related direct induction variables and fixed-step iter_args used by:
pto.addptr;For
index,i32, andi16recurrences, the proof covers:A proven recurrence is represented by an i16 shadow iter_arg with the matching
nswornuwoverflow flag. It is extended back to the original operand type when necessary.Unsupported, complex, wrapping, or externally used recurrences remain unchanged.
Reversible handoff
Normalization alone cannot guarantee that the complete memory operation can use post-update form. The final combined stride may still fail because of unit conversion, operand typing,
ConstantorSignedI8constraints, or operation-specific rules.The pass therefore uses
pto.address_recurrence_witnessto retain both the original value and the canonical value.VPTOSoftPostUpdatePass:This supports mixed commit and rollback within the same loop without leaving normalization overhead when post-update fails.
Shared operation descriptions
VPTOPostUpdateUtilsnow owns the complete candidate table used by both passes. Each entry describes:Element,Block,Byte, orAlignmentaddress unit;Dynamic,Constant, orSignedI8final stride constraint.The table currently covers all 14 soft post-update candidates, including:
vldus, which has no explicit ordinary-form stride;vstus, whose offset advances the returned state but does not participate in the current access address;vsldbandvsstb.Pass scope and pipeline
Both passes operate only inside
pto.vecscope, preventing normalization witnesses from being created outside the consumer's processing boundary.The VPTO pipeline now runs:
VPTO soft post-update is enabled by default and can be disabled with:
Disabling it skips both normalization and soft post-update.
Bisheng post-update controls
Because PTOAS performs this optimization before LLVM lowering, Bisheng LLVM-level auto post-update is disabled by default using its supported public options:
For diagnostic and comparison purposes,
--enable-bisheng-soft-postupdatechanges both options totrue.Device compilation now also rejects empty or invalid intermediate objects and verifies that the embedded AICore ELF contains executable content, preventing a compiler return code of zero from producing a silently empty fat object.
Testing
Lit coverage includes:
index,i32, andi16operands;Element,Block,Byte, andAlignmentaddress units;vldusandvstus;ConstantandSignedI8final stride constraints;pto.vecscopeownership boundaries;Full lit result:
CA-model smoke validation passed for:
micro-op/binary-vector/vaddmicro-op/vector-load-store/vlds-post-updateAll 14 targeted soft post-update SIM cases passed with:
The validated cases cover address units, combined base/offset deltas,
vldus/vstus, alignment addressing, wrap regressions, normalized operand types, mixed rollback, descending recurrences, nested/shared chains, block load/store, scalar/predicate stores, and predicate post-update.