-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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]add kernel dialect #54428
[IR]add kernel dialect #54428
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
void PhiKernelOp::Verify(const std::vector<ir::OpResult> &inputs, | ||
const std::vector<ir::Type> &outputs, | ||
const ir::AttributeMap &attributes) { | ||
VLOG(4) << "Verifying inputs, outputs and attributes for: SetParameterOp."; |
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.
VLOG(4) << "Verifying inputs, outputs and attributes for: SetParameterOp."; | |
VLOG(4) << "Verifying inputs, outputs and attributes for: PhiKernelOp."; |
|
||
// Verify if attributes contain attribute name in attributes_name: | ||
// if (!attributes.at("parameter_name").isa<StrAttribute>()) { | ||
// throw("Type of attribute: parameter_name is not right."); |
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.
这里是默认不进行任何Verify 操作么,期望行为就是这样的么?
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.
这个是需要在开发中,往内部添加数据,然后统一完善verify
AllocatedDenseTensorTypeStorage); | ||
|
||
static AllocatedDenseTensorType get(ir::IrContext *ctx, | ||
phi::Place place, |
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.
phi::Place place, | |
const phi::Place& place, |
static AllocatedDenseTensorType get(ir::IrContext *ctx, | ||
phi::Place place, | ||
ir::Type dtype, | ||
phi::DDim dims, |
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.
同上,这里的参数有些应该是const & 的?另外,参数的顺序是否有偏好要求,这个看起来是一个底层数据Type。
namespace paddle { | ||
namespace dialect { | ||
|
||
class PaddleKernelDialect : public ir::Dialect { |
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.
我看下面有些数据结构命名是以Phi开头,有些是以Paddle开头,我们的区分原则是什么,还是说「统一」成一个词也没有问题?
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.
phi 自己是一个不在paddle下的namespace; 这个确实可以统一下,都用同一个的单词
@@ -0,0 +1,62 @@ | |||
// Copyright (c) 2023 PaddlePaddle Authors. All Rights Reserved. |
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.
pd_kernel_dialect 要和已经存在 pd_dialect 都铺在一个目录下么?是否需要分成两个目录?
@@ -112,6 +112,15 @@ struct DenseTensorTypeStorage : public ir::TypeStorage { | |||
return ParamKey(dtype_, dims_, layout_, lod_, offset_) == key; | |||
} | |||
|
|||
bool operator==(const DenseTensorTypeStorage &storage) const { |
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.
这个接口在哪里有需求么?
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.
这个是AllocatedTensorTypeStrorage 依赖了 DenseTensorTypeStorage ,我一起删了
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.
LGTM
PR types
Others
PR changes
Others
Description
新增kernel dialect
Other
Pcard-67164