vsk added inline comments.

================
Comment at: lldb/include/lldb/Target/AppleArm64ExceptionClass.h:14
+
+enum class AppleArm64ExceptionClass : unsigned {
+#define APPLE_ARM64_EXCEPTION_CLASS(Name, Code) Name = Code,
----------------
shafik wrote:
> We should use fixed sized integer types whenever possible, I suppose in this 
> case `uint32_t`.
I don't think we should use fixed size integer types in cases where the size 
doesn't matter (e.g. when uint8_t would do, as it would in this case), since 
it's distracting.


================
Comment at: lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp:112
+          auto exception_class =
+              static_cast<AppleArm64ExceptionClass>(esr_val >> 26);
+          if (exception_class !=
----------------
jasonmolenda wrote:
> shafik wrote:
> > Does `26` have a meaning? I am guessing we are shifting to get the EC bits?
> Yeah I think the comment table in AppleArm64ExceptionClass.def would be 
> better placed here, showing the bit positions of the three fields in the esr 
> register probably.  AppleArm64ExceptionClass.def should make it clear that it 
> is operating on the exception class field of the esr register, but the bit 
> positions are more relevant here.
Good point - actually, maybe it's cleaner to keep knowledge of the EC bit 
position in the header. Lmk what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795

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

Reply via email to