diff --git a/include/slang/ast/expressions/MiscExpressions.h b/include/slang/ast/expressions/MiscExpressions.h index 850ddafe3f..2326114444 100644 --- a/include/slang/ast/expressions/MiscExpressions.h +++ b/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: diff --git a/source/ast/expressions/MiscExpressions.cpp b/source/ast/expressions/MiscExpressions.cpp index 6a39bae363..746ecb3c9d 100644 --- a/source/ast/expressions/MiscExpressions.cpp +++ b/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; diff --git a/source/ast/expressions/SelectExpressions.cpp b/source/ast/expressions/SelectExpressions.cpp index 7c35a700da..6a3be3da99 100644 --- a/source/ast/expressions/SelectExpressions.cpp +++ b/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(); + 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(); + return isSideEffectFree(binary.left()) && isSideEffectFree(binary.right()); + } + case ExpressionKind::Conversion: + return isSideEffectFree(expr.as().operand()); + case ExpressionKind::ElementSelect: { + auto& select = expr.as(); + return isSideEffectFree(select.value()) && isSideEffectFree(select.selector()); + } + case ExpressionKind::RangeSelect: { + auto& select = expr.as(); + return isSideEffectFree(select.value()) && isSideEffectFree(select.left()) && + isSideEffectFree(select.right()); + } + case ExpressionKind::MemberAccess: + return isSideEffectFree(expr.as().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 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().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()); } } diff --git a/tests/unittests/ast/EvalTests.cpp b/tests/unittests/ast/EvalTests.cpp index f9921bd7ed..d8bf59e505 100644 --- a/tests/unittests/ast/EvalTests.cpp +++ b/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); +}