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

[IR&PASS] part 3-1: Add PatternMatch base class. #54385

Merged
merged 2 commits into from
Jun 7, 2023

Conversation

jiweibo
Copy link
Contributor

@jiweibo jiweibo commented Jun 6, 2023

PR types

Others

PR changes

Others

Description

Pcard-71500

新IR下的新PASS体系,新PASS体系适配新IR。该系列PR主要包括如下几个部分:
主要包括四个部分:(1) Pass base & PassManager & PassAdaptor; #54023 (2) AnalysisManager & Pass Instrumentation (PassTiming, IRPrinting..); #54170 #54308 #54348 (3) Pattern Rewrite ... (4) 其他。


  1. 添加PatternMatch base class及单测。
  2. TODO:现有IR不支持修改操作,需IR支持后继续修改PatternMatch类;
    后续PR:
  • PatternApplicator, FronzenRewritePatternSet
  • GreedyPatternDriver

@paddle-bot
Copy link

paddle-bot bot commented Jun 6, 2023

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

@jiweibo jiweibo requested a review from yuanlehome June 6, 2023 08:28
@jiweibo jiweibo changed the title [PatternRewrite] Add PatternMatch base class. Part1. [IR&PASS] part 3-1: Add PatternMatch base class. Jun 6, 2023

std::string debug_name() const { return debug_name_; }

void set_debug_name(const std::string& name) { debug_name_ = name; }
Copy link
Contributor

Choose a reason for hiding this comment

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

设值函数统一驼峰式命名,SetDebugName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

};

//---------------------------------------------------------//
// Pattern Class
Copy link
Contributor

Choose a reason for hiding this comment

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

.h里这种注释给删了吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


const std::vector<std::string>& debug_labels() const { return debug_labels_; }

void add_debug_labels(const std::vector<std::string>& labels) {
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.

done

debug_labels_.insert(debug_labels_.end(), labels.begin(), labels.end());
}

void add_debug_labels(const std::string& label) {
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.

done

namespace ir {

//---------------------------------------------------------//
// Pattern Class
Copy link
Contributor

Choose a reason for hiding this comment

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

.cc里的这种注释可以参考pass.cc里的形式

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// const std::vector<std::string>& generated_names,
// PatternBenefit benefit,
// ir::IrContext* context);
std::string op_name_;
Copy link
Contributor

Choose a reason for hiding this comment

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

pass/下的编码习惯是,各成员变量以空行分隔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

google编码规范没有这个要求吧

#include "paddle/utils/small_vector.h"
namespace ir {

/// The design is mainly from MLIR, very thanks to the greate project.
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.

留着吧


unsigned benefit() { return val_; }

bool operator==(const PatternBenefit& rhs) const { return val_ == rhs.val_; }
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.

clang-format格式检查没换行

// op->erase();
// }
void RewriterBase::EraseOp(Operation* op) {
// assert(op->use_empty() && "expected 'op' to have no uses");
Copy link
Contributor

Choose a reason for hiding this comment

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

ir下有计划将assert和throw统一换成paddle下enforce和throw,后面需要统一改一下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

等从phi里独立出来之后统一改下

#include "paddle/ir/core/type_id.h"
#include "paddle/ir/core/type_name.h"
#include "paddle/ir/core/value.h"
#include "paddle/utils/small_vector.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

small_vector有用到?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@winter-wang winter-wang left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@winter-wang winter-wang left a comment

Choose a reason for hiding this comment

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

LGTM

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