[pir]Adding FusionOp and CinnFusionLoweringPass #60769

zhangyuqin1998 · 2024-01-11T16:27:18Z

PR types

New features

PR changes

Others

Description

In the previous code, CinnGroupLoweringPass took on too many tasks, including dividing the GroupOp into subgraphs that can be fused, merging the subgraphs, and constructing the JitKernelOp. This PR moves the merging of subgraphs and constructing the JitKernelOp to CinnFusionLoweringPass for future development.

paddle-bot · 2024-01-11T16:27:23Z

你的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.

zyfncg · 2024-01-15T03:26:08Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/cinn_fusion_lowering_pass.cc

+    auto group = RebuildGroup(fusion_op);
+    // Because the group is rebuilt, the order of group.output_values generated
+    // by BuildCUDAJITInfo may not be same with the order bound in the yield op,
+    // so a mapping is required.
+    std::unordered_map<::pir::Value, size_t> value2id;


这里的output可以直接做到按位对齐吗？

这里的output可以直接做到按位对齐吗？

要对齐的话就得记录下旧group里面output_ops和ops的顺序，感觉不太好搞

Group 内 op list 是有序的，理论上输出的value也可以按照在 op list 中出现的顺序来排列，如果这样处理的话会有什么问题？

Group 内 op list 是有序的，理论上输出的value也可以按照在 op list 中出现的顺序来排列，如果这样处理的话会有什么问题？

这里我再看下，理论上是可以的

zyfncg · 2024-01-15T03:29:14Z

paddle/cinn/hlir/dialect/operator/ir/manual_op.cc

+  argument.AddAttribute(
+      "op_pattern_kind",
+      pir::Int32Attribute::get(pir::IrContext::Instance(), op_pattern_kind));


op_pattern_kind 这个信息是不是必要的？可以通过op list推出来吗

op_pattern_kind 这个信息是不是必要的？可以通过op list推出来吗

这个可以通过op list推出来，但这样就得重复计算一遍

这里重复计算的开销估计不大，我们可以优先考虑设计的合理性

这里重复计算的开销估计不大，我们可以优先考虑设计的合理性

好滴

zyfncg · 2024-01-15T03:32:46Z

paddle/cinn/hlir/dialect/operator/ir/manual_op.h

+// FusionOp represents a subgraphs that can be fused. Every GroupOp
+// can be lowered to at least one FusionOp


解释里再强调下一个FusionOp对应会生成一个Kernel

解释里再强调下一个FusionOp对应会生成一个Kernel

好滴

zyfncg · 2024-01-15T03:33:14Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/cinn_fusion_lowering_pass.cc

@@ -0,0 +1,196 @@
+// Copyright (c) 2023 PaddlePaddle Authors. All Rights Reserved.


2023->2024

好滴

zyfncg · 2024-01-15T03:36:38Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/cinn_fusion_lowering_pass.cc

