Skip to content

[CINN] Disable unused pool2d symbolic infer path#78658

Open
ooooo-create wants to merge 1 commit intoPaddlePaddle:developfrom
ooooo-create:codex/remove-raw-cc-include-78417
Open

[CINN] Disable unused pool2d symbolic infer path#78658
ooooo-create wants to merge 1 commit intoPaddlePaddle:developfrom
ooooo-create:codex/remove-raw-cc-include-78417

Conversation

@ooooo-create
Copy link
Copy Markdown
Contributor

@ooooo-create ooooo-create commented Apr 13, 2026

PR Category

CINN

PR Types

Improvements

Description

该 PR 延续 #78417 的思路,继续收敛 cinn_op_infer_sym.cc 对 pooling 符号推导实现的源码级耦合。

本次排查后确认:
paddle/cinn/hlir/dialect/operator/transforms/pd_to_cinn_pass.cc 中虽然定义了 Pool2dOpPattern,但在 PdOpToCinnOpPass::InitializePatterns() 里并没有注册这个 pattern。也就是说,当前 pd_to_cinn_pass 并不会把 pd_op.pool2d 改写成 cinn_op.pool2d,因此 cinn::dialect::Pool2dOpInferSymbolicShape 这条路径当前并不在实际 pass 链路上。

基于这个现状,这个 PR 做了下面几件事:

  • paddle/fluid/pir/dialect/operator/interface/infer_symbolic_shape/cinn_op_infer_sym.cc 中注释掉 unary_infer_sym.cc 的 raw include。
  • paddle/fluid/pir/dialect/operator/interface/infer_symbolic_shape/cinn_op_infer_sym.h/.cc 中注释掉 Pool2dOpInferSymbolicShape 的声明和实现,避免保留一条当前不会被命中的 CINN pool2d symbolic infer 路径。
  • paddle/fluid/pir/dialect/operator/interface/infer_symbolic_shape/unary_infer_sym.cc 中顺手做了小范围清理:GetRealPadding 改为 const ref 传参,并把 UpdataPadding 更名为 UpdatePadding

这次改动的目标不是修改 pool2d 的符号推导规则,而是把 cinn_op_infer_sym 的依赖关系和实际 pass 路径对齐,避免保留一条未注册 pass 的死路径。

是否引起精度变化

Copilot AI review requested due to automatic review settings April 13, 2026 07:29
@paddle-bot
Copy link
Copy Markdown

paddle-bot bot commented Apr 13, 2026

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes a raw .cc inclusion that was aggregating unary symbolic-shape inference into cinn_op_infer_sym.o, which could misattribute incremental coverage to the wrong object file.

Changes:

  • Replace the source-level include of unary_infer_sym.cc with the proper header include unary_infer_sym.h.
  • Make cinn::dialect::Pool2dOpInferSymbolicShape delegate directly to paddle::dialect::Pool2dOpInferSymbolicShape to avoid relying on internal helpers from the included .cc.
  • Minor cleanup in GetRealPadding: fix lambda name typo and avoid unnecessary copies by using const& parameters.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
paddle/fluid/pir/dialect/operator/interface/infer_symbolic_shape/unary_infer_sym.cc Minor parameter/lambda cleanup in pooling padding inference helpers.
paddle/fluid/pir/dialect/operator/interface/infer_symbolic_shape/cinn_op_infer_sym.cc Remove raw .cc inclusion; reuse Paddle Pool2d symbolic-shape inference to decouple object files and coverage attribution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ooooo-create
Copy link
Copy Markdown
Contributor Author

目前经常需要开发者自己来证明这个文件的代码确实覆盖到了,例如: #78441 (comment)

@ooooo-create ooooo-create requested a review from SigureMo April 13, 2026 08:35
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (develop@212a3f6). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...nterface/infer_symbolic_shape/cinn_op_infer_sym.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             develop   #78658   +/-   ##
==========================================
  Coverage           ?   66.66%           
==========================================
  Files              ?        2           
  Lines              ?        3           
  Branches           ?        0           
==========================================
  Hits               ?        2           
  Misses             ?        1           
  Partials           ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Apr 14, 2026
@SigureMo
Copy link
Copy Markdown
Member

merge 下 develop 吧,覆盖率不够是没单测吗?

@ooooo-create ooooo-create marked this pull request as draft April 14, 2026 12:48
@ooooo-create ooooo-create force-pushed the codex/remove-raw-cc-include-78417 branch 2 times, most recently from 209cc92 to 0958eaf Compare April 14, 2026 14:47
@ooooo-create ooooo-create changed the title [CINN] Remove raw unary infer sym cc include [CINN] Disable unused pool2d symbolic infer path Apr 14, 2026
@ooooo-create ooooo-create force-pushed the codex/remove-raw-cc-include-78417 branch from 0958eaf to fa559d6 Compare April 14, 2026 14:47
@ooooo-create ooooo-create marked this pull request as ready for review April 14, 2026 14:48
Pool2dRawInferSymbolicShape(op, kernel_size, infer_context));
return true;
}
// bool Pool2dOpInferSymbolicShape(pir::Operation *op,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个地方的逻辑本身就写的不对,所以直接注释了先

@ooooo-create
Copy link
Copy Markdown
Contributor Author

merge 下 develop 吧,覆盖率不够是没单测吗?

pd_to_cinn_pass 里面没有注册这个转换,这个地方跑不到,原来的实现也不对😭

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

Labels

contributor External developers HappyOpenSource 快乐开源活动issue与PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants