Add cinn_instruction_run_op for launching execution of a cinn instruction #39435
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your contribution! |
… add_cinn_run_op
… add_cinn_run_op
… add_cinn_run_op
wzzju
reviewed
Feb 10, 2022
Comment on lines
8
to
9
| #op_library(cinn_launch_op SRCS cinn_launch_op.cc cinn_launch_op.cu.cc DEPS string_helper cinn cinn_compiler cinn_op_helper cinn_launch_context) | ||
| #op_library(cinn_instruction_run_op SRCS cinn_instruction_run_op.cc cinn_instruction_run_op.cu.cc DEPS string_helper cinn cinn_compiler cinn_op_helper cinn_launch_context) |
| res, paddle2cinn_varmap_.end(), | ||
| platform::errors::InvalidArgument( | ||
| "Variable(%s) not found in compilation result", paddle_var_name)); | ||
| auto it = name2argument_.find(res->second); |
There was a problem hiding this comment.
请在注释中注明name2argument_中的key使用的是cinn var的名字。
thisjiang
reviewed
Feb 14, 2022
| compiled_object.launch_context.get(); | ||
| auto share_argument_buffer_fn = [launch_context, | ||
| &ctx](const std::string& var_name) { | ||
| cinn_buffer_t* buffer = launch_context->GetCinnBufferOfVar(var_name); |
There was a problem hiding this comment.
这儿是否可能返回为空指针或者野指针啊？特别是在多线程环境下？建议加个PADDLE_ENFORCE_NE
There was a problem hiding this comment.
GetCinnBufferOfVar函数中有 ENFORCE 判断，若是变量名不存在会报错
| auto share_argument_buffer_fn = [launch_context, | ||
| &ctx](const std::string& var_name) { | ||
| cinn_buffer_t* buffer = launch_context->GetCinnBufferOfVar(var_name); | ||
| framework::Variable* var = ctx.scope().GetVar(var_name); |
There was a problem hiding this comment.
同理，这儿要不先判断下ctx.scope().HasVar(var_name)（如果有这个接口）？
There was a problem hiding this comment.
scope.GetVar 函数中有 ENFORCE 判断
| framework::Variable* var = ctx.scope().GetVar(var_name); | ||
| auto* tensor = var->template GetMutable<framework::LoDTensor>(); | ||
| buffer->memory = | ||
| reinterpret_cast<uint8_t*>(tensor->mutable_data<T>(ctx.GetPlace())); |
There was a problem hiding this comment.
这儿为啥要转为uint8_t*啊？CINN的buffer指针都是uint8_t*类型的么？
There was a problem hiding this comment.
裸指针，是。cinn 会根据实际数据类型读取
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR types
Performance optimization
PR changes
OPs
Describe
cinn_instruction_run_op: detail explanation of this operator can refer to the comments in its OpMaker.CinnCompiler: add aindex->cached objectmap as the real storage of allCinnCompiledObjectsand the old two cache maps save their indexes as value, and add a new interfaceGetCompiledObjectfor getting the cached result by a index.cinn variable name -> paddle variable nameinCinnLaunchContext, which is generated by the current mappaddle variable name -> cinn variable namethat is also extended to include the internal variables(the map only include external variables originally).GetCinnBufferOfVarinCinnLaunchContextfor getting thecinn_buffer_tobject of a specified variable.