-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[RISC-V] Implement SOS related Debugger API code. #94454
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3988,14 +3988,14 @@ interface ICorDebugRegisterSet : IUnknown | |||||||||||||||||||||||||||
REGISTER_LOONGARCH64_F31, | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
REGISTER_RISCV64_PC = 0, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_RA, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_SP, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_FP, | ||||||||||||||||||||||||||||
jkotas marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||
REGISTER_RISCV64_RA, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_GP, | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am still not sure about
Note, current implementation aimed and tested only with diagnostics SOS command with simple tests, that is extremely limited and I belive not cover all cases of this code usage. So, I leave it as is for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @clamp03 could you please clarify, is X0 here used in the R0 meaning? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
runtime/src/coreclr/inc/cordebuginfo.h Line 183 in 0ad3723
and (looks like) REGISTER_RISCV64_X0 used just as "padding" for proper mapping with g_JITToCorDbgReg runtime/src/coreclr/debug/inc/riscv64/primitives.h Lines 155 to 165 in 23695f9
I am not really strong in this code, do we really could remove R0 from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This padding is an internal implementation detail. It should not leak through the public ICorDebug surface. Arm64 has similar wzr register and we do not have a constant for it in the ICorDebug register enum either. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for explanation, I got the idea. Will prepare appropriate changes. |
||||||||||||||||||||||||||||
REGISTER_RISCV64_TP, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_T0, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_T1, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_T2, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_FP, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_S1, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_A0, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_A1, | ||||||||||||||||||||||||||||
|
@@ -4050,8 +4050,7 @@ interface ICorDebugRegisterSet : IUnknown | |||||||||||||||||||||||||||
REGISTER_RISCV64_F28, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_F29, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_F30, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_F31, | ||||||||||||||||||||||||||||
REGISTER_RISCV64_X0, // TODO-RISCV64-CQ: Add X0 for an use in debug. Need to check. | ||||||||||||||||||||||||||||
REGISTER_RISCV64_F31 | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// other architectures here | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
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.
Also, could you please update the assert in
ConvertRegNumToCorDebugRegister
from_ASSERTE(reg >= 0);
to_ASSERTE(reg > 0);
.`
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.
Sure, will do.
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.
Noti: Build fails on clang16
Error message
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! This probably mean we should use something like
(CorDebugRegister)(255)
instead here. Initially, the sense of-1
usage was "out of range for sure". Will fix this in next PR.