[Lldb-commits] [PATCH] D108739: [LLDB] AArch64 SVE restore SVE registers after expression

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:543
+  bool contains_sve_reg_data =
+      (data_sp->GetByteSize() > (reg_data_min_size + GetSVEHeaderSize()));
+
----------------
omjavaid wrote:
> DavidSpickett wrote:
> > I'm a bit dubious about these size checks as a way to tell if things are present, should this one have `GetRegisterInfo().IsSVEEnabled()` also?
> > 
> > I'm thinking what if PAC plus MTE plus <random future ext> adds enough registers to exceed the size of the SVE header, but the process doesn't in fact have SVE.
> > 
> > You could do the size checks in terms of bytes left to read from the data_sp, that might simplify it some too.
> We are not sure if SVE was enabled or not when we restore. So size checks is the only way we can figure out if a register checkpoint was created when SVE was enabled. This works for now but if we have to stack another register set which is greater than size of SVE header then we will have to put information about enabled register sets inside the checkpoint maybe in a bitmap describing which register set is enabled and look for those at predefined offsets. I thought this wasn't needed for now and will over complicate already complicated variable sized checkpoint.
Just to shed more light on this if SVE is note enabled we just have GPR + FPR + MTE predictable from size. As MTE is just 8 bytes we can be sure if checkpoint has data beyond FPR which is greater than GPR + FPR + SVE_HEADER that means we atleast have SVE header present. Then we check if its a valid header using !sve_vl_valid(m_sve_header.vl)) check and write it to configure SVE registers size before writing actual registers.
If we dont have register data beyond greater than GPR+FPR+MTE then we can safely say there is no SVE present at all.


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

https://reviews.llvm.org/D108739