-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[NPU] Move serialize functions to ir_serializer #32241
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
base: master
Are you sure you want to change the base?
[NPU] Move serialize functions to ir_serializer #32241
Conversation
Signed-off-by: alexandruenache1111 <[email protected]>
* before copying. | ||
* @details This is meant as a replacement for the legacy "ie_memcpy" function coming from the OpenVINO API. | ||
*/ | ||
void checkedMemcpy(void* destination, size_t destinationSize, const void* source, size_t numberOfBytes); |
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.
why is this part of the header as well?
#include "custom_stream_buffer.hpp" | ||
#include "intel_npu/utils/logger/logger.hpp" | ||
#include "openvino/pass/manager.hpp" | ||
#include "ze_graph_ext_wrappers.hpp" |
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.
Is this really needed here?
* API, the layout fields shall be filled with default values in order to assure the backward compatibility | ||
* with the driver. | ||
*/ | ||
SerializedIR serializeIR(const std::shared_ptr<const ov::Model>& model, |
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.
why is this a global function and not part of the IRSerializer?
* @brief For driver backward compatibility reasons, the given value shall be converted to a string corresponding to the | ||
* adequate legacy precision. | ||
*/ | ||
std::string ovPrecisionToLegacyPrecisionString(const ov::element::Type& precision); |
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.
why is this needed in the .hpp?
* the default one may lead either to error or to accuracy failures since unwanted transposition layers may be | ||
* introduced. | ||
*/ | ||
std::string rankToLegacyLayoutString(const size_t rank); |
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.
same question
*/ | ||
std::string rankToLegacyLayoutString(const size_t rank); | ||
|
||
std::string serializeIOInfo(const std::shared_ptr<const ov::Model>& model, const bool useIndices); |
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.
why isn't this part of the IRSerializer class?
|
||
std::string serializeIOInfo(const std::shared_ptr<const ov::Model>& model, const bool useIndices); | ||
|
||
std::string serializeConfig(const Config& config, |
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.
same question.
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, we use dedicated function to support flex version control in driver compiler adapter before. Since #32121 removed support for some old ext versions, refactor also work for me.
Details:
serializeIR
,serializeIOInfo
,serializeConfig
,checkedMemcpy
,ovPrecisionToLegacyPrecisionString
, andrankToLegacyLayoutString
fromdriver_compiler_adapter
toir_serializer
.Tickets: