Skip to content

Commit

Permalink
Add option to allow module instantiations to provide a param assignme…
Browse files Browse the repository at this point in the history
…nt without parentheses
  • Loading branch information
MikePopoloski committed Feb 16, 2024
1 parent 018ac57 commit e13feb8
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 14 deletions.
3 changes: 2 additions & 1 deletion bindings/python/CompBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ void registerCompilation(py::module_& m) {
.value("SuppressUnused", CompilationFlags::SuppressUnused)
.value("IgnoreUnknownModules", CompilationFlags::IgnoreUnknownModules)
.value("RelaxStringConversions", CompilationFlags::RelaxStringConversions)
.value("AllowRecursiveImplicitCall", CompilationFlags::AllowRecursiveImplicitCall);
.value("AllowRecursiveImplicitCall", CompilationFlags::AllowRecursiveImplicitCall)
.value("AllowBareValParamAssignment", CompilationFlags::AllowBareValParamAssignment);

py::class_<CompilationOptions>(m, "CompilationOptions")
.def(py::init<>())
Expand Down
8 changes: 7 additions & 1 deletion docs/command-line-ref.dox
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ The default is 65535.
`--compat vcs`

Attempt to increase compatibility with the specified tool. Various options will
be set and warnings will be silenced to mimic the given tool as closely as possible.
be set and some warnings will be silenced to mimic the given tool as closely as possible.
Currently only 'vcs' is supported as a value.

`-T,--timing min|typ|max`
Expand Down Expand Up @@ -314,6 +314,12 @@ Allow implicit call expressions (ones that lack parentheses) to be recursive fun
The LRM states that directly recursive function calls require parentheses but many other tools
allow this anyway so this flag can be used for compatibility purposes.

`--allow-bare-value-param-assigment`

Allow module instantiations to provide a single parameter value assignment without enclosing it
in parentheses. SystemVerilog doesn't permit this, 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
Expand Down
7 changes: 5 additions & 2 deletions include/slang/ast/Compilation.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,12 @@ enum class SLANG_EXPORT CompilationFlags {
RelaxStringConversions = 1 << 9,

/// Allow implicit call expressions (lacking parentheses) to be recursive function calls.
AllowRecursiveImplicitCall = 1 << 10
AllowRecursiveImplicitCall = 1 << 10,

/// Allow module parameter assignments to elide the parentheses.
AllowBareValParamAssignment = 1 << 11
};
SLANG_BITMASK(CompilationFlags, AllowRecursiveImplicitCall)
SLANG_BITMASK(CompilationFlags, AllowBareValParamAssignment)

/// Contains various options that can control compilation behavior.
struct SLANG_EXPORT CompilationOptions {
Expand Down
42 changes: 33 additions & 9 deletions source/ast/symbols/InstanceSymbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,10 @@ void PrimitiveInstanceSymbol::fromSyntax(const PrimitiveInstantiationSyntax& syn
auto& comp = context.getCompilation();
auto name = syntax.type.valueText();

auto missing = [&](TokenKind tk, SourceLocation loc) {
return Token::createMissing(comp, tk, loc);
};

if (syntax.type.kind == TokenKind::Identifier) {
auto def = comp.getDefinition(name, *context.scope, syntax.type.range(),
diag::UnknownPrimitive);
Expand All @@ -1226,16 +1230,36 @@ void PrimitiveInstanceSymbol::fromSyntax(const PrimitiveInstantiationSyntax& syn
implicitNets);
return;
}

SLANG_ASSERT(syntax.strength || syntax.delay);
if (syntax.strength) {
context.addDiag(diag::InstanceWithStrength, syntax.strength->sourceRange()) << name;
}
else if (comp.hasFlag(CompilationFlags::AllowBareValParamAssignment) &&
syntax.delay->kind == SyntaxKind::DelayControl) {
// We're allowing this to be a hierarchical instantiation with a single param
// assignment, and just pretending the parentheses were provided.
auto& delay = syntax.delay->as<DelaySyntax>();
auto& delayVal = *delay.delayValue;

SmallVector<TokenOrSyntax> parameters;
parameters.push_back(comp.emplace<OrderedParamAssignmentSyntax>(delayVal));

auto pvas = comp.emplace<ParameterValueAssignmentSyntax>(
delay.hash,
missing(TokenKind::OpenParenthesis, delayVal.getFirstToken().location()),
parameters.copy(comp),
missing(TokenKind::CloseParenthesis, delayVal.getLastToken().location()));

auto instantiation = comp.emplace<HierarchyInstantiationSyntax>(
syntax.attributes, syntax.type, pvas, syntax.instances, syntax.semi);
InstanceSymbol::fromSyntax(comp, *instantiation, context, results, implicitNets,
/* isFromBind */ false);
return;
}
else {
SLANG_ASSERT(syntax.strength || syntax.delay);
if (syntax.strength) {
context.addDiag(diag::InstanceWithStrength, syntax.strength->sourceRange())
<< name;
}
else {
context.addDiag(diag::InstanceWithDelay,
syntax.delay->getFirstToken().location() + 1);
}
context.addDiag(diag::InstanceWithDelay,
syntax.delay->getFirstToken().location() + 1);
}
}
UninstantiatedDefSymbol::fromSyntax(comp, syntax, context, results, implicitNets);
Expand Down
5 changes: 4 additions & 1 deletion source/driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ void Driver::addStandardArgs() {
"Allow top-level modules to have interface ports.");
addCompFlag(CompilationFlags::AllowRecursiveImplicitCall, "--allow-recursive-implicit-call",
"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::StrictDriverChecking, "--strict-driver-checking",
"Perform strict driver checking, which currently means disabling "
"procedural 'for' loop unrolling.");
Expand Down Expand Up @@ -410,7 +412,8 @@ bool Driver::processOptions() {
CompilationFlags::AllowUseBeforeDeclare,
CompilationFlags::RelaxEnumConversions,
CompilationFlags::RelaxStringConversions,
CompilationFlags::AllowRecursiveImplicitCall};
CompilationFlags::AllowRecursiveImplicitCall,
CompilationFlags::AllowBareValParamAssignment};

for (auto flag : vcsCompatFlags) {
auto& option = options.compilationFlags.at(flag);
Expand Down
30 changes: 30 additions & 0 deletions tests/unittests/ast/MemberTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2475,3 +2475,33 @@ endmodule
compilation.addSyntaxTree(tree);
NO_COMPILATION_ERRORS;
}

TEST_CASE("Parameter assignment missing paren compat") {
auto tree = SyntaxTree::fromText(R"(
module a (i0, i1, o1);
parameter B_SIZE = 8;
input [B_SIZE-1:0] i0;
input [B_SIZE-1:0] i1;
output [B_SIZE-1:0] o1;
assign o1[B_SIZE-1:0] = i0[B_SIZE-1:0] + i1[B_SIZE-1:0];
endmodule
module b(i0, i1, o1);
input [65:0] i0;
input [65:0] i1;
output [65:0] o1;
a #66 new_adder(.i0 (i0 ), .i1(i1 ), .o1(o1 ));
endmodule
)");
CompilationOptions options;
options.flags |= CompilationFlags::AllowBareValParamAssignment;

Compilation compilation(options);
compilation.addSyntaxTree(tree);
NO_COMPILATION_ERRORS;

auto& bsize = compilation.getRoot().lookupName<ParameterSymbol>("b.new_adder.B_SIZE");
CHECK(bsize.getValue().integer() == 66);
}

0 comments on commit e13feb8

Please sign in to comment.