Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ibuclaw wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions compiler/include/dmd/expression.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ class Expression : public ASTNode
Type *type; // !=NULL means that semantic() has been run
Loc loc; // file location
EXP op; // to minimize use of dynamic_cast
private:
uint8_t bitFields;
uint16_t astNodeBitFields;

@rainers rainers Aug 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ibuclaw Aug 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 rainers Aug 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

public:

bool parens() const;
bool parens(bool v);
Expand Down Expand Up @@ -397,8 +400,6 @@ class AssocArrayLiteralExp final : public Expression
class StructLiteralExp final : public Expression
{
public:
uint8_t bitFields;

// if this is true, use the StructDeclaration's init symbol
bool useStaticInit() const;
bool useStaticInit(bool v);
Expand All @@ -413,7 +414,8 @@ class StructLiteralExp final : public Expression
* 'inlinecopy' uses similar 'stageflags' and from multiple evaluation 'doInline'
* (with infinite recursion) of this expression.
*/
uint8_t stageflags;
uint8_t stageflags() const;
uint8_t stageflags(uint8_t v);

StructDeclaration *sd; // which aggregate this is for
Expressions *elements; // parallels sd->fields[] with NULL entries for fields to skip
Expand Down Expand Up @@ -882,10 +884,7 @@ class SliceExp final : public UnaExp
bool lowerIsLessThanUpper(bool v);
bool arrayop() const; // an array operation, rather than a slice
bool arrayop(bool v);
private:
uint8_t bitFields;

public:
SliceExp *syntaxCopy() override;

void accept(Visitor *v) override { v->visit(this); }
Expand Down
9 changes: 3 additions & 6 deletions compiler/src/dmd/dinterpret.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6568,8 +6568,7 @@ private Expression scrubReturnValue(Loc loc, Expression e)
sle.ownedByCtfe = OwnedBy.code;
if (!(sle.stageflags & StructLiteralExp.StageFlags.scrub))
{
const old = sle.stageflags;
sle.stageflags |= StructLiteralExp.StageFlags.scrub; // prevent infinite recursion
const old = sle.setStageFlag(StructLiteralExp.StageFlags.scrub); // prevent infinite recursion
if (auto ex = scrubArray(sle.elements, true))
return ex;
sle.stageflags = old;
Expand Down Expand Up @@ -6648,8 +6647,7 @@ private Expression scrubCacheValue(Expression e)
sle.ownedByCtfe = OwnedBy.cache;
if (!(sle.stageflags & StructLiteralExp.StageFlags.scrub))
{
const old = sle.stageflags;
sle.stageflags |= StructLiteralExp.StageFlags.scrub; // prevent infinite recursion
const old = sle.setStageFlag(StructLiteralExp.StageFlags.scrub); // prevent infinite recursion
if (auto ex = scrubArrayCache(sle.elements))
return ex;
sle.stageflags = old;
Expand Down Expand Up @@ -6725,8 +6723,7 @@ private Expression copyRegionExp(Expression e)
{
if (1 || !(sle.stageflags & StructLiteralExp.StageFlags.scrub))
{
const old = sle.stageflags;
sle.stageflags |= StructLiteralExp.StageFlags.scrub; // prevent infinite recursion
const old = sle.setStageFlag(StructLiteralExp.StageFlags.scrub); // prevent infinite recursion
copyArray(sle.elements);
sle.stageflags = old;
}
Expand Down
24 changes: 21 additions & 3 deletions compiler/src/dmd/expression.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ extern (C++) abstract class Expression : ASTNode
import dmd.common.bitfields;
mixin(generateBitFields!(BitFields, ubyte));

// This is the remaining padding between Expression and all of its derived
// classes, it is used as a space to hold per-expression bitfields, flags,
// or a value of these derived AST nodes to save on space.
private ushort astNodeBitFields;

extern (D) this(Loc loc, EXP op) scope @safe
{
//printf("Expression::Expression(op = %d) this = %p\n", op, this);
Expand Down Expand Up @@ -1441,10 +1446,10 @@ extern (C++) final class StructLiteralExp : Expression
bool useStaticInit; /// if this is true, use the StructDeclaration's init symbol
bool isOriginal = false; /// used when moving instances to indicate `this is this.origin`
OwnedBy ownedByCtfe = OwnedBy.code;
StageFlags stageflags;
}
import dmd.common.bitfields;
mixin(generateBitFields!(BitFields, ubyte));
StageFlags stageflags;
mixin(generateBitFields!(BitFields, ushort, "astNodeBitFields"));

StructDeclaration sd; /// which aggregate this is for
Expressions* elements; /// parallels sd.fields[] with null entries for fields to skip
Expand Down Expand Up @@ -1508,6 +1513,19 @@ extern (C++) final class StructLiteralExp : Expression
return exp;
}

/** Mark `stageflags` with the bit `flag`
* Params:
* flag = StageFlag to set
* Returns:
* The previous value of `stageflags`
*/
extern (D) StageFlags setStageFlag(StageFlags flag)

@ibuclaw ibuclaw Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
const old = stageflags;
stageflags = StageFlags(old | flag);
return old;
}

override void accept(Visitor v)
{
v.visit(this);
Expand Down Expand Up @@ -2730,7 +2748,7 @@ extern (C++) final class SliceExp : UnaExp
bool arrayop; // an array operation, rather than a slice
}
import dmd.common.bitfields : generateBitFields;
mixin(generateBitFields!(BitFields, ubyte));
mixin(generateBitFields!(BitFields, ushort, "astNodeBitFields"));

/************************************************************/
extern (D) this(Loc loc, Expression e1, IntervalExp ie) @safe
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dmd/hdrgen.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2482,8 +2482,7 @@ private void expressionPrettyPrint(Expression e, ref OutBuffer buf, ref HdrGenSt
buf.put("<recursion>");
else
{
const old = e.stageflags;
e.stageflags |= StructLiteralExp.StageFlags.toCBuffer;
const old = e.setStageFlag(StructLiteralExp.StageFlags.toCBuffer);
argsToBuffer(e.elements, buf, hgs);
e.stageflags = old;
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dmd/initsem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1670,8 +1670,7 @@ private bool hasNonConstPointers(Expression e)
{
if (!(se.stageflags & StructLiteralExp.StageFlags.searchPointers))
{
const old = se.stageflags;
se.stageflags |= StructLiteralExp.StageFlags.searchPointers;
const old = se.setStageFlag(StructLiteralExp.StageFlags.searchPointers);
bool ret = checkArray(se.elements);
se.stageflags = old;
return ret;
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dmd/inline.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1735,8 +1735,7 @@ public:
//printf("StructLiteralExp.inlineScan()\n");
if (e.stageflags & StructLiteralExp.StageFlags.inlineScan)
return;
const old = e.stageflags;
e.stageflags |= StructLiteralExp.StageFlags.inlineScan;
const old = e.setStageFlag(StructLiteralExp.StageFlags.inlineScan);
arrayInlineScan(e.elements);
e.stageflags = old;
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dmd/optimize.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,7 @@ Expression optimize(Expression e, int result, bool keepLvalue = false)
{
if (e.stageflags & StructLiteralExp.StageFlags.optimize)
return;
const old = e.stageflags;
e.stageflags |= StructLiteralExp.StageFlags.optimize;
const old = e.setStageFlag(StructLiteralExp.StageFlags.optimize);
if (e.elements)
{
foreach (ref ex; (*e.elements)[])
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dmd/visitor/package.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ extern (C++) class SemanticTimeTransitiveVisitor : SemanticTimePermissiveVisitor
alias flag = ASTCodegen.StructLiteralExp.StageFlags.toCBuffer;
if (!(e.stageflags & flag))
{
const old = e.stageflags;
e.stageflags |= flag;
const old = e.setStageFlag(flag);
foreach (el; *e.elements)
if (el)
el.accept(this);
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dmd/visitor/postorder.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ public:
{
if (e.stageflags & StructLiteralExp.StageFlags.apply)
return;
const old = e.stageflags;
e.stageflags |= StructLiteralExp.StageFlags.apply;
const old = e.setStageFlag(StructLiteralExp.StageFlags.apply);
doCond(e.elements.peekSlice()) || applyTo(e);
e.stageflags = old;
}
Expand Down
Loading