[Lldb-commits] [PATCH] D25864: Fix arm64 floating point register spill recording in UnwindPlan analysis

[Jason Molenda via lldb-commits]

jasonmolenda added a comment.

Hi Tamas, sorry for not replying earlier, something urgent came up that I needed to look at.

Thanks for the review.  I agree with using your existing arm64 register enums in lldb-arm64-register-enums.h.  I'd like to remove the enums in RegisterInfos_arm64.h instead of having two copies that could diverge.

You asked about having EmulateInstructionARM64 handle both eRegisterKindDWARF and eRegisterKindLLDB.  I'm not sure how that would work - each UnwindPlan must can use only one register numbering scheme.  We could use eRegisterKindDWARF if there were no floating point register save/restores - but that seems a bit complicated; we'd conditionally use eRegisterKindLLDB sometimes, eRegisterKindDWARF most of the time.

As for the GetRegisterName() function in ARM64_LLDB_Registers.cpp, we could have EmulateInstructionARM64 with the register file definition (we need to define seven preprocessor symbols before we can include RegisterInfos_arm64.h and then we'll need to write a GetRegisterName() method in EmulateInstructionARM64.cpp.  Wouldn't it be clearer to have a lldb-arm64-register-enums.cpp with this method?

I was talking about these register numbering problems with Greg Clayton and he half-jokingly said we should switch from register numbers to using ConstString register names in the UnwindPlans.  It's not a bad idea!  As long as everyone can agree on the register names for an architecture, anyway.


Repository:
  rL LLVM

https://reviews.llvm.org/D25864