DavidSpickett added a comment.
Can "and make breakpoint work" be made into its own patch? I'd much rather see
"register read and write" and "software breakpoints" in their own changes (and
please differentiate in commit title/messages between hardware break and
software break).
(otherwise the content seems fine just some comments on names and formatting)
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h:49
+ {
\
+#reg, #alt, 8, GPR_OFFSET(gpr_##reg - gpr_first), lldb::eEncodingUint, \
+ lldb::eFormatHex, GPR64_KIND(gpr_##reg, generic_kind), nullptr,
\
----------------
clang-format will align these to the left. So I suggest adding "// clang-format
off" to disable that and go with the "clang-format on" below. Then indent them
as if they were normal code.
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h:130
+
+ // clang-format on
+};
----------------
I check the arm64 file and it also had an orphaned "on". I fixed that in
https://github.com/llvm/llvm-project/commit/552dccf311c6134c6c895328602172821e3efaed.
See comment above.
================
Comment at: lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h:20
+ gpr_pc = gpr_first,
+ gpr_x1,
+ gpr_x2,
----------------
We want to keep the "riscv" tag here. It provides a sort of namespace for the
enumerator otherwise overlapping names in different anonymous enums is an error.
https://godbolt.org/z/1W8P1z83T
E.g. if arm64 just used "gpr_x0" we'd get an error, so it uses "gpr_x0_arm64"
to prevent that.
================
Comment at: lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h:94
- k_first_vcr_riscv,
- vcr_v0_riscv = k_first_vcr_riscv,
- vcr_v1_riscv,
- vcr_v2_riscv,
- vcr_v3_riscv,
- vcr_v4_riscv,
- vcr_v5_riscv,
- vcr_v6_riscv,
- vcr_v7_riscv,
- vcr_v8_riscv,
- vcr_v9_riscv,
- vcr_v10_riscv,
- vcr_v11_riscv,
- vcr_v12_riscv,
- vcr_v13_riscv,
- vcr_v14_riscv,
- vcr_v15_riscv,
- vcr_v16_riscv,
- vcr_v17_riscv,
- vcr_v18_riscv,
- vcr_v19_riscv,
- vcr_v20_riscv,
- vcr_v21_riscv,
- vcr_v22_riscv,
- vcr_v23_riscv,
- vcr_v24_riscv,
- vcr_v25_riscv,
- vcr_v26_riscv,
- vcr_v27_riscv,
- vcr_v28_riscv,
- vcr_v29_riscv,
- vcr_v30_riscv,
- vcr_v31_riscv,
- vcr_vstart_riscv,
- vcr_vxsat_riscv,
- vcr_vxrm_riscv,
- vcr_vcsr_riscv,
- vcr_vl_riscv,
- vcr_vtype_riscv,
- vcr_vlenb_riscv,
- k_last_vcr_riscv = vcr_vlenb_riscv,
-
- k_num_registers_riscv,
- k_num_gpr_registers_riscv = k_last_gpr_riscv - k_first_gpr_riscv + 1,
- k_num_fpr_registers_riscv = k_last_fpr_riscv - k_first_fpr_riscv + 1,
- k_num_vcr_registers_riscv = k_last_vcr_riscv - k_first_vcr_riscv + 1,
+ k_num_registers
};
----------------
Why did the vector registers get removed?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130342/new/
https://reviews.llvm.org/D130342
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
