Skip to content

Conversation

aobolensk
Copy link
Contributor

Tickets:

  • N/A

@aobolensk aobolensk requested review from a team as code owners September 17, 2025 16:34
@aobolensk aobolensk requested review from ValentinaKats and removed request for a team September 17, 2025 16:35
@github-actions github-actions bot added category: CPU OpenVINO CPU plugin category: build OpenVINO cmake script / infra category: docs OpenVINO documentation labels Sep 17, 2025
@aobolensk aobolensk changed the title [Snippets][DEBUG_CAPS] Implement RegPrinter for ARM64 [Snippets][CPU][DEBUG_CAPS] Implement RegPrinter for ARM64 Sep 17, 2025
@aobolensk aobolensk added the platform: arm OpenVINO on ARM / ARM64 label Sep 17, 2025
} // namespace

template <typename T>
void RegPrinter::print_reg_prc(const char* name, const char* ori_name, T* ptr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ori_name -> orig_name is better abbreviation for original

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied

Comment on lines 18 to 20
// 1. Include this header in AArch64 JIT kernels when register printing is required.
// 2. Invoke RegPrinter::print method with a supported register/precision combination.
// Examples:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// 1. Include this header in AArch64 JIT kernels when register printing is required.
// 2. Invoke RegPrinter::print method with a supported register/precision combination.
// Examples:
// 1. Include this header in AArch64 JIT kernels if register printing is required.
// 2. Invoke the RegPrinter::print method with a supported register/precision combination.
// Examples:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied, thanks

Copy link
Contributor

@v-Golubev v-Golubev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No critical comments from my side, LGTM

Comment on lines +40 to +50
inline const char* get_original_name(const VReg& reg) {
return vreg_names[reg.getIdx()];
}

inline const char* get_original_name(const XReg& reg) {
return xreg_names[reg.getIdx()];
}

inline const char* get_original_name(const WReg& reg) {
return wreg_names[reg.getIdx()];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: maybe we can make a common template method with reg remplate parameter?

}

template <typename PRC_T, size_t vlen>
void RegPrinter::print_vmm_prc(const char* name, const char* orig_name, PRC_T* ptr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: it looks like print_vmm_prc and print_reg_prc are ISA agnostic and can be reused in both X64 and aarch64 RegPrinter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what we can do with that at this point, since we don't have any common space for x64 & aarch64 debug caps

void RegPrinter::print_vmm(jit_generator_t& h, const REG_T& vmm, const char* name) {
preamble(h);

const size_t vmm_bytes = vmm.getBit() / 8;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const size_t vmm_bytes = vmm.getBit() / 8;
const size_t vmm_bytes = vmm.getBit() / CHAR_BIT;

void RegPrinter::print_reg(jit_generator_t& h, const REG_T& reg, const char* name) {
preamble(h);

const size_t reg_bytes = reg.getBit() / 8;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const size_t reg_bytes = reg.getBit() / 8;
const size_t reg_bytes = reg.getBit() / CHAR_BIT;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: build OpenVINO cmake script / infra category: CPU OpenVINO CPU plugin category: docs OpenVINO documentation platform: arm OpenVINO on ARM / ARM64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants