[llvm-dev] RFC: Refactor SubclassData

[Ehud Katz via llvm-dev]

The solution in Clang is still very complicated and error prone. A lot of
synchronization work (between different classes - at least in the same
hierarchy) needs to be done manually.

I'll summarize the capabilities of the 3 approaches in the following
(kinda) table, using the following columns *[ Current LLVM | Clang | New
RFC ]* :

   - *[_|X|X] static assert *- that the declared accumulated bitfields do
   not exceed the underlying subclass data size (note that int the New
   implementation it is automatically added on declaration)
      - *[_|_|X] automatic static assert *- is adding the static assert
      needs to be manually or is it done automatically with the declaration of
      the new bitfield.
   - *[_|_|X] runtime assert* - that a new value set, fits into the the
   bitfield (without truncation).
   - *[_|_|X] typed* - as opposed to using a representative type (like
   `int`) and then cast to the actual required type (like `bool` or `enum`).
   Typed (ordinary) bitfields cannot be implemented correctly in MSVC, as the
   types of all the bitfields must be of the same type. Using typed bitfields
   also saves us the need to synchronize the use of `unsigned/signed int` with
   the actual type needed.
   - *[X|_|X] declare in actual class* - as opposed to one of the base
   classes.
   - *[_|_|X] declare (a bitfield) in a single line* - as opposed to the
   need to declare helpers or somekind, like `enum` (manually).
   - *[_|_|X] clean bitfields* - without exposing a bit manipulation `enum`.
   - *[_|_|X] automatic inheritance of unused bits* - no need to get offset
   from super (manually).
   - *[_|_|X] automatic calculation of unused bits* - changing a single
   bitfield doesn't require any other change, but the actual bitfield itself
   (as opposed to changing also the sum of the bit count used by the class, in
   an `enum` - for exmple).
   - *[_|_|X] implicit reference to superclass* - as opposed to the need to
   use the base class' info explicitly.
   - *[_|_|X] no need to know anything about any of the base classes*.

I think the table speaks for itself.

Craig, regarding the `getSubclassDataFromInstruction()`, it still does not
turn the tides of the table, above, into the current implementation.

On Fri, Dec 27, 2019 at 8:57 PM Craig Topper <craig.topper at gmail.com> wrote:

> Ehud, can you elaborate on which classes you're trying to change. I know
> some of the classes already use methods
> like getSubclassDataFromInstruction() to hide bits from the subclasses.
> They could probably shift the data too.
>
> ~Craig
>
>
> On Fri, Dec 27, 2019 at 9:35 AM Bruno Ricci via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi,
>>
>> On 26/12/2019 20:53, Ehud Katz via llvm-dev wrote:
>> > I've tested it on MSVC, gcc, clang and icc.
>> >
>> > The solution in clang (in Decl.h) is not ideal (as you have said
>> yourself). The solution I offer, is using a union of fields of class
>> BitField (this is a new class that implements a bitfield of a specific type
>> requested). With this, the definition, of the required bitfields in the
>> subclass data, remains in the hands of the actual class needing them. And
>> these are all restricted hierarchically by the superclasses of that class.
>>
>> I spent some time packing various bits of data into bit-fields in clang.
>> My experience
>> is that the solution in clang actually works just fine. The Stmt
>> hierarchy has a huge
>> number of bit-field classes (I count more than 40! [1]), but because the
>> offsets are
>> relative adding a bit means only adding one to an enum value a few lines
>> below
>> (a static_assert then checks that the union is not too large).
>>
>> [1]
>> https://github.com/llvm/llvm-project/blob/f20fc65887e2085332d111ee0b05736794423c72/clang/include/clang/AST/Stmt.h#L948
>>
>> Bruno Ricci
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191230/4bb98238/attachment.html>