omjavaid added a comment. In D91241#2397083 <https://reviews.llvm.org/D91241#2397083>, @labath wrote:
> I disagree, and my main reason for that is that this approach requires > hardcoding the "supported architectures" for which we do this fixup in the > DynamicRegisterInfo class. Besides being wrong as a matter of priniciple, I > suspect this will also be a problem in practice -- for example, I have no > idea what will happen when this code starts interacting with the aarch64 > debugserver (which will still be sending the offsets). Doing this decision > based on whether the server sends the offsets avoids this issue, and the > process can be seen as an extension of what we're already doing when > communicating with offset-less stubs. > > In D91241#2396445 <https://reviews.llvm.org/D91241#2396445>, @omjavaid wrote: > >> In LLDB we are trying to support legacy offset field > > I wouldn't go so far as to call this a "legacy" field. Let's just say that > for this particular problem on this architecture, we chose to stop using the > offset field. > >> which means offset calculation has to be done in location where we parse an >> XML register entry (ParseRegisters) or qRegisterInfo packet >> (ProcessGDBRemote::BuildDynamicRegisterInfo). > > Why is that? I don't see the connection here. > > I don't see why the offsets could not be computed in the DynamicRegisterInfo > class. The `ParseRegisters`/`BuildDynamicRegisterInfo` functions could just > refrain from filling in the offset fields (leave them as invalid, or > something) if the server did not provide them. Then DynamicRegisterInfo could > use that invalid value to know which offsets need filling in. > > In fact, if we wanted to be truly faithful to the gdb algorithm you described > ("gdb creates a sorted list based of register numbers and then assigns > offsets accordingly"), then it seems like we would _have to_ do this as a > separate step, once all register numbers are known. I have made some updates which are sufficient for current AArch64 scenario where offset field is not present. For ordering registers based on register numbers I think we may skip doing that for now though i intend to come back to it after getting SVE through. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91241/new/ https://reviews.llvm.org/D91241 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
