From 8108e8a7608ef9c5c02b2e8923e5afda5c4a73c4 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 28 Sep 2025 00:02:21 +0200 Subject: [PATCH] libexpr: tag Value::Acb Value is already tagged. Value::Acb blocks are allocated for lambdas (so we can fit the value tag into the three bits we have available), but the current layout is rather wasteful for this purpose. the type bits can be stored together with parts of pointers, which in the lambda case will be the scope the lambda captures. the expr could also be used, but Env is a gc-allocated item and thus guaranteed to be aligned properly for tagging Change-Id: Ia685875387c7795bc4a00d73d1ce3cfea84e7297 --- lix/libexpr/eval-inline.hh | 2 +- lix/libexpr/eval.cc | 16 ++++++--- lix/libexpr/eval.hh | 2 +- lix/libexpr/value.cc | 7 ++++ lix/libexpr/value.hh | 73 +++++++++++++++++++++++++++----------- 5 files changed, 73 insertions(+), 27 deletions(-) diff --git a/lix/libexpr/eval-inline.hh b/lix/libexpr/eval-inline.hh index b3f1a7a85..3b23c6593 100644 --- a/lix/libexpr/eval-inline.hh +++ b/lix/libexpr/eval-inline.hh @@ -38,7 +38,7 @@ inline Value::Value(thunk_t, EvalMemory & mem, Env & env, Expr & expr) inline Value::Value(lambda_t, EvalMemory & mem, Env & env, ExprLambda & lambda) { auto lp = mem.allocType(); - *lp = Lambda{{Acb::tLambda}, &env, &lambda}; + new (lp) Lambda{env, lambda}; raw = tag(tAuxiliary, lp); } diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index a941ba723..870e2ab40 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -118,7 +118,7 @@ std::string showType(const Value & v) case tAuxiliary: #pragma GCC diagnostic push #pragma GCC diagnostic error "-Wswitch-enum" - switch (v.auxiliary()->type) { + switch (v.auxiliary()->type()) { case Value::Acb::tExternal: return v.external()->showType(); case Value::Acb::tFloat: @@ -1624,7 +1624,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, ExprLambda & lambda(*vCur.lambda().fun); - Env & env2 = lambda.pattern->match(lambda, *this, *vCur.lambda().env, args[0], pos); + Env & env2 = lambda.pattern->match(lambda, *this, *vCur.lambda().env(), args[0], pos); ctx.stats.nrFunctionCalls++; if (ctx.stats.countCalls) ctx.stats.addCall(lambda); @@ -1821,12 +1821,18 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res, PosI if (j) { attrs.insert(*j); } else if (!i.def) { - ctx.errors.make(R"(cannot evaluate a function that has an argument without a value ('%1%') + ctx.errors + .make( + R"(cannot evaluate a function that has an argument without a value ('%1%') Lix attempted to evaluate a function as a top level expression; in this case it must have its arguments supplied either by default values, or passed explicitly with '--arg' or '--argstr'. See -https://docs.lix.systems/manual/lix/stable/language/constructs.html#functions)", ctx.symbols[i.name]) - .atPos(i.pos).withFrame(*fun.lambda().env, *fun.lambda().fun).debugThrow(); +https://docs.lix.systems/manual/lix/stable/language/constructs.html#functions)", + ctx.symbols[i.name] + ) + .atPos(i.pos) + .withFrame(*fun.lambda().env(), *fun.lambda().fun) + .debugThrow(); } } } diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index 505ef0386..c714c4098 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -63,7 +63,7 @@ struct Constant using ValMap = GcMap; -struct Env +struct alignas(Value::Acb::TAG_ALIGN) Env { Env * up; Value * values[0]; diff --git a/lix/libexpr/value.cc b/lix/libexpr/value.cc index d6129dbae..5224d2aaa 100644 --- a/lix/libexpr/value.cc +++ b/lix/libexpr/value.cc @@ -20,6 +20,13 @@ static_assert(alignof(Value::List) >= Value::TAG_ALIGN); static_assert(alignof(Value::Thunk) >= Value::TAG_ALIGN); static_assert(alignof(Value::App) >= Value::TAG_ALIGN); +static_assert(alignof(Value::External) >= Value::Acb::TAG_ALIGN); +static_assert(alignof(Value::Float) >= Value::Acb::TAG_ALIGN); +static_assert(alignof(Value::Null) >= Value::Acb::TAG_ALIGN); +static_assert(alignof(Value::PrimOp) >= Value::Acb::TAG_ALIGN); +static_assert(alignof(Value::Int) >= Value::Acb::TAG_ALIGN); +static_assert(alignof(Value::Lambda) >= Value::Acb::TAG_ALIGN); + static void copyContextToValue(Value::String & s, const NixStringContext & context) { if (!context.empty()) { diff --git a/lix/libexpr/value.hh b/lix/libexpr/value.hh index e460ff538..68de3d32d 100644 --- a/lix/libexpr/value.hh +++ b/lix/libexpr/value.hh @@ -348,7 +348,7 @@ public: raw = tInt | (uintptr_t(i.value) << TAG_BITS); } else { auto ip = gcAllocType(); - ip->type = Acb::tInt; + ip->raw = Acb::tInt; ip->value = i; raw = tag(tAuxiliary, ip); } @@ -359,7 +359,7 @@ public: Value(floating_t, NixFloat f) { auto fp = gcAllocType(); - fp->type = Acb::tFloat; + fp->raw = Acb::tFloat; fp->value = f; raw = tag(tAuxiliary, fp); } @@ -540,7 +540,7 @@ public: Value(external_t, ExternalValueBase & external) { auto ext = gcAllocType(); - ext->type = Acb::tExternal; + ext->raw = Acb::tExternal; ext->external = &external; raw = tag(tAuxiliary, ext); } @@ -605,11 +605,11 @@ public: // type() == nFunction inline bool isLambda() const { - return internalType() == tAuxiliary && auxiliary()->type == Acb::tLambda; + return internalType() == tAuxiliary && auxiliary()->type() == Acb::tLambda; }; inline bool isPrimOp() const { - return internalType() == tAuxiliary && auxiliary()->type == Acb::tPrimOp; + return internalType() == tAuxiliary && auxiliary()->type() == Acb::tPrimOp; } inline bool isPrimOpApp() const { @@ -712,14 +712,48 @@ public: /// these blocks are usually heap-allocated in GC memory space. struct alignas(TAG_ALIGN) Acb { - enum { + // NOTE value.cc contains alignment assertions for pointers tagged thusly. + // *always* ensure that these assertions match the tag types declared here + enum Type { tExternal, tFloat, tNull, tPrimOp, tLambda, tInt, - } type; + }; + + uintptr_t raw; + + static constexpr size_t TAG_BITS = 3; + static constexpr size_t TAG_ALIGN = 1 << TAG_BITS; + static constexpr uintptr_t TAG_MASK = (1 << TAG_BITS) - 1; + + static uintptr_t tag(Type t, auto v) + { + if constexpr (std::is_null_pointer_v) { + return t; + } else if constexpr (std::is_pointer_v) { + return (reinterpret_cast(v)) | t; + } else { + return (static_cast(v) << TAG_BITS) | t; + } + } + + template + T untag() const + { + if constexpr (std::is_pointer_v) { + return reinterpret_cast(raw & ~TAG_MASK); + } else { + return static_cast((raw & ~TAG_MASK) >> TAG_BITS); + } + } + + Type type() const + { + return Type(raw & TAG_MASK); + } }; struct External : Acb { @@ -742,8 +776,14 @@ public: struct Lambda : Acb { - Env * env; ExprLambda * fun; + + Lambda(Env & env, ExprLambda & fun) : Acb{tag(tLambda, &env)}, fun(&fun) {} + + Env * env() const + { + return untag(); + } }; /** @@ -885,7 +925,7 @@ public: memcpy(&tmp, &raw, sizeof(tmp)); return NixInt(tmp >> 3); } else { - assert(internalType() == tAuxiliary && untag()->type == Acb::tInt); + assert(internalType() == tAuxiliary && untag()->type() == Acb::tInt); return untag()->value; } } @@ -922,19 +962,19 @@ public: const PrimOp * primOp() const { - assert(internalType() == tAuxiliary && untag()->type == Acb::tPrimOp); + assert(internalType() == tAuxiliary && untag()->type() == Acb::tPrimOp); return untag(); } const ExternalValueBase * external() const { - assert(internalType() == tAuxiliary && untag()->type == Acb::tExternal); + assert(internalType() == tAuxiliary && untag()->type() == Acb::tExternal); return untag()->external; } NixFloat fpoint() const { - assert(internalType() == tAuxiliary && untag()->type == Acb::tFloat); + assert(internalType() == tAuxiliary && untag()->type() == Acb::tFloat); return untag()->value; } @@ -974,13 +1014,6 @@ struct alignas(Value::TAG_ALIGN) Value::Thunk } }; -/** - * Returns the normal type of a Value. This only returns nThunk if - * the Value hasn't been forceValue'd - * - * @param invalidIsThunk Instead of aborting an an invalid (probably - * 0, so uninitialized) internal type, return `nThunk`. - */ inline ValueType Value::type(bool invalidIsThunk) const { again: @@ -996,7 +1029,7 @@ again: case tList: return nList; case tAuxiliary: - switch (untag()->type) { + switch (untag()->type()) { case Acb::tExternal: return nExternal; case Acb::tFloat: