Skip to content

Enable op fusion by default on A5#860

Draft
Zhendong404 wants to merge 1 commit into
hw-native-sys:mainfrom
Zhendong404:enable-fusion-default
Draft

Enable op fusion by default on A5#860
Zhendong404 wants to merge 1 commit into
hw-native-sys:mainfrom
Zhendong404:enable-fusion-default

Conversation

@Zhendong404

Copy link
Copy Markdown
Contributor

Summary

  • make --enable-op-fusion default to enabled on A5 and disabled on A3
  • reject --enable-op-fusion=true on A3 instead of silently ignoring it
  • keep --enable-op-fusion=false as the explicit opt-out on A5 and update docs/tests

Validation

  • CCACHE_DISABLE=1 ninja -C build ptoas
  • build/tools/ptoas/ptoas --pto-arch=a5 --pto-level=level2 --emit-pto-ir test/lit/tile_fusion/op_fusion_nonfused_control.pto -o -
  • build/tools/ptoas/ptoas --pto-arch=a5 --pto-level=level2 --enable-op-fusion=false --emit-pto-ir test/lit/tile_fusion/op_fusion_nonfused_control.pto -o -
  • build/tools/ptoas/ptoas --pto-arch=a5 --pto-level=level1 --enable-op-fusion=true test/lit/tile_fusion/op_fusion_cli_flags.pto -o /dev/null
  • build/tools/ptoas/ptoas --pto-arch=a3 --enable-op-fusion=true test/lit/tile_fusion/op_fusion_cli_flags.pto -o /dev/null

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@reedhecre

reedhecre commented Jun 24, 2026

Copy link
Copy Markdown

Codex Review

该评论由 review 机器人自动更新。

  • PR: Enable op fusion by default on A5 #860 Enable op fusion by default on A5
  • Author: Zhendong404
  • Base/Head: main / enable-fusion-default
  • Head SHA: c60bf0892526
  • Trigger: 检测到新的 open PR
  • Generated At: 2026-06-24T06:28:16Z
  • Status: completed

Summary

PR #860 introduces a P1 regression: the new default-on A5 fusion path is only partially enabled for VPTO, so fused IR can be produced without the required post-lowering lifecycle passes.

Findings

  1. P1 Default A5 fusion does not enable the VPTO post-lowering fusion lifecycle tools/ptoas/ptoas.cpp:1488

The PR changes the frontend gate to enable fusion by default on A5 via opFusionEnabled in compilePTOASModule, but the VPTO post-lowering lifecycle still keys off the raw CLI option object: enableA5VPTOPostLoweringFusionLifecycle = enableOpFusion && moduleArchAttr && moduleArchAttr.getValue() == "a5". With the new tri-state flag, the default A5 path leaves enableOpFusion unset, so FusionPlan/OpScheduling/PTOFusionRegionGen now run by default, but PTOLowLevelLoopFusion, predicate/load-store elision, and flattening are skipped. That contradicts the documented contract in this PR and leaves pto.fusion_region in the VPTO path without the cleanup pipeline that previously made fused IR consumable, which is a high-risk correctness/runtime regression for the new default mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants