[Lldb-commits] [lldb] [lldb] Introduce RegisterType base class for all register type classes (PR #196960)

[David Spickett via lldb-commits]

DavidSpickett wrote:

> For fields and enums that was very easy, but we want to add support for structs and unions soon.

It's pretty easy for unions too - https://github.com/llvm/llvm-project/pull/196032/changes/4d5ac3881496f824b51ff60ae193f59d37ad463a. At least when all union members are other RegisterTypes, I did not implement union of basic types like int/float.

(I see what you mean though, easy at first doesn't mean easy long term or free of tech debt, much like manually dumping the values could have been "easy" but would quickly get out of hand)

> My idea is to avoid making our own custom type system like types and have the RegisterType make its own CompilerType using clang AST types much like DWARF is converted to AST types.

What I have now is `c++ representation of XML types` -> `XML` -> `c++ representation of types` -> `CompilerType` so if I could remove a few layers that'd be great.

I have a few concerns about doing that:
* I think we already link in CompilerType into lldb-server, but I'm not sure we use it and whether we'd need to link in a TypeSystem to use it. If we have already paid that cost / are ok paying it, fine.
* Whether XML to CompilerType would end up being lossy for some types, specifically whether exact bit layouts can be preserved. In https://github.com/llvm/llvm-project/pull/189590 I was able to provide the layout via. a callback but I think that's only called when you go to dump the value. So if we had a CompilerType for a bitfiield struct (aka `flags`), we would need to have some side data that has the exact layout. Though the class would still be more compact than it is now, CompilerType + list of name/range pairs.

> We can easily make a ValueObjectRegister from and RegisterInfo and have it be able to use the ValueObject code to display iteself instead of the custom DumpRegisterValue.cpp and DumpRegisterInfo.cpp.

I don't think that that much of DumpRegisterValue would be removed by doing this, unless you're saying we could move the whole of `lldb_private::DumpRegisterValue` into the ValueObject which would be a more logical place for it.

(as I think the raw value is always going to have its uses)

I'm not sure if you are suggesting that we use the normal type printing for register info, or just put the custom formatting logic into the ValueObjects, but I do want to be clear that at least for `flags` I do not want  to print the default C type for , as it doesn't show the bit ranges which are often useful for low level work.

For example:
```
(lldb) register info por
       Name: por
       Size: 8 bytes (64 bits)
    In sets: Permission Overlay Registers (index 7)

| 63-60  | 59-56  | 55-52  | 51-48  | 47-44  | 43-40  | 39-36 | 35-32 | 31-28 | 27-24 | 23-20 | 19-16 | 15-12 | 11-8  |  7-4  |  3-0  |
|--------|--------|--------|--------|--------|--------|-------|-------|-------|-------|-------|-------|-------|-------|-------|-------|
| Perm15 | Perm14 | Perm13 | Perm12 | Perm11 | Perm10 | Perm9 | Perm8 | Perm7 | Perm6 | Perm5 | Perm4 | Perm3 | Perm2 | Perm1 | Perm0 |

Perm15: 0 = No Access, 1 = Read, 2 = Execute, 3 = Read, Execute, 4 = Write, 5 = Write, Read, 6 = Write, Execute, 7 = Read, Write, Execute
<...>
Perm0: 0 = No Access, 1 = Read, 2 = Execute, 3 = Read, Execute, 4 = Write, 5 = Write, Read, 6 = Write, Execute, 7 = Read, Write, Execute
```
Yes, it does print the same enum 15 times, once for each field that uses it :rofl: This is a good sign that my manual printing code, while I think it has good intentions, is going to fall apart when we start nesting types.

So the extra bits we need are the field enums, which can be a pre or post print thing, and the bit positions. Perhaps we can extend the normal printing with annotations like:
```
struct por {
  uint64_t Perm15:4; // [63-60]
<...>
```
For `vector` and `union` I expect the default path would be fine. Laying it all out is a challenge, so reusing the existing code would really help with that, as it has done for value printing.

I also want one day (:unicorn: ) to add descriptions for registers and register `flags` fields. The former is easy, it's just header text, the latter might get awkward but it's just a future thought so I won't make that a blocker here.

If we really found that we had two audiences:
* People who want the C type in C form, or at least a "compact" form
* People who want the equivalent of the architecture manual, with text and tables and so on

And we can't do a good job for both in one command, `register type` could be the compact one, or we can tell people how to `expr typeof($register)` to see the raw C type when they need to make expressions.

So in summary:
* Sounds great if we can keep `register info` in a useful format.
* I do not have the time at the moment to implement the `CompilerType` approach myself.
* If there were a prototype where value printing worked, I will make the time to deal with the `register info` part (since I am the one most concerned with it).

https://github.com/llvm/llvm-project/pull/196960