-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[Snippets][CPU][DEBUG_CAPS] Implement RegPrinter for ARM64 #32124
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][DEBUG_CAPS] Implement RegPrinter for ARM64 #32124
Conversation
} // namespace | ||
|
||
template <typename T> | ||
void RegPrinter::print_reg_prc(const char* name, const char* ori_name, T* ptr) { |
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.
ori_name -> orig_name is better abbreviation for original
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.
Applied
// 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: |
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.
// 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: |
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.
Applied, thanks
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.
No critical comments from my side, LGTM
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()]; | ||
} |
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.
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) { |
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.
Minor: it looks like print_vmm_prc
and print_reg_prc
are ISA agnostic and can be reused in both X64 and aarch64 RegPrinter
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.
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; |
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 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; |
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 size_t reg_bytes = reg.getBit() / 8; | |
const size_t reg_bytes = reg.getBit() / CHAR_BIT; |
Tickets: