CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
[CINN / OpFusion ] Refinement of HorizontalPattern #63340
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
Conversation
… add_llama_fw_bw_test
你的PR提交成功,感谢你对开源项目的贡献! |
namespace cinn::fusion { | ||
|
||
template <typename T> | ||
struct PatternContent {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个名字比较突兀。是不是改成OpPattern更顺一些?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个名字得考虑一下,这个变量其实表示初始的传入给Graph的数据,后续会按照这个结构来构造Pattern,可以理解为Pattern的初始参数。改为PatternInitializer是不是好点?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可以,尽可能名副其实就行
template <typename T> | ||
StmtPattern<T> RT_x_Trivial(const ReduceTreePattern<T>& first, | ||
const TrivialPattern<T>& second) { | ||
CHECK(false) << "Please specialization!"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static_assert(false, "Please specialize!");
做静态断言。下同。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个地方不能改为 static_assert ,否则会直接编译错误,这个部分应该是运行到这里才挂,而不是编译直接挂。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我试了一下,如果是
static_assert(std::is_same_v<T, FrontendStage > || std::is_same_v<T, BackendStage >, "Please specialization!");
应该是没问题的。
但是这么做了,就有错误依赖的嫌疑。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个地方不能改为 static_assert ,否则会直接编译错误,这个部分应该是运行到这里才挂,而不是编译直接挂。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class HorizontalFusionPattern {}; | ||
|
||
template <typename T> | ||
using StmtPattern = std::variant<TrivialPattern<T>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这种定义会导致编译错误非常杂乱。换一下。
template <typename T>
using StmtPatternBase = ...;
template <typename T>
struct StmtPattern final : public StmtPatternBase {
using StmtPatternBase::StmtPatternBase;
const StmtPatternBase& variant() const {
return static_cast<const StmtPatternBase&>(*this);
}
};
注意,这么做了之后,std::visit的第二个参数不能直接填stmt_pattern,而应该填stmt_pattern.variant();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个问题影响面有点大,我后续修复一下哈~
template class PatternGraph<FrontendStage>; | ||
template class PatternGraph<BackendStage>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
【建议,可以不改】在cc里模板实例化不是一个良好的编码习惯。不过没有太多其他Stage,这么做无关紧要。
--------- Co-authored-by: phlrain <phliuhongyu@126.com>
PR Category
CINN
PR Types
Improvements
Description
Change the Horizontal pattern to recursive definition.
Pcard-76996