-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[NewIR]support new ir load combine #56101
[NewIR]support new ir load combine #56101
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
❌ The PR is not created using PR's template. You can refer to this Demo. |
op_yaml_info_parser, | ||
&(op_func_node.kernel_context_)); | ||
} | ||
::ir::BuildPhiContext<phi::KernelContext, |
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.
随着执行器接入的完善,BuildOpFuncList函数后续应该没有什么用处了。TODO:后续随执行器代码清理 pr 一并清理
@@ -1547,7 +1547,8 @@ void NewIRInterpreter::BuildInstruction() { | |||
} | |||
|
|||
if (op_name == "pd.fused_softmax_mask_upper_triangle" || | |||
op_name == "pd.fused_softmax_mask_upper_triangle_grad") { | |||
op_name == "pd.fused_softmax_mask_upper_triangle_grad" || | |||
op_name == "pd.load_combine") { |
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.
看起来 Legacy 的策略是用「白名单」来识别的?感觉这里的判断连同「白名单」一起封装成一个函数 IsLegacyKernelOp(op_name)
比较好一些?
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.
已经优化; 理想情况,这款应该用dialect来区分,但是暂时暂时缺少对应的dialect,所以暂时用了白名单
std::vector<paddle::framework::Variable*> vec_tmp = {var}; | ||
|
||
runtime_ctx->outputs[legacy_arg_name] = vec_tmp; |
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.
std::vector<paddle::framework::Variable*> vec_tmp = {var}; | |
runtime_ctx->outputs[legacy_arg_name] = vec_tmp; | |
runtime_ctx->outputs[legacy_arg_name] = {var}; |
|
||
auto& val = attr.second; | ||
|
||
if (val.isa<ir::StrAttribute>()) { |
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.
这里的Attribute解析数据的逻辑在paddle/fluid/ir/dialect/utils.h
已经有GetAttributeData 函数实现了,可以看下是否能够复用?
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.
我确认了下 GetAttributeData 返回的是 phi的attribute, 这里需要的是framework::attribute, 定义还是有些差别,不能直接用同一个
@@ -387,6 +388,9 @@ std::unique_ptr<ir::Program> PdOpLowerToKernelPass(ir::Program* prog, | |||
GetKernelKey(op_item, place, map_value_pair, op_info_parser.get()); | |||
VLOG(6) << "kernel type " << kernel_key; | |||
|
|||
if (op_item->name() == "pd.load_combine") { | |||
kernel_key.set_dtype(phi::DataType::FLOAT32); |
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.
这里为什么直接设置 kernel_key 为FP32了?我看load_combile 是支持很多其他数据类型的?是先验证了FP32,还是后面逻辑会自适应的修改?
若是前者,这里是否需要加一下 TODO 标记下?
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.
load_combine 是一个没有输入的op,无法从输入的参数表里面推导输出的类型,但是kernel key不是有一个类型
… support_new_ir_load_combine
… support_new_ir_load_combine
… support_new_ir_load_combine
PR types
New features
PR changes
Others
Description
新IR 支持load combine op
Others
Pcard-67164