[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier<clang::CXXRecordDecl>(
+      ConstString(register_type_name.c_str()));
+
----------------
DavidSpickett wrote:
> aprantl wrote:
> > DavidSpickett wrote:
> > > aprantl wrote:
> > > > bulbazord wrote:
> > > > > This seems highly specific to C++... Let's try to find another way to do this, ideally with `TypeSystem` instead of `TypeSystemClang` and `clang::CXXRecordDecl`.
> > > > Are we saving a lot of code by going through the clang typesystem here, or would walking the bits and formatting the directly be roughly the same amount of code?
> > > Funny you should mention that.
> > > 
> > > I was initially doing that, but in the RFC it was suggested I use the existing printers for C types (https://discourse.llvm.org/t/rfc-showing-register-fields-in-lldb/64676/2). There's also a hint that this would make assigning to specific fields more easy (though that is a very far off goal).
> > > 
> > > This is what the first version looked like:
> > > ```
> > > (lldb) register read cpsr
> > >     cpsr = 0x60001000
> > > | N | Z | C | V | TCO | DIT | UAO | PAN | SS | IL | SSBS | BTYPE | D | A | I | F | nRW | EL | SP |
> > > | 0 | 1 | 1 | 0 |  0  |  0  |  0  |  0  | 0  | 0  |  1   |   0   | 0 | 0 | 0 | 0 |  0  | 0  | 0  |
> > > ```
> > > Of course it would be refined based on line length and field size etc etc. A lot of the things the type printers already do, which is why using them looked like it would save some effort.
> > > 
> > > Some of that extra formatting code will get written anyway because I want to eventually add a register command that will show you the register layout. So table formatting is needed either way. That would produce something like:
> > > ```
> > > (lldb) register info cpsr
> > > | 31 | 30 | ...
> > > | N  |  Z | ...
> > > ```
> > > 
> > > So overall no there's nothing preventing me from walking the bits. Assuming you already have code to format text tables nicely, it would be about the same amount of code.
> > > 
> > > I need to check if we have any generic table formatting code, or could extract some from somewhere.
> > Sorry I wasn't aware of the history :-)
> Do you have an idea of how much work it would be to make TypeSystem do the same things TypeSystemClang does? Maybe more to the point, would I be trying to make TypeSystem do something it just isn't designed to do.
Fundamentally, creating new types is something so tightly coupled to a specific type system that it may not be a good idea to expose this through the generic TypeSystem interface.

We could design a generic interface that looks like 
```
CompilerType ty = CreateRecordType({{"field1", CompilerType1}, {"field2", ...}})
```
but here you explicitly want to control packing and bitfields, and so you will end up with something that most likely only TypeSystemClang can fulfill anyway.

So the better solution may be along the lines of a register formatting plugin that depends on TypeSystemClang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145580