diff --git a/bindings/python/CompBindings.cpp b/bindings/python/CompBindings.cpp index 16a7c66eb..9ae2d9900 100644 --- a/bindings/python/CompBindings.cpp +++ b/bindings/python/CompBindings.cpp @@ -53,7 +53,9 @@ void registerCompilation(py::module_& m) { .value("IgnoreUnknownModules", CompilationFlags::IgnoreUnknownModules) .value("RelaxStringConversions", CompilationFlags::RelaxStringConversions) .value("AllowRecursiveImplicitCall", CompilationFlags::AllowRecursiveImplicitCall) - .value("AllowBareValParamAssignment", CompilationFlags::AllowBareValParamAssignment); + .value("AllowBareValParamAssignment", CompilationFlags::AllowBareValParamAssignment) + .value("AllowSelfDeterminedStreamConcat", + CompilationFlags::AllowSelfDeterminedStreamConcat); py::class_(m, "CompilationOptions") .def(py::init<>()) diff --git a/docs/command-line-ref.dox b/docs/command-line-ref.dox index c6767959e..9ec63b6ba 100644 --- a/docs/command-line-ref.dox +++ b/docs/command-line-ref.dox @@ -331,6 +331,12 @@ Allow module instantiations to provide a single parameter value assignment witho in parentheses. SystemVerilog doesn't permit this, but some tools allow it anyway so this flag can be used for compatibility purposes. +`--allow-self-determined-stream-concat` + +Allow self-determined streaming concatenation expressions. SystemVerilog only permits these +expressions in assignment-like contexts but some tools allow it anyway so this flag can be +used for compatibility purposes. + `--strict-driver-checking` Perform strict driver checking, which currently means disabling procedural 'for' @ref loop-unroll diff --git a/include/slang/ast/Compilation.h b/include/slang/ast/Compilation.h index bbd36547b..3a18702bf 100644 --- a/include/slang/ast/Compilation.h +++ b/include/slang/ast/Compilation.h @@ -117,9 +117,13 @@ enum class SLANG_EXPORT CompilationFlags { AllowRecursiveImplicitCall = 1 << 10, /// Allow module parameter assignments to elide the parentheses. - AllowBareValParamAssignment = 1 << 11 + AllowBareValParamAssignment = 1 << 11, + + /// Allow self-determined streaming concatenation expressions; normally these + /// can only be used in specific assignment-like contexts. + AllowSelfDeterminedStreamConcat = 1 << 12 }; -SLANG_BITMASK(CompilationFlags, AllowBareValParamAssignment) +SLANG_BITMASK(CompilationFlags, AllowSelfDeterminedStreamConcat) /// Contains various options that can control compilation behavior. struct SLANG_EXPORT CompilationOptions { diff --git a/include/slang/ast/expressions/OperatorExpressions.h b/include/slang/ast/expressions/OperatorExpressions.h index 73d42ba90..04b469d1c 100644 --- a/include/slang/ast/expressions/OperatorExpressions.h +++ b/include/slang/ast/expressions/OperatorExpressions.h @@ -288,7 +288,7 @@ class SLANG_EXPORT StreamingConcatenationExpression : public Expression { static Expression& fromSyntax(Compilation& compilation, const syntax::StreamingConcatenationExpressionSyntax& syntax, - const ASTContext& context, const Type* assignmentTarget); + const ASTContext& context); static bool isKind(ExpressionKind kind) { return kind == ExpressionKind::Streaming; } diff --git a/source/ast/Expression.cpp b/source/ast/Expression.cpp index 863e48837..2658eb765 100644 --- a/source/ast/Expression.cpp +++ b/source/ast/Expression.cpp @@ -849,8 +849,7 @@ Expression& Expression::create(Compilation& compilation, const ExpressionSyntax& break; case SyntaxKind::StreamingConcatenationExpression: result = &StreamingConcatenationExpression::fromSyntax( - compilation, syntax.as(), context, - assignmentTarget); + compilation, syntax.as(), context); break; case SyntaxKind::ElementSelectExpression: result = &bindSelectExpression(compilation, syntax.as(), diff --git a/source/ast/expressions/OperatorExpressions.cpp b/source/ast/expressions/OperatorExpressions.cpp index 34ea918ce..245f7e7d4 100644 --- a/source/ast/expressions/OperatorExpressions.cpp +++ b/source/ast/expressions/OperatorExpressions.cpp @@ -1751,13 +1751,9 @@ void ReplicationExpression::serializeTo(ASTSerializer& serializer) const { Expression& StreamingConcatenationExpression::fromSyntax( Compilation& comp, const StreamingConcatenationExpressionSyntax& syntax, - const ASTContext& context, const Type* assignmentTarget) { + const ASTContext& context) { - // The sole purpose of assignmentTarget here is to know whether this is a - // "destination" stream (i.e. an unpack operation) or whether it is a source / pack. - // Streaming concatenation is self-determined and its size/type should not be affected - // by assignmentTarget. - const bool isDestination = !assignmentTarget; + const bool isDestination = context.flags.has(ASTFlags::LValue); const bool isRightToLeft = syntax.operatorToken.kind == TokenKind::LeftShift; uint64_t sliceSize = 0; @@ -1767,7 +1763,8 @@ Expression& StreamingConcatenationExpression::fromSyntax( std::span(), syntax.sourceRange())); }; - if (!context.flags.has(ASTFlags::StreamingAllowed)) { + if (!context.flags.has(ASTFlags::StreamingAllowed) && + (isDestination || !comp.hasFlag(CompilationFlags::AllowSelfDeterminedStreamConcat))) { context.addDiag(diag::BadStreamContext, syntax.operatorToken.location()); return badResult(); } @@ -1811,21 +1808,12 @@ Expression& StreamingConcatenationExpression::fromSyntax( uint64_t bitstreamWidth = 0; SmallVector buffer; for (const auto argSyntax : syntax.expressions) { - Expression* arg; - if (assignmentTarget && - argSyntax->expression->kind == SyntaxKind::StreamingConcatenationExpression) { - arg = &create(comp, *argSyntax->expression, context, ASTFlags::StreamingAllowed, - assignmentTarget); - } - else { - arg = &selfDetermined(comp, *argSyntax->expression, context, - ASTFlags::StreamingAllowed); - } - - if (arg->bad()) + auto& arg = selfDetermined(comp, *argSyntax->expression, context, + ASTFlags::StreamingAllowed); + if (arg.bad()) return badResult(); - const Type* argType = arg->type; + const Type* argType = arg.type; const Expression* withExpr = nullptr; std::optional constantWithWidth; if (argSyntax->withRange) { @@ -1833,11 +1821,11 @@ Expression& StreamingConcatenationExpression::fromSyntax( // (including a queue). if (!argType->isUnpackedArray() || argType->isAssociativeArray() || !argType->getArrayElementType()->isFixedSize()) { - context.addDiag(diag::BadStreamWithType, arg->sourceRange); + context.addDiag(diag::BadStreamWithType, arg.sourceRange); return badResult(); } - withExpr = &bindSelector(comp, *arg, *argSyntax->withRange->range, + withExpr = &bindSelector(comp, arg, *argSyntax->withRange->range, context.resetFlags(ASTFlags::StreamingWithRange)); if (withExpr->bad()) return badResult(); @@ -1861,17 +1849,17 @@ Expression& StreamingConcatenationExpression::fromSyntax( } if (!argType->isBitstreamType(isDestination)) { - context.addDiag(diag::BadStreamExprType, arg->sourceRange) << *arg->type; + context.addDiag(diag::BadStreamExprType, arg.sourceRange) << *arg.type; return badResult(); } - if (!Bitstream::checkClassAccess(*argType, context, arg->sourceRange)) + if (!Bitstream::checkClassAccess(*argType, context, arg.sourceRange)) return badResult(); } uint64_t argWidth = 0; - if (arg->kind == ExpressionKind::Streaming) { - argWidth = arg->as().getBitstreamWidth(); + if (arg.kind == ExpressionKind::Streaming) { + argWidth = arg.as().getBitstreamWidth(); } else if (withExpr) { if (constantWithWidth) { @@ -1895,14 +1883,28 @@ Expression& StreamingConcatenationExpression::fromSyntax( return badResult(); } - buffer.push_back({arg, withExpr, constantWithWidth}); + buffer.push_back({&arg, withExpr, constantWithWidth}); } - // Streaming concatenation has no explicit type. Use void to prevent problems when - // its type is passed to context-determined expressions. - return *comp.emplace(comp.getVoidType(), sliceSize, - bitstreamWidth, buffer.ccopy(comp), - syntax.sourceRange()); + // So normally the type of a streaming concatenation is never inspected, + // since it can only be used in assignments and there is explicit handling + // of these expressions there. We use a void type for the result to represent + // this (also because otherwise what type can we use for e.g. non fixed-size + // streams). However, in VCS compat mode the error about requiring an assignment + // context can be silenced, so we need to come up with a real result type here, + // which we do by converting to a packed bit vector of bitstream width. + auto& result = *comp.emplace( + comp.getVoidType(), sliceSize, bitstreamWidth, buffer.ccopy(comp), syntax.sourceRange()); + + if (!context.flags.has(ASTFlags::StreamingAllowed)) { + // Cap the width so we don't overflow. The conversion will error for us + // since the target width will be less than the source width. + auto width = std::min(bitstreamWidth, (uint64_t)SVInt::MAX_BITS); + auto& type = comp.getType(bitwidth_t(width), IntegralFlags::FourState); + return convertAssignment(context, type, result, result.sourceRange.start()); + } + + return result; } ConstantValue StreamingConcatenationExpression::evalImpl(EvalContext& context) const { diff --git a/source/driver/Driver.cpp b/source/driver/Driver.cpp index dddbb20d8..798e976e8 100644 --- a/source/driver/Driver.cpp +++ b/source/driver/Driver.cpp @@ -169,6 +169,9 @@ void Driver::addStandardArgs() { "Allow implicit call expressions to be recursive function calls."); addCompFlag(CompilationFlags::AllowBareValParamAssignment, "--allow-bare-value-param-assigment", "Allow module parameter assignments to elide the parentheses."); + addCompFlag(CompilationFlags::AllowSelfDeterminedStreamConcat, + "--allow-self-determined-stream-concat", + "Allow self-determined streaming concatenation expressions"); addCompFlag(CompilationFlags::StrictDriverChecking, "--strict-driver-checking", "Perform strict driver checking, which currently means disabling " "procedural 'for' loop unrolling."); @@ -415,7 +418,8 @@ bool Driver::processOptions() { CompilationFlags::RelaxEnumConversions, CompilationFlags::RelaxStringConversions, CompilationFlags::AllowRecursiveImplicitCall, - CompilationFlags::AllowBareValParamAssignment}; + CompilationFlags::AllowBareValParamAssignment, + CompilationFlags::AllowSelfDeterminedStreamConcat}; for (auto flag : vcsCompatFlags) { auto& option = options.compilationFlags.at(flag); diff --git a/tests/unittests/ast/ExpressionTests.cpp b/tests/unittests/ast/ExpressionTests.cpp index 8175c781b..771c15e59 100644 --- a/tests/unittests/ast/ExpressionTests.cpp +++ b/tests/unittests/ast/ExpressionTests.cpp @@ -1578,7 +1578,7 @@ module sub(input byte b); }; for (const auto& test : illegal) { - if (testBitstream(test.sv, test.msg) != 1) { + if (testBitstream(test.sv, test.msg) == 0) { FAIL_CHECK(test.sv); } } @@ -3163,3 +3163,31 @@ endmodule CHECK(diags[0].code == diag::SelectAfterRangeSelect); CHECK(diags[1].code == diag::SelectAfterRangeSelect); } + +TEST_CASE("Streaming concat in non-stream context") { + auto tree = SyntaxTree::fromText(R"( +module m; + logic [16777214:0] a; + int i = {<<{a, a}} + 1; + + let known(sig) = (!($isunknown({>>{sig}}))); + + property known_prop(sig, clk); + @(clk) (!($isunknown({>>{sig}}))); + endproperty + + wire clk; + assert property (known_prop(known(clk), clk)); +endmodule +)"); + + CompilationOptions options; + options.flags |= CompilationFlags::AllowSelfDeterminedStreamConcat; + + Compilation compilation(options); + compilation.addSyntaxTree(tree); + + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 1); + CHECK(diags[0].code == diag::BadStreamSize); +}