[PIR] Refine conditional_block op translator #59723

zhangbo9674 · 2023-12-05T13:14:38Z

PR types

New features

PR changes

Others

Description

为了保证cond 算子翻译的成功率，本 PR对翻译策略进行优化，采用的原则是：

遇到 conditional_block_op，直接翻译成一个对应的 if_op，该 if_op 将只有 true 分支，没有 false 分支

本PR主要内容包括：

实现新的翻译策略；
cond_instruction 重命名为 if_instruction
if_op 支持仅有 true block（有2个 region，false region 的 block 为空），适配上下游组件，包括：if op verify 检查、lower 对 false 分支的处理、if_instruction 中对 false 分支的处理
支持 select_input 算子，同时适配下游内容：翻译、lower、执行（该算子暂时不提供对外组网 api）

Pcard-67164

paddle-bot · 2023-12-05T13:14:47Z

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

… dev/reconstruct_if_translate

paddle/fluid/framework/new_executor/instruction/select_input_instruction.h

winter-wang · 2023-12-08T07:09:51Z

paddle/fluid/pir/dialect/operator/ir/control_flow_op.cc

+      phi::errors::PreconditionNotMet("The size %d of true_region must be 1.",
+                                      (*this)->region(0).size()));
+  auto &true_last_op = (*this)->region(0).front().back();
+  PADDLE_ENFORCE_EQ(true,


这儿应该考虑一下if没有返回值的情况。

winter-wang

LGTM

Aurelius84

LGTM， commet 单独提PR fix

Aurelius84 · 2023-12-11T02:18:02Z

paddle/fluid/framework/new_executor/instruction/select_input_instruction.cc

+ private:
+  void copy_tensor(const phi::DenseTensor &lod_tensor,
+                   phi::DenseTensor *out) const {
+    if (!lod_tensor.IsInitialized()) return;


这里为什么直接return了，而不是Throw Error?

Aurelius84 · 2023-12-11T02:23:03Z

paddle/fluid/ir_adaptor/translator/op_translator.cc


+static std::vector<int64_t> ParseCompatibleShapes(
+    const std::vector<int64_t>& dim1, const std::vector<int64_t>& dim2) {
+  IR_ENFORCE(dim1.size() == dim2.size(),


这里是不是可以用PADDLE_ENFORCE？另外想了解下，IR_ENFORCE 抛出的异常显示的python callstack 与框架目前的栈有什么差异，会被pybind层Exception正确映射么?如果能的话，那这里就无所谓了

Aurelius84 · 2023-12-11T02:24:13Z

paddle/fluid/ir_adaptor/translator/op_translator.cc

+    auto op_info = this->LoopkUpOpInfo(ctx, op_desc);
+
+    std::vector<pir::Value> op_inputs = {};
+    auto Mask_name = op_desc.Input("Mask")[0];


Suggested change

auto Mask_name = op_desc.Input("Mask")[0];

auto mask_name = op_desc.Input("Mask")[0];

这里命名首字母不需要大写？

Aurelius84 · 2023-12-11T02:24:31Z

paddle/fluid/ir_adaptor/translator/op_translator.cc

+               op_desc.Type(),
+               Mask_name);
+    op_inputs.push_back(param_map->at(Mask_name).value);
+    for (auto in_name : Input_name) {


Suggested change

for (auto in_name : Input_name) {

for (auto& in_name : Input_name) {

Aurelius84 · 2023-12-11T02:24:56Z

paddle/fluid/ir_adaptor/translator/op_translator.cc

+
+    OpOutputMapping arg_to_idx;
+    OpOutputTypeList op_output_types;
+    auto Out_name = op_desc.Output("Out")[0];


Suggested change

auto Out_name = op_desc.Output("Out")[0];

auto& Out_name = op_desc.Output("Out")[0];

Aurelius84 · 2023-12-11T02:26:26Z

paddle/fluid/ir_adaptor/translator/program_translator.cc

+    array_op.out().set_type(type);
+    return array_op.operation();
+  }
+  return nullptr;


这里直接Throw NotImplement 会不会更好一些？这样下游不用检查这个函数的返回是否有效

Aurelius84 · 2023-12-11T02:27:17Z

paddle/fluid/ir_adaptor/translator/program_translator.cc

-  for (auto input_name : input_names) {
+  auto cond_op_cond = op->Input("Cond")[0];
+  auto& cond_op_inputs = op->Input("Input");
+  for (auto input_name : cond_op_inputs) {


Suggested change

for (auto input_name : cond_op_inputs) {

for (auto& input_name : cond_op_inputs) {

Aurelius84 · 2023-12-11T02:29:45Z

paddle/fluid/pir/dialect/operator/ir/manual_op.cc

+          tensor1.dtype(),
+          tensor2.dtype());
+      IR_ENFORCE(tensor1.data_layout() == tensor2.data_layout(),
+                 "The 1st input data_layout %s should be equal to 2ed input "


Suggested change

"The 1st input data_layout %s should be equal to 2ed input "

"The 1st input data_layout %s should be equal to 2nd input "

winter-wang

LGTM

XiaoguangHu01

LGTM

kangguangli · 2023-12-11T03:14:05Z

paddle/fluid/framework/new_executor/instruction/if_instruction.cc

  false_branch_inter_ =
      new PirInterpreter(place,
                         {},
-                         &false_branch_block,
+                         &if_op.false_block(),


建议改用make_unique

kangguangli · 2023-12-11T03:15:02Z

paddle/fluid/framework/new_executor/instruction/if_instruction.h

+  PirInterpreter* true_branch_inter_ = nullptr;

-  PirInterpreter* false_branch_inter_;
+  PirInterpreter* false_branch_inter_ = nullptr;


换用unique_ptr表明所有权更好吧

kangguangli · 2023-12-11T03:26:04Z

paddle/fluid/ir_adaptor/translator/op_translator.cc

+                             pir::Block* block) override {
+    VLOG(10) << "[op select_input] start transcribing";
+    auto op_info = this->LoopkUpOpInfo(ctx, op_desc);
+


建议在这里添加一句

Suggested change

this->InsertSliceOperationForInput(ctx, param_map, op_desc, input_infos, block);

保证输入的类型正确

kangguangli · 2023-12-11T03:27:22Z

paddle/fluid/pir/dialect/operator/ir/manual_op.h

+  pir::Value mask() { return operand_source(0); }
+  pir::OpResult out() { return result(0); }


少了input的相关接口

refine

6d1d3b3

zhangbo9674 added 21 commits December 5, 2023 13:29

fix

ed4eb36

add select_input

41d3326

fix

0ac12cc

fix

86ffb07

fix

639b601

fix

fb1d157

refine if_op without falseblock

368edfb

fix bug

0346fdd

fix

2ecd269

fix

14a7114

fix

29b9d25

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

731bb65

… dev/reconstruct_if_translate

fix

7b61ae0

fix

6e1127f

fix

48cb766

fix

5a6c28f

fix

a3349dc

fix

b172157

fix

59cedeb

fix conflict

9ae3de0

fix

1b37472

winter-wang reviewed Dec 8, 2023

View reviewed changes

paddle/fluid/framework/new_executor/instruction/select_input_instruction.h Show resolved Hide resolved

winter-wang reviewed Dec 8, 2023

View reviewed changes

paddle/fluid/framework/new_executor/instruction/select_input_instruction.h Show resolved Hide resolved

winter-wang reviewed Dec 8, 2023

View reviewed changes

winter-wang previously approved these changes Dec 8, 2023

View reviewed changes

fix

66dd4d3

zhangbo9674 dismissed winter-wang’s stale review via 66dd4d3 December 8, 2023 08:54

Aurelius84 approved these changes Dec 11, 2023

View reviewed changes

winter-wang approved these changes Dec 11, 2023

View reviewed changes

XiaoguangHu01 approved these changes Dec 11, 2023

View reviewed changes

From00 approved these changes Dec 11, 2023

View reviewed changes

zhangbo9674 merged commit e5fbff4 into PaddlePaddle:develop Dec 11, 2023

kangguangli approved these changes Dec 11, 2023

View reviewed changes

	auto Mask_name = op_desc.Input("Mask")[0];
	auto mask_name = op_desc.Input("Mask")[0];

	for (auto in_name : Input_name) {
	for (auto& in_name : Input_name) {

	auto Out_name = op_desc.Output("Out")[0];
	auto& Out_name = op_desc.Output("Out")[0];

	for (auto input_name : cond_op_inputs) {
	for (auto& input_name : cond_op_inputs) {

	"The 1st input data_layout %s should be equal to 2ed input "
	"The 1st input data_layout %s should be equal to 2nd input "


	this->InsertSliceOperationForInput(ctx, param_map, op_desc, input_infos, block);

		pir::Value mask() { return operand_source(0); }
		pir::OpResult out() { return result(0); }

[PIR] Refine conditional_block op translator #59723

[PIR] Refine conditional_block op translator #59723

Uh oh!

Conversation

zhangbo9674 commented Dec 5, 2023 • 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 Dec 5, 2023

Uh oh!

Uh oh!

Uh oh!

winter-wang Dec 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

winter-wang left a comment

Choose a reason for hiding this comment

Uh oh!

Aurelius84 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

winter-wang left a comment

Choose a reason for hiding this comment

Uh oh!

XiaoguangHu01 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zhangbo9674 commented Dec 5, 2023 •

edited

Loading

winter-wang Dec 8, 2023 •

edited

Loading

Aurelius84 left a comment •

edited

Loading