mgorny marked an inline comment as done.
mgorny added a comment.

I'm sorry for not replying earlier.



================
Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp:141
+  uint64_t f31;
+  uint64_t fcsr;
+  uint64_t fir;
----------------
jrtc27 wrote:
> MIPS calls it FCSR, FreeBSD's regnum.h calls it FSR. I don't know which is 
> the right thing to use, but just thought I should point it out if you hadn't 
> noticed.
I have noticed. Others part of LLDB (and LLVM) call it FCSR, so I've figured 
out it's better to be consistent with that.


================
Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp:161-167
+  // Check if RegisterSet is available
+  if (set < k_num_register_sets)
+    return &g_reg_sets_mips64[set];
+  return nullptr;
 }
 
+size_t RegisterContextFreeBSD_mips64::GetRegisterSetCount() const {
----------------
jrtc27 wrote:
> These diffs should be separated from this commit IMO.
Do you mean reformatting? I suppose I can commit the result of `clang-format` 
earlier. Figured the changes are trivial enough.


================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h:26
 // Computes the offset of the given FPR in the extended data area.
+#ifdef LINUX_MIPS64
 #define FPR_OFFSET(regname)                                                    
\
----------------
jrtc27 wrote:
> Should these not live in the respective RegisterContext<OS>_mips64.cpp?
I really don't know. I've followed suit with the other macros already here.


================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h:37
 // Computes the offset of the given MSA in the extended data area.
 #define MSA_OFFSET(regname)                                                    
\
   (LLVM_EXTENSION offsetof(UserArea, msa) +                                    
\
----------------
jrtc27 wrote:
> This and DEFINE_GPR_INFO should not be defined like this on FreeBSD (or at 
> all?).
I suppose we could make them conditional too but I'm not sure if this wouldn't 
just add complexity for no real change.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96766/new/

https://reviews.llvm.org/D96766

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to