-
Notifications
You must be signed in to change notification settings - Fork 3.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
[Relay][External Codegen] Support data types for CSourceModuleCodegen args and output #4934
Conversation
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.
Thanks for improving it. Overall LGTM, just left some minor comments.
@@ -70,6 +72,12 @@ class CodegenC : public ExprVisitor, public CodegenCBase { | |||
for (size_t i = 0; i < in_shape.size(); ++i) { | |||
macro_stream << ", " << in_shape[i]; | |||
} | |||
|
|||
auto type_node = call->checked_type().as<TensorTypeNode>(); |
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 auto* type_node
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.
Updated
@@ -116,29 +123,30 @@ class CodegenCBase { | |||
* | |||
* \endcode | |||
*/ | |||
void GenerateBackendCFunc(const std::string& func_name, int arg_cnt) { | |||
void GenerateBackendCFunc(const std::string& func_name, Array<Var> args, const Output& out) { |
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 Array& args
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.
Updated
@@ -207,17 +215,20 @@ class CodegenCBase { | |||
* | |||
* \return The emitted code string. | |||
*/ | |||
std::string JitImpl(std::string ext_func_id, std::vector<std::string> args, | |||
std::string JitImpl(std::string ext_func_id, Array<Var> args, |
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.
please also use const reference here for all parameters.
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.
Updated
* | ||
* \return The dtype string. | ||
*/ | ||
std::string GetDtypeString(Var var) { |
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 Var&
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.
Updated
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
… args and output (apache#4934) * Support int args and no extra buffers * Fixes * remove testing code * fix style * more style * use const args * style Co-authored-by: Jon Soifer <[email protected]>
… args and output (apache#4934) * Support int args and no extra buffers * Fixes * remove testing code * fix style * more style * use const args * style Co-authored-by: Jon Soifer <[email protected]>
… args and output (apache#4934) * Support int args and no extra buffers * Fixes * remove testing code * fix style * more style * use const args * style Co-authored-by: Jon Soifer <[email protected]>
Adds a struct,
Output
, to hold information about outputs forCSourceModuleCodegen
. The codegen then uses this information to generate code with the correct datatype and copy it to the output buffer (if necessary).@comaniac @zhiics would you mind taking a look?
I added a unit test for dtype, but did not add a unit test for ignoring the
memcpy
. I was having a hard time making the codegen code look clean.