dmd.expression: Utilize padding between Expression and derived AST nodes to store bitfields

[ibuclaw]

• Reduces the class instance size of SliceExp from 57 -> 56.
• Reduces the class instance size of StructLiteralExp from 72 -> 64.

Can be used to make more space reductions too.

• DsymbolExp, SymbolExp, DotVarExp, DelegateExp: hasOverloads field
• StringExp, InterpExp, VectorExp, ArrayLiteralExp, AssocArrayLiteralExp: all contain ownByCtfe and bool flags that fit.
• NewExp, VarExp, DotIdExp, CallExp, DeleteExp, ArrayExp, CommaExp, IndexExp: all contain bool flags that fit.
• FuncExp, IsExp, CastExp, AssignExp, DefaultInitExp: all contain one or two byte fields that fit.

[ibuclaw]

Moving the stageflags field into bitFields could be done in a separate PR. Though it does nothing to reduce the instance size of StructLiteralExp itself.

[ibuclaw]

FYI @rikkimax this ushort field could also be used directly as the length field for IntegerExp's, if ever the value field becomes a variable-sized array for representing double (aka cent) or wide (aka bigint) integer values - supporting integers up to ushort.max*ulong.sizeof bytes should be big enough for everyone.

[rainers]

#23519 does the same slightly more transparently for all expressions.

[ibuclaw]

#23519 does the same slightly more transparently for all expressions.

Oh nice that someone else came to the same realisation. :-)

You can do it for more expressions. But I agree that the 16 byte allocations used by bump-the-pointer may well make it underwhelming.

I was a bit annoyed of having reduced StringExp to a non-multiple of 8 (52->41), and was thinking of doing something similar to what you have there, but I'll just copy-paste it as-is to save some time. :-)

[rainers]

But I agree that the 16 byte allocations used by bump-the-pointer may well make it underwhelming.

#23557 hopefully helps with that, but it needs a host compiler with a new druntime that has #23595

[rainers]

My PR didn't touch stageFlags, so can add on top of this by also using astNodeBitFields on other Expressions.

[rainers]

I made bitFields an ushort instead in #23519, but that causes a padding byte before it. Your versions allows accessing all 21 remaining bits and is probably simpler most of the time. "Appending" to an existing bitfield might be needed with mutliple derivation steps, but I don't remember actually requiring that.

[ibuclaw]

I think you can get around that with the bitOff parameter.

class Base {
    ushort padding;
}
class Derive1 : Base {
    struct BitFields { ubyte field; }
    mixin(generateBitFields!(BitFields, ushort, "padding");
}
class Derive2 : Derive1 {
    struct BitFields { bool flag; }
    mixin(generateBitFields!(BitFields, ushort, "padding", 8);
}

It's a bit awkward, and I guess none of this is actually unit-tested. :-)

[rainers]

I think you can get around that with the bitOff parameter.

That's what the appendBitfields mixin in my PR does, but also evaluates the offset automatically from the base class.

[ibuclaw]

@rainers FYI, these were the other two potential follow up commits.

• Similar to yours, but I think covers more classes? ibuclaw@dbb3f79
• Token.len is a uint, make StringExp.len uint too. ibuclaw@0b94975

[ibuclaw]

But I agree that the 16 byte allocations used by bump-the-pointer may well make it underwhelming.

#23557 hopefully helps with that, but it needs a host compiler with a new druntime that has #23595

I don't think that's much of a concern my end at least.

1. gdc release is always bootstrapped by itself twice (host -> stage1 -> stage2 -> stage3)
2. if there's a code path that fails with older host compiler, use static if (__VERSION__ < desired-version).

[ibuclaw]

I'll leave this in draft for ~72h, so that everyone has enough time to reflect. Also, so I can consider what's going on in #23519 too.

[rainers]

1. gdc release is always bootstrapped by itself twice (host -> stage1 -> stage2 -> stage3)

Unfortunately, dmd cannot do it ilke that, it has to be built with LDC or GDC to not increase compile times by about 30%.

2. if there's a code path that fails with older host compiler, use static if (__VERSION__ < desired-version).

Yes, the PR already does something similar, i.e. checks whether BlkAttr.ALIGNMENT_MASK exists.

[Herringway]

It's only a problem if the builds of LDC or GDC used for that never get updated again, which seems unlikely. It might just take a little while for things to improve.