Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CINN] Add pass for broadcast tree to condition op in dynamic shape #60828

Merged
merged 23 commits into from
Jan 23, 2024

Conversation

zyfncg
Copy link
Contributor

@zyfncg zyfncg commented Jan 15, 2024

PR types

Others

PR changes

Others

Description

Pcard-73448

Add pass for broadcast tree to condition op in dynamic shape.

Copy link

paddle-bot bot commented Jan 15, 2024

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

zhangbo9674
zhangbo9674 previously approved these changes Jan 22, 2024
Copy link
Contributor

@zhangbo9674 zhangbo9674 left a comment

Choose a reason for hiding this comment

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

LGTM overrall

Comment on lines 58 to 61
VLOG(4) << "Run func_args_ size: " << func_args_.size();
for (const auto& args : func_args_) {
VLOG(4) << " args type_code: " << args.type_code();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
VLOG(4) << "Run func_args_ size: " << func_args_.size();
for (const auto& args : func_args_) {
VLOG(4) << " args type_code: " << args.type_code();
}
if (VLOG_IS_ON(4)) {
VLOG(4) << "Run func_args_ size: " << func_args_.size();
for (const auto& args : func_args_) {
VLOG(4) << " args type_code: " << args.type_code();
}
}

Copy link
Contributor Author

@zyfncg zyfncg Jan 22, 2024

Choose a reason for hiding this comment

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

已修改,thx~

@@ -88,6 +91,11 @@ class CinnJitInstruction::FnPtrImpl {
sizeof(int32_t*)));
}

VLOG(4) << "InferShape func_args_ size: " << func_args_.size();
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

std::ostream& operator<<(std::ostream& os, const BroadcastTree& tree);

} // namespace cinn::common
Copy link
Contributor

Choose a reason for hiding this comment

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

运算符重载为什么要删了呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BroadcastTree有多个构造接口,与其他输出流重载函数会出现编译冲突

}

std::vector<::pir::Value> output_values;
for (auto* op : this->ops) {
Copy link
Contributor

Choose a reason for hiding this comment

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

返回值 output_values 是个 vector,,这里似乎要从 group_ops_set 里面遍历,否则 output_values 里面可能有重复值

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ops带有图的拓扑结构,理论上不会出现重复op

@zyfncg zyfncg merged commit cb5c1c7 into PaddlePaddle:develop Jan 23, 2024
29 checks passed
@zyfncg zyfncg deleted the broadcast_tree_to_cond_op branch January 23, 2024 03:07
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.

None yet

3 participants