+  std::shared_ptr<pir::ShapeConstraintIRAnalysis> shape_analysis_{nullptr};
+};
+
+class CinnFusionLoweringPass : public pir::PatternRewritePass {


感觉叫LowerCinnFusionOpPass更匹配些
原来的CINNGroupLoweringPass名字应该也需要调整下

感觉叫LowerCinnFusionOpPass更匹配些原来的CINNGroupLoweringPass名字应该也需要调整下

好滴

zyfncg · 2024-01-15T05:03:18Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/cinn_fusion_lowering_pass.cc

+    if (FLAGS_cinn_enable_map_expr) {
+      cinn::adt::TryGenerateMapExprFromGroup(group);
+    }
+
+    auto fn_ptr_res = ir_compiler->BuildCUDAJITInfo({group});


这里加个TODO吧，用一个新的Group数据结构替换现有的这个Group

这里加个TODO吧，用一个新的Group数据结构替换现有的这个Group

好滴

BiynXu

LGTM

zyfncg · 2024-01-16T12:31:40Z

paddle/cinn/hlir/dialect/operator/ir/manual_op.cc

+void FusionOp::Build(pir::Builder& builder,
+                     pir::OperationArgument& argument,
+                     const std::vector<pir::Type>& output_types,
+                     const int op_pattern_kind) {


op_pattern_kind这个参数没有使用了，可以去掉

op_pattern_kind这个参数没有使用了，可以去掉

好滴

zyfncg · 2024-01-16T12:34:35Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/cinn_fusion_lowering_pass.h

@@ -0,0 +1,29 @@
+// Copyright (c) 2023 PaddlePaddle Authors. All Rights Reserved.


zyfncg · 2024-01-16T12:43:59Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/cinn_group_lowering_pass.cc

@@ -289,9 +300,9 @@ class GroupOpPattern : public pir::OpRewritePattern<cinn::dialect::GroupOp> {
  std::shared_ptr<pir::ShapeConstraintIRAnalysis> shape_analysis_{nullptr};
 };

-class CinnGroupLoweringPass : public pir::PatternRewritePass {
+class LowerCinnGroupOpPass : public pir::PatternRewritePass {


这里叫lower稍微有点不直观，这个pass主要是在做op group的融合，看是不是可以用 merge 或者 fuse？

这里叫lower稍微有点不直观，这个pass主要是在做op group的融合，看是不是可以用 merge 或者 fuse？

好滴，我改成MergeCinnGroupOpPass

zyfncg · 2024-01-16T12:46:06Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/cinn_group_lowering_pass.cc

文件名与pass命名上要对应

文件名与pass命名上要对应

好滴

Aurelius84 · 2024-01-17T02:28:26Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/merge_cinn_group_pass.cc

+*/
+
+std::vector<pir::Value> GetBlockOutsideOutput(
+    const std::vector<pir::Operation*> op_list) {


Suggested change

const std::vector<pir::Operation*> op_list) {

const std::vector<pir::Operation*>& op_list) {

Aurelius84 · 2024-01-17T02:34:55Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/merge_cinn_group_pass.cc

-      for (size_t i = 0; i < group->output_values.size(); ++i) {
-        vec_types.push_back(group->output_values[i].type());
+      auto fusion_op = rewriter.Build<cinn::dialect::FusionOp>(
+          output_types, group->op_pattern_kind);


FusionOp 的构建还需要 op_pattern_kind信息么？我看其定义并没有存这个信息。

FusionOp 的构建还需要 op_pattern_kind信息么？我看其定义并没有存这个信息。

是的，这里已经改了，还没push上来~

Aurelius84 · 2024-01-17T02:39:50Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/merge_cinn_group_pass.cc

@@ -289,11 +300,11 @@ class GroupOpPattern : public pir::OpRewritePattern<cinn::dialect::GroupOp> {
  std::shared_ptr<pir::ShapeConstraintIRAnalysis> shape_analysis_{nullptr};
 };

-class CinnGroupLoweringPass : public pir::PatternRewritePass {
+class MergeCinnGroupOpPass : public pir::PatternRewritePass {


从逻辑上来看，此Pass的效果是将一个GroupOp替换为了FusionOp，这里的Pass名称是否可以调整下以更容易理解。比如CinnGroupToFusionPass ？

从逻辑上来看，此Pass的效果是将一个GroupOp替换为了FusionOp，这里的Pass名称是否可以调整下以更容易理解。比如CinnGroupToFusionPass ？

好滴，这里我再斟酌下

Aurelius84 · 2024-01-17T02:41:01Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/lower_cinn_fusion_pass.cc

+using cinn::hlir::framework::pir::CompatibleInfo;
+
+std::vector<pir::Value> GetBlockOutsideInput(
+    const std::vector<pir::Operation*> op_list) {


Suggested change

const std::vector<pir::Operation*> op_list) {

const std::vector<pir::Operation*>& op_list) {

tc20042008 · 2024-01-18T07:39:32Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/divide_group_op_to_fusion_op_pass.cc

+      pir::Block* fusion_block = fusion_op.block();
+
+      for (auto op : group->ops) {
+        op->MoveTo(fusion_block, fusion_block->end());


move没法处理recompute的情形

move没法处理recompute的情形

好滴

tc20042008 · 2024-01-18T07:41:12Z

paddle/cinn/hlir/dialect/operator/ir/manual_op.cc

+  argument.output_types = output_types;
+}
+
+pir::Block* FusionOp::block() {


严格分离读写。

const pir::Block* FusionOp::block() const; pir::Block* FusionOp::mut_block() const;

严格分离读写。

const pir::Block* FusionOp::block() const; pir::Block* FusionOp::mut_block() const;

好滴

tc20042008 · 2024-01-18T07:43:51Z

paddle/cinn/hlir/dialect/operator/ir/manual_op.cc

+  return &region.front();
+}
+
+std::vector<pir::Operation*> FusionOp::ops() {


小写的方法名一般只用于getter/setter。如果其中会产生大结构体，就应该写成“动词+名词”形式的普通函数名，比如GetOperators()

小写的方法名一般只用于getter/setter。如果其中会产生大结构体，就应该写成“动词+名词”形式的普通函数名，比如GetOperators()

好滴

zyfncg · 2024-01-18T07:55:18Z

paddle/cinn/hlir/dialect/operator/transforms/group_merge/divide_group_op_to_fusion_op_pass.cc

+      pir::Block* fusion_block = fusion_op.block();
+
+      for (auto op : group->ops) {
+        op->MoveTo(fusion_block, fusion_block->end());


目前Group融合时，recompute处理后会出现同一个op同时被多个group持有的情况，move的话确实会有问题，这里可以先使用group的clone

Aurelius84

LGTM

zhangyuqin1998 force-pushed the fusion_lowing_pass branch from 7e0aca7 to 077af2f Compare January 11, 2024 16:40

paddle-bot bot added the contributor External developers label Jan 11, 2024

zhangyuqin1998 changed the title ~~Fusion lowing pass~~ [pir]Adding FusionOp and CinnFusionLoweringPass Jan 12, 2024

zhangyuqin1998 force-pushed the fusion_lowing_pass branch from ded95b3 to b445b18 Compare January 12, 2024 16:08

zyfncg reviewed Jan 15, 2024

View reviewed changes

BiynXu approved these changes Jan 15, 2024

View reviewed changes

zyfncg reviewed Jan 16, 2024

View reviewed changes

Aurelius84 reviewed Jan 17, 2024

View reviewed changes

zhangyuqin1998 force-pushed the fusion_lowing_pass branch 3 times, most recently from f904762 to 54cab46 Compare January 18, 2024 04:17

tc20042008 reviewed Jan 18, 2024

View reviewed changes

zyfncg reviewed Jan 18, 2024

View reviewed changes

[pir]Adding FusionOp and CinnFusionLoweringPass

2a13134

zhangyuqin1998 force-pushed the fusion_lowing_pass branch from 54cab46 to 42f521b Compare January 18, 2024 12:12

fix

4526757

zhangyuqin1998 force-pushed the fusion_lowing_pass branch from 4ec1b09 to 4526757 Compare January 18, 2024 13:13

zhangyuqin1998 added 2 commits January 18, 2024 14:35

fix

65a10ee

fix

7bf7a4c

zyfncg approved these changes Jan 19, 2024

View reviewed changes

Aurelius84 approved these changes Jan 19, 2024

View reviewed changes

zyfncg merged commit 65047de into PaddlePaddle:develop Jan 19, 2024

zhangyuqin1998 deleted the fusion_lowing_pass branch May 7, 2024 16:17

		// FusionOp represents a subgraphs that can be fused. Every GroupOp
		// can be lowered to at least one FusionOp

		@@ -0,0 +1,196 @@
		// Copyright (c) 2023 PaddlePaddle Authors. All Rights Reserved.

		@@ -0,0 +1,29 @@
		// Copyright (c) 2023 PaddlePaddle Authors. All Rights Reserved.

	const std::vector<pir::Operation*> op_list) {
	const std::vector<pir::Operation*>& op_list) {

[pir]Adding FusionOp and CinnFusionLoweringPass #60769

[pir]Adding FusionOp and CinnFusionLoweringPass #60769

Uh oh!

Conversation

zhangyuqin1998 commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR types

PR changes

Description

Uh oh!

paddle-bot bot commented Jan 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BiynXu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

zhangyuqin1998 commented Jan 11, 2024 •

edited

Loading

zyfncg Jan 18, 2024 •

edited

Loading