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+PIR]Fix IsSupportCINN Logic #61225

Merged
merged 2 commits into from
Jan 27, 2024

Conversation

Aurelius84
Copy link
Contributor

@Aurelius84 Aurelius84 commented Jan 26, 2024

PR types

Bug fixes

PR changes

Others

Description

Pcard-67164

修复后,scale和concat都可以融合进group 子图了
image

auto value = op.operand_source(i);
if (value && value.type()) {
auto value_type = value.type();
if (value_type.isa<::pir::VectorType>() &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

concat 是因为其输入是一个VectorType,导致AllInputDenseTensor返回了False,因此这里扩充了这个分支,即可修复此问题

@@ -56,6 +56,7 @@ const std::unordered_map<std::string, std::string> CompatibleInfo::OP_NAMES = {
{"pd_op.split_with_num", "split"},
{"pd_op.reshape", "reshape"},
{"pd_op.expand", "broadcast_to"},
{"pd_op.concat", "concat"},
Copy link
Contributor Author

@Aurelius84 Aurelius84 Jan 26, 2024

Choose a reason for hiding this comment

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

这个名单在过渡期其实应该只有一个角色:即处理PIR与CINN AST端 OpName 映射不一致的问题,已清理逻辑

@@ -133,32 +134,62 @@ bool UnimplementOps(const ::pir::Operation& op) {
bool HaveZeroDimInput(const ::pir::Operation& op) {
Copy link
Contributor

Choose a reason for hiding this comment

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

可以把这里的代码整理成如下风格吗?

auto MatchXXX = [&](int i) {
   ...
};
auto MatchYYY = [&](int i) {
   ...
};
for (size_t i = 0; i < op.num_operands(); ++i) {
  if (MatchXXX(i)) return true;
  if (MatchYYY(i)) return true;
}
return false;

尽可能减少圈复杂度。提高可读性。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for advice, fixed

@Aurelius84 Aurelius84 merged commit 4f8daf0 into PaddlePaddle:develop Jan 27, 2024
29 of 30 checks passed
eee4017 pushed a commit to eee4017/Paddle that referenced this pull request Jan 30, 2024
* [CINN+PIR]Fix IsSupportCINN Logic

* fix comment
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.

3 participants