-
Notifications
You must be signed in to change notification settings - Fork 3k
[Snippets][CPU] Refactor verbose utils to shared common helpers #33948
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?
[Snippets][CPU] Refactor verbose utils to shared common helpers #33948
Conversation
727b3d4 to
811822c
Compare
src/plugins/intel_cpu/src/emitters/snippets/common/verbose_utils.hpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_cpu/src/emitters/snippets/common/verbose_utils.hpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_cpu/src/emitters/snippets/common/verbose_utils.hpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_cpu/src/emitters/snippets/aarch64/verbose.hpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_cpu/src/emitters/snippets/aarch64/verbose.cpp
Outdated
Show resolved
Hide resolved
| std::string format_memory_emitter_info(const MemoryEmitter* emitter) { | ||
| std::stringstream ss; | ||
| ss << " src_precision:" << emitter->src_prc << " dst_precision:" << emitter->dst_prc | ||
| << " load/store_element_number:" << emitter->count << " byte_offset:" << emitter->compiled_byte_offset; | ||
| return ss.str(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Format kernel emitter common info | ||
| * | ||
| * This function formats common kernel emitter information that is shared across | ||
| * all architectures. | ||
| */ | ||
| template <typename KernelEmitter> | ||
| std::string format_kernel_emitter_info(const KernelEmitter* emitter) { | ||
| std::stringstream ss; | ||
| ss << " jcp.exec_domain:" << vector_to_string(emitter->jcp.exec_domain) << " num_inputs:" << emitter->num_inputs | ||
| << " num_outputs:" << emitter->num_outputs << " num_unique_buffers:" << emitter->num_unique_buffers | ||
| << " data_ptr_regs_idx:" << vector_to_string(emitter->data_ptr_regs_idx); | ||
| return ss.str(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Format segfault detector emitter info | ||
| */ | ||
| template <typename SegfaultDetectorEmitter> | ||
| std::string format_segfault_detector_info(const SegfaultDetectorEmitter* emitter) { |
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.
It looks like these helpers are not used anywhere. Could you please double check?
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.
Checked, they are indeed unused, removed
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.
But shouldn't we reuse them for all 3 platforms instead of removing? As it is done for memory emitters
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.
The print content seems to be different on different platforms. So it is not as a straightforward as there
bd8d2e3 to
1bdf9b6
Compare
080aa84 to
e755a53
Compare
| std::string format_memory_emitter_info(const MemoryEmitter* emitter) { | ||
| std::stringstream ss; | ||
| ss << " src_precision:" << emitter->src_prc << " dst_precision:" << emitter->dst_prc | ||
| << " load/store_element_number:" << emitter->count << " byte_offset:" << emitter->compiled_byte_offset; | ||
| return ss.str(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Format kernel emitter common info | ||
| * | ||
| * This function formats common kernel emitter information that is shared across | ||
| * all architectures. | ||
| */ | ||
| template <typename KernelEmitter> | ||
| std::string format_kernel_emitter_info(const KernelEmitter* emitter) { | ||
| std::stringstream ss; | ||
| ss << " jcp.exec_domain:" << vector_to_string(emitter->jcp.exec_domain) << " num_inputs:" << emitter->num_inputs | ||
| << " num_outputs:" << emitter->num_outputs << " num_unique_buffers:" << emitter->num_unique_buffers | ||
| << " data_ptr_regs_idx:" << vector_to_string(emitter->data_ptr_regs_idx); | ||
| return ss.str(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Format segfault detector emitter info | ||
| */ | ||
| template <typename SegfaultDetectorEmitter> | ||
| std::string format_segfault_detector_info(const SegfaultDetectorEmitter* emitter) { |
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.
But shouldn't we reuse them for all 3 platforms instead of removing? As it is done for memory emitters
| static std::string init_info_jit_load_memory_emitter(const jit_load_memory_emitter* emitter) { | ||
| std::stringstream ss; | ||
| std::string memory_emitter_info = init_info_jit_memory_emitter(emitter); | ||
| std::string memory_emitter_info = snippets_common::format_memory_emitter_info(emitter); | ||
| ss << "Emitter_type_name:jit_load_memory_emitter" << memory_emitter_info; | ||
| return ss.str(); | ||
| } | ||
|
|
||
| static std::string init_info_jit_load_broadcast_emitter(const jit_load_broadcast_emitter* emitter) { | ||
| std::stringstream ss; | ||
| std::string memory_emitter_info = init_info_jit_memory_emitter(emitter); | ||
| std::string memory_emitter_info = snippets_common::format_memory_emitter_info(emitter); | ||
| ss << "Emitter_type_name:jit_load_broadcast_emitter" << memory_emitter_info; | ||
| return ss.str(); | ||
| } | ||
|
|
||
| static std::string init_info_jit_store_memory_emitter(const jit_store_memory_emitter* emitter) { | ||
| std::stringstream ss; | ||
| std::string memory_emitter_info = init_info_jit_memory_emitter(emitter); | ||
| std::string memory_emitter_info = snippets_common::format_memory_emitter_info(emitter); | ||
| ss << "Emitter_type_name:jit_store_memory_emitter" << memory_emitter_info; | ||
| return ss.str(); | ||
| } |
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.
Actually this code is also duplicated across the platforms. Can we move it to verbose_utils maybe?
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, done
Details:
Unify verbose emitters across different platforms:
jit_emitter_info_t(introducejit_emitter_info_base) across x64/aarch64/riscv64Tickets: