diff --git a/include/slang/ast/ASTContext.h b/include/slang/ast/ASTContext.h index b08ae227d..a2295de35 100644 --- a/include/slang/ast/ASTContext.h +++ b/include/slang/ast/ASTContext.h @@ -193,9 +193,12 @@ enum class SLANG_EXPORT ASTFlags : uint64_t { DisallowUDNT = 1ull << 44, /// AST binding is for a bind instantiation (port connection or param value). - BindInstantiation = 1ull << 45 + BindInstantiation = 1ull << 45, + + // The expression is inside a sequence + InsideSequenceExpr = 1ull << 46 }; -SLANG_BITMASK(ASTFlags, BindInstantiation) +SLANG_BITMASK(ASTFlags, InsideSequenceExpr) // clang-format off #define DK(x) \ diff --git a/scripts/diagnostics.txt b/scripts/diagnostics.txt index 5433f4e43..e29b2da29 100644 --- a/scripts/diagnostics.txt +++ b/scripts/diagnostics.txt @@ -739,6 +739,8 @@ error VoidAssignment "expression of type 'void' cannot be used in assignments" error VirtualIfaceDefparam "interface instance cannot be assigned to a virtual interface because it is the target of a defparam or bind directive" error MultipleDefaultDistWeight "'default' cannot be used more than once in dist item list" error DistRealRangeWeight "'dist' for a range of real values must include an explicit weight and must use the ':/' operator" +error ContinuousAssignTriggered "sequence triggered method should not be used in the right-hand side of an continuous assignment" +error AlwaysCombAssignTriggered "sequence triggered method should not be used in the right-hand side of an assignment in an always_comb procedure" warning ignored-slice IgnoredSlice "slice size ignored for left-to-right streaming operator" warning unsized-concat UnsizedInConcat "unsized integer in concat; using a width of {}" warning width-expand WidthExpand "implicit conversion expands from {} to {} bits" @@ -838,6 +840,7 @@ error NestedDisableIff "cannot instantiate '{}' here because it has a disable if error GlobalSampledValueNested "global clocking future sampled value functions may not be nested" error DisableIffLocalVar "local variables cannot be used in disable iff conditions" error DisableIffMatched "'matched' method cannot be used in disable iff conditions" +error MatchedOutsideSeq "'matched' method should be used inside sequence expressions" error PropAbortLocalVar "local variables cannot be used in property abort conditions" error PropAbortMatched "'matched' method cannot be used in property abort conditions" error RecursivePropDisableIff "recursive properties cannot have disable iff conditions" @@ -1004,7 +1007,8 @@ error SeqMethodInputLocalVar "cannot call '{}' on a sequence that has 'input' or error SampledValueLocalVar "local variables cannot be used in sampled value system functions" error SampledValueMatched "'matched' method cannot be used in sampled value system functions" error GlobalSampledValueAssertionExpr "global clocking future sampled value functions may be invoked only in sequence and property expressions" -error SequenceMethodLocalVar "local variables can only be passed to sequences on which '{}' is called if they are the entire argument and not a sub-expression" +error TriggeredMethodLocalVar "local variables can only be passed to sequences on which 'triggered' is called if they are the entire argument and not a sub-expression" +error TriggeredMethodUseBeforeInit "arguments of sequences on which 'triggered' is called should be assigned after it is referenced" error PlaRangeInAscendingOrder "must specify dimensions in ascending order for PLA input type {}" error SysFuncHierarchicalNotAllowed "hierarchical references are not allowed in calls to '{}'" error TypeIsNotAClass "{} is not a class type" diff --git a/source/ast/builtins/MiscSystemFuncs.cpp b/source/ast/builtins/MiscSystemFuncs.cpp index 73bf648c4..287c677e8 100644 --- a/source/ast/builtins/MiscSystemFuncs.cpp +++ b/source/ast/builtins/MiscSystemFuncs.cpp @@ -276,12 +276,10 @@ class InferredValueFunction : public SystemSubroutine { bool isClockFunc; }; -struct SequenceMethodExprVisitor { +struct TriggeredMethodArgExprVisitor { const ASTContext& context; - std::string name; - SequenceMethodExprVisitor(const ASTContext& context, std::string name) : - context(context), name(std::move(name)) {} + TriggeredMethodArgExprVisitor(const ASTContext& context) : context(context) {} template void visit(const T& expr) { @@ -291,17 +289,45 @@ struct SequenceMethodExprVisitor { if (sym->kind == SymbolKind::LocalAssertionVar || (sym->kind == SymbolKind::AssertionPort && sym->template as().isLocalVar())) { - context.addDiag(diag::SequenceMethodLocalVar, expr.sourceRange) << name; + context.addDiag(diag::TriggeredMethodLocalVar, expr.sourceRange); } } } } - if constexpr (HasVisitExprs) + if constexpr (HasVisitExprs) expr.visitExprs(*this); } }; +struct TriggeredMethodSeqBodyVisitor + : public ASTVisitor { + const Symbol* arg = nullptr; + bool isAssignmentMatched = false; + SmallVector foundedSR; + + TriggeredMethodSeqBodyVisitor(const NamedValueExpression* nVE) : arg(&nVE->symbol) {} + + template + void visit(const T& expr) { + if constexpr (std::is_base_of_v) { + if (!isAssignmentMatched) + foundedSR.push_back(expr.sourceRange); + } + + if constexpr (std::is_base_of_v) { + if (const auto* lhs = expr.left().template as_if(); + lhs && &lhs->symbol == arg) { + isAssignmentMatched = true; + } + } + + if constexpr (HasVisitExprs) + expr.visitExprs(*this); + visitDefault(*this); + } +}; + class SequenceMethod : public SystemSubroutine { public: SequenceMethod(const std::string& name) : SystemSubroutine(name, SubroutineKind::Function) {} @@ -348,12 +374,17 @@ class SequenceMethod : public SystemSubroutine { } } + if (name != "triggered"sv) + return; + // Arguments to sequence instances that have triggered invoked can only // reference local variables if that is the entire argument. - SequenceMethodExprVisitor visitor(context, name); + TriggeredMethodArgExprVisitor visitor(context); for (auto& [formal, actualArg] : aie.arguments) { std::visit( - [&visitor](auto&& arg) { + [&visitor, &context, &aie](auto&& arg) { + const NamedValueExpression* validatedArg = nullptr; + // Local vars are allowed at the top level, so we need to check if // the entire argument is a named value and if so don't bother // checking it. @@ -361,12 +392,16 @@ class SequenceMethod : public SystemSubroutine { if constexpr (std::is_same_v) { if (arg->kind != ExpressionKind::NamedValue) arg->visit(visitor); + else + validatedArg = &arg->template as(); } else if constexpr (std::is_same_v) { if (arg->kind == AssertionExprKind::Simple) { auto& sae = arg->template as(); if (sae.repetition || sae.expr.kind != ExpressionKind::NamedValue) arg->visit(visitor); + else if (sae.expr.kind == ExpressionKind::NamedValue) + validatedArg = &sae.expr.template as(); } else { arg->visit(visitor); @@ -379,6 +414,9 @@ class SequenceMethod : public SystemSubroutine { sec.expr.kind != ExpressionKind::NamedValue) { arg->visit(visitor); } + else if (sec.expr.kind == ExpressionKind::NamedValue) { + validatedArg = &sec.expr.template as(); + } } else { arg->visit(visitor); @@ -387,6 +425,20 @@ class SequenceMethod : public SystemSubroutine { else { static_assert(always_false::value, "Missing case"); } + + // After we validate a named value argument then check that it is not + // referenced before assignment in sequence instance body on which triggered + // method is called. + if (validatedArg) { + TriggeredMethodSeqBodyVisitor seqBodyVisitor(validatedArg); + aie.body.visit(seqBodyVisitor); + + if (seqBodyVisitor.isAssignmentMatched) { + for (auto& sR : seqBodyVisitor.foundedSR) { + context.addDiag(diag::TriggeredMethodUseBeforeInit, sR); + } + } + } }, actualArg); } diff --git a/source/ast/expressions/AssertionExpr.cpp b/source/ast/expressions/AssertionExpr.cpp index 78fb14b46..c4a486ce6 100644 --- a/source/ast/expressions/AssertionExpr.cpp +++ b/source/ast/expressions/AssertionExpr.cpp @@ -524,7 +524,9 @@ void SequenceRepetition::serializeTo(ASTSerializer& serializer) const { AssertionExpr& SimpleAssertionExpr::fromSyntax(const SimpleSequenceExprSyntax& syntax, const ASTContext& context, bool allowDisable) { auto& comp = context.getCompilation(); - auto& expr = bindExpr(*syntax.expr, context, /* allowInstances */ true); + ASTContext ctx = context; + ctx.flags |= ASTFlags::InsideSequenceExpr; + auto& expr = bindExpr(*syntax.expr, ctx, /* allowInstances */ true); std::optional repetition; if (syntax.repetition) { diff --git a/source/ast/expressions/AssignmentExpressions.cpp b/source/ast/expressions/AssignmentExpressions.cpp index dae181471..b32f27795 100644 --- a/source/ast/expressions/AssignmentExpressions.cpp +++ b/source/ast/expressions/AssignmentExpressions.cpp @@ -8,6 +8,7 @@ #include "slang/ast/expressions/AssignmentExpressions.h" #include "slang/ast/ASTSerializer.h" +#include "slang/ast/ASTVisitor.h" #include "slang/ast/Bitstream.h" #include "slang/ast/Compilation.h" #include "slang/ast/EvalContext.h" @@ -17,6 +18,7 @@ #include "slang/ast/expressions/MiscExpressions.h" #include "slang/ast/expressions/OperatorExpressions.h" #include "slang/ast/expressions/SelectExpressions.h" +#include "slang/ast/symbols/BlockSymbols.h" #include "slang/ast/symbols/ClassSymbols.h" #include "slang/ast/symbols/CoverSymbols.h" #include "slang/ast/symbols/InstanceSymbols.h" @@ -26,6 +28,7 @@ #include "slang/diagnostics/ExpressionsDiags.h" #include "slang/diagnostics/LookupDiags.h" #include "slang/diagnostics/NumericDiags.h" +#include "slang/diagnostics/StatementsDiags.h" #include "slang/diagnostics/TypesDiags.h" #include "slang/numeric/MathUtils.h" #include "slang/syntax/AllSyntax.h" @@ -36,6 +39,32 @@ using namespace slang; using namespace slang::ast; using namespace slang::syntax; +struct TriggeredAssignmentVisitor { + const ASTContext& context; + DiagCode triggeredCode; + + TriggeredAssignmentVisitor(const ASTContext& context, DiagCode triggeredCode) : + context(context), triggeredCode(triggeredCode) {} + + template + void visit(const T& expr) { + if constexpr (std::is_base_of_v) { + if (expr.kind == ExpressionKind::Call) { + auto& call = expr.template as(); + if (call.isSystemCall()) { + if (call.getSubroutineName() == "triggered"sv && !call.arguments().empty() && + call.arguments()[0]->type->isSequenceType()) { + context.addDiag(triggeredCode, expr.sourceRange); + } + } + } + else if constexpr (HasVisitExprs) { + expr.visitExprs(*this); + } + } + } +}; + // This function exists to handle a case like: // integer i; // enum { A, B } foo; @@ -644,6 +673,17 @@ Expression& AssignmentExpression::fromSyntax(Compilation& compilation, } } + // Check that there is no sequence method triggered call in continuous assignment or + // assignment in always_comb block. + if ((context.getProceduralBlock() && + context.getProceduralBlock()->procedureKind == ProceduralBlockKind::AlwaysComb) || + context.flags.has(ASTFlags::NonProcedural)) { + bool isContinuous = context.flags.has(ASTFlags::NonProcedural); + TriggeredAssignmentVisitor tAV(context, (isContinuous) ? diag::ContinuousAssignTriggered + : diag::AlwaysCombAssignTriggered); + rhs->visit(tAV); + } + return fromComponents(compilation, op, assignFlags, lhs, *rhs, syntax.operatorToken.range(), timingControl, syntax.sourceRange(), context); } diff --git a/source/ast/expressions/CallExpression.cpp b/source/ast/expressions/CallExpression.cpp index 0e55b66f9..e2448a88d 100644 --- a/source/ast/expressions/CallExpression.cpp +++ b/source/ast/expressions/CallExpression.cpp @@ -20,6 +20,7 @@ #include "slang/diagnostics/ExpressionsDiags.h" #include "slang/diagnostics/LookupDiags.h" #include "slang/diagnostics/ParserDiags.h" +#include "slang/diagnostics/StatementsDiags.h" #include "slang/syntax/AllSyntax.h" namespace slang::ast { @@ -401,6 +402,9 @@ Expression& CallExpression::fromSystemMethod( return badExpr(compilation, &expr); } + if (subroutine->name == "matched"sv && !context.flags.has(ASTFlags::InsideSequenceExpr)) + context.addDiag(diag::MatchedOutsideSeq, expr.sourceRange); + return createSystemCall(compilation, *subroutine, &expr, syntax, withClause, syntax ? syntax->sourceRange() : expr.sourceRange, context); } diff --git a/tests/unittests/ast/AssertionTests.cpp b/tests/unittests/ast/AssertionTests.cpp index f5ee16c37..7022a189b 100644 --- a/tests/unittests/ast/AssertionTests.cpp +++ b/tests/unittests/ast/AssertionTests.cpp @@ -476,6 +476,21 @@ module m; int i; s5(i + 1, e, i + 1).triggered ##1 s5(i, e, i).triggered; endsequence + + sequence s7(lv); + (lv, lv = 1); + endsequence + + sequence s8; + int v1; + ##1 s7(v1).triggered ##1 (1 == v1); + endsequence + + sequence s9; + int v1; + ##1 s7(v1).matched ##1 (1 == v1); + endsequence + endmodule )"); @@ -483,10 +498,11 @@ endmodule compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 3); + REQUIRE(diags.size() == 4); CHECK(diags[0].code == diag::SeqMethodInputLocalVar); - CHECK(diags[1].code == diag::SequenceMethodLocalVar); - CHECK(diags[2].code == diag::SequenceMethodLocalVar); + CHECK(diags[1].code == diag::TriggeredMethodLocalVar); + CHECK(diags[2].code == diag::TriggeredMethodLocalVar); + CHECK(diags[3].code == diag::TriggeredMethodUseBeforeInit); } TEST_CASE("Sequence event control") { @@ -1004,9 +1020,10 @@ endmodule compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 2); + REQUIRE(diags.size() == 3); CHECK(diags[0].code == diag::DisableIffLocalVar); CHECK(diags[1].code == diag::DisableIffMatched); + CHECK(diags[2].code == diag::MatchedOutsideSeq); } TEST_CASE("abort property condition restrictions") { @@ -1028,9 +1045,10 @@ endmodule compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 2); + REQUIRE(diags.size() == 3); CHECK(diags[0].code == diag::PropAbortLocalVar); - CHECK(diags[1].code == diag::PropAbortMatched); + CHECK(diags[1].code == diag::MatchedOutsideSeq); + CHECK(diags[2].code == diag::PropAbortMatched); } TEST_CASE("Sequence properties with empty matches") { diff --git a/tests/unittests/ast/ExpressionTests.cpp b/tests/unittests/ast/ExpressionTests.cpp index 1c1f4c256..f2e7ab998 100644 --- a/tests/unittests/ast/ExpressionTests.cpp +++ b/tests/unittests/ast/ExpressionTests.cpp @@ -3644,3 +3644,32 @@ assign z = x & y; ~ ^ ~ )"); } + +TEST_CASE("Invalid sequence method triggered usage in assignments") { + auto tree = SyntaxTree::fromText(R"( +checker my_check(input logic clk, output logic a, output logic b); + sequence s; 1; endsequence + + assign b = s.triggered; // illegal + + always_comb a = s.triggered; // illegal + + reg c, d; + + always_latch c = s.triggered; // legal + always_ff @clk d <= s.triggered; // legal +endchecker + +module m(input logic clk, output logic a, output logic b); + my_check mc(clk, a, b); +endmodule +)"); + + Compilation compilation; + compilation.addSyntaxTree(tree); + + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 2); + CHECK(diags[0].code == diag::ContinuousAssignTriggered); + CHECK(diags[1].code == diag::AlwaysCombAssignTriggered); +} diff --git a/tests/unittests/ast/SystemFuncTests.cpp b/tests/unittests/ast/SystemFuncTests.cpp index d5c3536af..9ae7254e7 100644 --- a/tests/unittests/ast/SystemFuncTests.cpp +++ b/tests/unittests/ast/SystemFuncTests.cpp @@ -7,6 +7,7 @@ #include "slang/ast/symbols/CompilationUnitSymbols.h" #include "slang/ast/symbols/ParameterSymbols.h" #include "slang/ast/types/Type.h" +#include "slang/diagnostics/StatementsDiags.h" #include "slang/syntax/AllSyntax.h" TEST_CASE("I/O system tasks") { @@ -886,13 +887,14 @@ endmodule compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 6); + REQUIRE(diags.size() == 7); CHECK(diags[0].code == diag::PastNumTicksInvalid); CHECK(diags[1].code == diag::NoGlobalClocking); - CHECK(diags[2].code == diag::SampledValueMatched); - CHECK(diags[3].code == diag::SampledValueLocalVar); - CHECK(diags[4].code == diag::GlobalSampledValueAssertionExpr); - CHECK(diags[5].code == diag::GlobalSampledValueNested); + CHECK(diags[2].code == diag::MatchedOutsideSeq); + CHECK(diags[3].code == diag::SampledValueMatched); + CHECK(diags[4].code == diag::SampledValueLocalVar); + CHECK(diags[5].code == diag::GlobalSampledValueAssertionExpr); + CHECK(diags[6].code == diag::GlobalSampledValueNested); } TEST_CASE("Global clock sys func") { @@ -1473,3 +1475,35 @@ endmodule REQUIRE(diags.size() == 1); CHECK(diags[0].code == diag::FormatMismatchedType); } + +TEST_CASE("matched method outside sequence") { + auto tree = SyntaxTree::fromText(R"( +module m(); + +sequence e1; + 1; +endsequence + +sequence e2; + 1; +endsequence + +initial begin + wait (e1.matched || e2.matched); // violation + if (e1.matched) // violation + $display("e1 passed"); + if (e2.matched) // violation + $display("e2 passed"); +end +endmodule +)"); + + Compilation compilation; + compilation.addSyntaxTree(tree); + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 4); + CHECK(diags[0].code == diag::MatchedOutsideSeq); + CHECK(diags[1].code == diag::MatchedOutsideSeq); + CHECK(diags[2].code == diag::MatchedOutsideSeq); + CHECK(diags[3].code == diag::MatchedOutsideSeq); +}