labath added a comment.
Looks good. Some minor questions inline.
================
Comment at:
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:523
+ case DBRegSet: {
+ uint8_t *data = GetOffsetRegSetData(set, reg_info->byte_offset);
+ FXSAVE *fpr = reinterpret_cast<FXSAVE*>(m_fpr.data());
----------------
if you make data a `void*`, then you can remove the `reinterpret_cast` two
lines below.
================
Comment at:
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:528
+ else
+ ::memcpy(GetOffsetRegSetData(set, reg_info->byte_offset),
+ reg_value.GetBytes(), reg_value.GetByteSize());
----------------
reuse data from above
================
Comment at:
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:537
+ uint16_t sw = m_xstate->fxsave.fstat;
+ llvm::ArrayRef<MMSReg> st_regs{m_xstate->fxsave.stmm, 8};
+ reg_value.SetUInt16(AbridgedToFullTagWord(abridged_tw, sw, st_regs));
----------------
Is this really needed? I would have hoped that just passing
`m_xstate->fxsave.stmm` would be enough and that the c array constructor of
ArrayRef would do the right thing...
================
Comment at: lldb/unittests/Process/Utility/RegisterContextTest.cpp:55
+ for (const TagWordTestVector &x : tag_word_test_vectors) {
+ std::array<MMSReg, 8> test_regs;
+ for (int i = 0; i < x.st_reg_num; ++i)
----------------
add `SCOPED_TRACE` macro to make it clear which test case failed, when it
fails. You can use llvm::enumerate to produce sequence numbers for the test
cases...
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91504/new/
https://reviews.llvm.org/D91504
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
