[CINN] Fixed dynamic arange symbolic values extraction. #74412
Conversation
| 你的PR提交成功，感谢你对开源项目的贡献! |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in CINN where cinn_op.generate_shape operations produce new symbolic shapes that aren't properly replaced with actual symbols during lowering. The issue manifests in complex op fusion scenarios where temporary symbols (SS0, SS1) are created but not replaced with the real symbols (S41, S42), causing LLVM compilation failures.
Key changes:
- Added
ReplaceGenerateShapeAttributefunction to replace temporary symbols with actual ones - Added detection for groups containing both
cinn_op.arangeandcinn_op.generate_shapeoperations - Implemented attribute replacement logic in the lowering process
Comments suppressed due to low confidence (2)
paddle/cinn/hlir/framework/pir/op_lowering_impl.cc:524
- The parameter name 'unsafe_op' is misleading. Consider renaming it to 'op' or 'generate_shape_op' since the function specifically handles generate_shape operations.
::pir::Operation* unsafe_op) { paddle/cinn/hlir/framework/pir/op_lowering_impl.cc:605
- The variable name 'unsafe_op_' with trailing underscore is inconsistent with typical naming conventions. Consider using 'mutable_op' or simply 'op_mut' to better indicate its purpose.
auto unsafe_op_ = const_cast<::pir::Operation*>(op); | // 'output_dim_exprs' attribute is used almost nowhere but symbolic arange, it | ||
| // will be fine to use a unsafe const_cast. It will actually be better, if we | ||
| // can trace verify whether this `cinn_op.generate_shape` is the input to the | ||
| // arange op. |
There was a problem hiding this comment.
The comment explanation is unclear and contains grammatical issues. Consider rewriting: 'The operation parameter is made mutable via const_cast to modify the output_dim_exprs attribute. This is safe because this attribute is primarily used for symbolic arange operations.'
| // arange op. | |
| // The unsafe_op parameter is made mutable via const_cast to allow modification | |
| // of the 'output_dim_exprs' attribute. This is considered safe because this | |
| // attribute is primarily used for symbolic arange operations. For improved | |
| // safety, it would be better to verify that 'cinn_op.generate_shape' is used | |
| // as input to an arange operation before applying this modification. |
| "The size of dim expr vector should not be empty.")); | ||
| // TODO(heqianyue): more secure impl is to check whether the old DimExpr has | ||
| // the same structure as the updated one. We can use a binary tree comparison | ||
| // algorithm. Yet normally we will be fine without checking. |
There was a problem hiding this comment.
The TODO comment contains grammatical errors. Consider: 'TODO(heqianyue): A more secure implementation would check whether the old DimExpr has the same structure as the updated one. We could use a binary tree comparison algorithm, though normally we should be fine without this check.'
| // algorithm. Yet normally we will be fine without checking. | |
| // TODO(heqianyue): A more secure implementation would check whether the old DimExpr | |
| // has the same structure as the updated one. We could use a binary tree comparison | |
| // algorithm, though normally we should be fine without this check. |
Enigmatisms changed the title 
| /re-run all-failed |
| /re-run Model-Benchmark |
| /re-run all-failed |
…#74412) * [CINN] Fix cinn_op.generate_op attribute storing useless dim_expr * [CINN] Removed unnecessary VLOGs * [CINN] Simplify dynamic arange logic and fix bugs.
…#74412) * [CINN] Fix cinn_op.generate_op attribute storing useless dim_expr * [CINN] Removed unnecessary VLOGs * [CINN] Simplify dynamic arange logic and fix bugs.
2 participants
PR Category
CINN
PR Types
Bug fixes
Description
修复了 与arange dynamic shape 支持 PR：
所进一步暴露的问题。问题的表现形式如下：
在一些非常复杂的 op fusion 过程中，generate shape 会生成新的 shape symbol，这些 symbol 会被直接记录在
cinn_op.generate_shape的 attribute（output_dim_exprs）中。而记录的output_dim_exprs其中的 symbol 在后续应该被替换为实际的 symbol，但 CINN 中并没有这么做，比如：Expr: Mul(SS0, SS1, 4)SS0 / SS1 这些是在 Lowering 过程中产生的，但实际代表的是实际参与 shape 计算的 symbol（实际存在的 symbol）：
本 PR 利用 GetTensor 时向 Tensor Value 内部填入的 ShapeOrData 的 Data 数据，替换了 tensor->operation 中较为复杂和隐晦的 operation attribute（以及一系列的 attribute from/to DimExpr&Expr）操作。简化了实现，并且可以获得正确的 symbol 信息。
Pcard-89620