perf(ast): read local variables in place when evaluating selects

include/slang/ast/expressions/MiscExpressions.h

@@ -60,6 +60,12 @@ class SLANG_EXPORT NamedValueExpression final : public ValueExpressionBase {
     LValue evalLValueImpl(EvalContext& context) const;
     bool isEquivalentImpl(const NamedValueExpression& rhs) const;
 
+    /// If this expression names a local variable of the current call frame,
+    /// evaluates it without copying the value: returns a pointer to the
+    /// variable's current value, or to an invalid value if the evaluation
+    /// failed. Returns nullptr, having evaluated nothing, for any other symbol.
+    const ConstantValue* evalLocalRef(EvalContext& context) const;
+
     static bool isKind(ExpressionKind kind) { return kind == ExpressionKind::NamedValue; }
 
 private:

source/ast/expressions/MiscExpressions.cpp

@@ -385,6 +385,29 @@ ConstantValue NamedValueExpression::evalImpl(EvalContext& context) const {
     return nullptr;
 }
 
+const ConstantValue* NamedValueExpression::evalLocalRef(EvalContext& context) const {
+    if (getConstant() || bad())
+        return nullptr;
+
+    switch (symbol.kind) {
+        case SymbolKind::Parameter:
+        case SymbolKind::EnumValue:
+        case SymbolKind::Specparam:
+            return nullptr;
+        default:
+            break;
+    }
+
+    ConstantValue* v = context.findLocal(&symbol);
+    if (!v)
+        return nullptr;
+
+    if (!checkConstant(context))
+        return &ConstantValue::Invalid;
+
+    return v;
+}
+
 LValue NamedValueExpression::evalLValueImpl(EvalContext& context) const {
     if (!checkConstant(context))
         return nullptr;

source/ast/expressions/SelectExpressions.cpp

@@ -13,8 +13,10 @@
 #include "slang/ast/Lookup.h"
 #include "slang/ast/TypeProvider.h"
 #include "slang/ast/expressions/CallExpression.h"
+#include "slang/ast/expressions/ConversionExpression.h"
 #include "slang/ast/expressions/LiteralExpressions.h"
 #include "slang/ast/expressions/MiscExpressions.h"
+#include "slang/ast/expressions/OperatorExpressions.h"
 #include "slang/ast/symbols/ClassSymbols.h"
 #include "slang/ast/symbols/CoverSymbols.h"
 #include "slang/ast/symbols/SubroutineSymbols.h"
@@ -211,8 +213,71 @@ bool ElementSelectExpression::isConstantSelect(EvalContext& context) const {
     return value().type->hasFixedRange() && !selector().eval(context).bad();
 }
 
+// True if evaluating the expression can't write to any variable.
+static bool isSideEffectFree(const Expression& expr) {
+    switch (expr.kind) {
+        case ExpressionKind::IntegerLiteral:
+        case ExpressionKind::RealLiteral:
+        case ExpressionKind::TimeLiteral:
+        case ExpressionKind::UnbasedUnsizedIntegerLiteral:
+        case ExpressionKind::NullLiteral:
+        case ExpressionKind::UnboundedLiteral:
+        case ExpressionKind::StringLiteral:
+        case ExpressionKind::NamedValue:
+            return true;
+        case ExpressionKind::UnaryOp: {
+            auto& unary = expr.as<UnaryExpression>();
+            switch (unary.op) {
+                case UnaryOperator::Preincrement:
+                case UnaryOperator::Predecrement:
+                case UnaryOperator::Postincrement:
+                case UnaryOperator::Postdecrement:
+                    return false;
+                default:
+                    return isSideEffectFree(unary.operand());
+            }
+        }
+        case ExpressionKind::BinaryOp: {
+            auto& binary = expr.as<BinaryExpression>();
+            return isSideEffectFree(binary.left()) && isSideEffectFree(binary.right());
+        }
+        case ExpressionKind::Conversion:
+            return isSideEffectFree(expr.as<ConversionExpression>().operand());
+        case ExpressionKind::ElementSelect: {
+            auto& select = expr.as<ElementSelectExpression>();
+            return isSideEffectFree(select.value()) && isSideEffectFree(select.selector());
+        }
+        case ExpressionKind::RangeSelect: {
+            auto& select = expr.as<RangeSelectExpression>();
+            return isSideEffectFree(select.value()) && isSideEffectFree(select.left()) &&
+                   isSideEffectFree(select.right());
+        }
+        case ExpressionKind::MemberAccess:
+            return isSideEffectFree(expr.as<MemberAccessExpression>().value());
+        default:
+            return false;
+    }
+}
+
+// Evaluates the value operand of a select. A local variable is read in place
+// rather than copied whole, unless one of the selectors evaluated after it
+// could write to it.
+static const ConstantValue* evalSelectValue(const Expression& value,
+                                            std::initializer_list<const Expression*> selectors,
+                                            EvalContext& context, ConstantValue& storage) {
+    if (value.kind == ExpressionKind::NamedValue &&
+        std::ranges::all_of(selectors, [](auto sel) { return isSideEffectFree(*sel); })) {
+        if (auto ref = value.as<NamedValueExpression>().evalLocalRef(context))
+            return ref;
+    }
+
+    storage = value.eval(context);
+    return &storage;
+}
+
 ConstantValue ElementSelectExpression::evalImpl(EvalContext& context) const {
-    ConstantValue cv = value().eval(context);
+    ConstantValue storage;
+    auto& cv = *evalSelectValue(value(), {&selector()}, context, storage);
     if (!cv)
         return nullptr;
 
@@ -257,7 +322,7 @@ ConstantValue ElementSelectExpression::evalImpl(EvalContext& context) const {
     if (valType.isString())
         return cv.getSlice(range->left, range->right, nullptr);
 
-    return std::move(cv).at(size_t(range->left));
+    return cv.at(size_t(range->left));
 }
 
 LValue ElementSelectExpression::evalLValueImpl(EvalContext& context) const {
@@ -670,7 +735,8 @@ bool RangeSelectExpression::isConstantSelect(EvalContext& context) const {
 }
 
 ConstantValue RangeSelectExpression::evalImpl(EvalContext& context) const {
-    ConstantValue cv = value().eval(context);
+    ConstantValue storage;
+    auto& cv = *evalSelectValue(value(), {&left(), &right()}, context, storage);
     if (!cv)
         return nullptr;
 
@@ -1206,7 +1272,8 @@ static bool checkPackedUnionTag(const Type& valueType, const SVInt& val, uint32_
 }
 
 ConstantValue MemberAccessExpression::evalImpl(EvalContext& context) const {
-    ConstantValue cv = value().eval(context);
+    ConstantValue storage;
+    auto& cv = *evalSelectValue(value(), {}, context, storage);
     if (!cv)
         return nullptr;
 
@@ -1256,10 +1323,10 @@ ConstantValue MemberAccessExpression::evalImpl(EvalContext& context) const {
     else {
         int32_t io = (int32_t)field.bitOffset;
         int32_t width = (int32_t)type->getBitWidth();
-        cv = cv.integer().slice(width + io - 1, io);
+        ConstantValue result = cv.integer().slice(width + io - 1, io);
 
         // Make sure sign and four-statedness are correct.
-        return cv.convertToInt(type->getBitWidth(), type->isSigned(), type->isFourState());
+        return result.convertToInt(type->getBitWidth(), type->isSigned(), type->isFourState());
     }
 }
 

tests/unittests/ast/EvalTests.cpp

@@ -2684,3 +2684,61 @@ endfunction
 
     NO_SESSION_ERRORS;
 }
+
+TEST_CASE("Eval selects of local variables") {
+    ScriptSession session;
+    session.eval(R"(
+function automatic int sum_unpacked;
+    int a[8];
+    int b[2];
+    int s = 0;
+    for (int i = 0; i < 8; i++) a[i] = i * 3;
+    for (int i = 0; i < 8; i++) s += a[i] + a[7 - i];
+    b = a[2:3];
+    return s + b[1];
+endfunction
+)");
+    CHECK(session.eval("sum_unpacked()").integer() == 177);
+
+    session.eval(R"(
+function automatic logic [5:0] packed_bits;
+    logic [3:0][7:0] p = {8'h12, 8'h34, 8'h56, 8'h78};
+    return {p[3][4], p[2][1], p[1][7:4]};
+endfunction
+)");
+    CHECK(session.eval("packed_bits()").integer() == 37);
+
+    session.eval(R"(
+typedef struct { int x; int y[2]; } s_t;
+function automatic int struct_member;
+    s_t s;
+    s.x = 5;
+    s.y[1] = 7;
+    return s.x * s.y[1];
+endfunction
+)");
+    CHECK(session.eval("struct_member()").integer() == 35);
+
+    // The value is read before the selector's side effect.
+    session.eval(R"(
+function automatic int side_effect_select;
+    int a[4] = '{0, 20, 30, 40};
+    return a[a[0]++];
+endfunction
+)");
+    CHECK(session.eval("side_effect_select()").integer() == 0);
+
+    session.eval(R"(
+function automatic int oob_select;
+    int a[4] = '{1, 2, 3, 4};
+    int i = 9;
+    return a[i];
+endfunction
+)");
+    CHECK(session.eval("oob_select()").integer() == 0);
+
+    auto diags = session.getDiagnostics();
+    INFO(report(diags));
+    REQUIRE(diags.size() == 1);
+    CHECK(diags[0].code == diag::IndexOOB);
+}