From 06b1a766a3c329ecf63f55cf3a0b6fd91a8c555d Mon Sep 17 00:00:00 2001 From: Mike Popoloski Date: Sun, 12 Jan 2025 11:22:30 -0500 Subject: [PATCH] Include inline genvars in generate block array member lists --- source/ast/symbols/BlockSymbols.cpp | 48 ++++++++++++++---------- tests/unittests/ast/HierarchyTests.cpp | 4 +- tests/unittests/ast/MemberTests.cpp | 2 +- tests/unittests/parsing/VisitorTests.cpp | 2 +- 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/source/ast/symbols/BlockSymbols.cpp b/source/ast/symbols/BlockSymbols.cpp index 0c05a8ef1..0c9312697 100644 --- a/source/ast/symbols/BlockSymbols.cpp +++ b/source/ast/symbols/BlockSymbols.cpp @@ -671,14 +671,13 @@ static uint64_t getGenerateLoopCount(const Scope& parent) { return count ? count : 1; } -GenerateBlockArraySymbol& GenerateBlockArraySymbol::fromSyntax(Compilation& compilation, +GenerateBlockArraySymbol& GenerateBlockArraySymbol::fromSyntax(Compilation& comp, const LoopGenerateSyntax& syntax, SymbolIndex scopeIndex, const ASTContext& context, uint32_t constructIndex) { auto [name, loc] = getGenerateBlockName(*syntax.block); - auto result = compilation.emplace(compilation, name, loc, - constructIndex); + auto result = comp.emplace(comp, name, loc, constructIndex); result->setSyntax(syntax); result->setAttributes(*context.scope, syntax.attributes); @@ -686,11 +685,13 @@ GenerateBlockArraySymbol& GenerateBlockArraySymbol::fromSyntax(Compilation& comp if (genvar.isMissing()) return *result; + auto genvarSyntax = comp.emplace(genvar); + // Walk up the tree a bit to see if we're nested inside another generate loop. // If we are, we'll include that parent's array size in our decision about // wether we've looped too many times within one generate block. const uint64_t baseCount = getGenerateLoopCount(*context.scope); - const uint64_t loopLimit = compilation.getOptions().maxGenerateSteps; + const uint64_t loopLimit = comp.getOptions().maxGenerateSteps; // If the loop initializer has a `genvar` keyword, we can use the name directly // Otherwise we need to do a lookup to make sure we have the actual genvar somewhere. @@ -707,26 +708,33 @@ GenerateBlockArraySymbol& GenerateBlockArraySymbol::fromSyntax(Compilation& comp return *result; } - compilation.noteReference(*symbol); + comp.noteReference(*symbol); + } + else { + // Fabricate a genvar symbol to live in this array since it was declared inline. + auto genvarSymbol = comp.emplace(genvar.valueText(), genvar.location()); + genvarSymbol->setSyntax(*genvarSyntax); + result->addMember(*genvarSymbol); } SmallVector entries; auto createBlock = [&, blockLoc = loc](ConstantValue value, bool isUninstantiated) { // Spec: each generate block gets their own scope, with an implicit // localparam of the same name as the genvar. - auto block = compilation.emplace(compilation, "", blockLoc, - (uint32_t)entries.size(), - isUninstantiated); - auto implicitParam = compilation.emplace( - genvar.valueText(), genvar.location(), true /* isLocal */, false /* isPort */); + auto block = comp.emplace(comp, "", blockLoc, (uint32_t)entries.size(), + isUninstantiated); + auto implicitParam = comp.emplace(genvar.valueText(), genvar.location(), + true /* isLocal */, false /* isPort */); + implicitParam->setSyntax(*genvarSyntax); + comp.noteReference(*implicitParam); block->addMember(*implicitParam); block->setSyntax(*syntax.block); addBlockMembers(*block, *syntax.block); - implicitParam->setType(compilation.getIntegerType()); - implicitParam->setValue(compilation, std::move(value), /* needsCoercion */ false); + implicitParam->setType(comp.getIntegerType()); + implicitParam->setValue(comp, std::move(value), /* needsCoercion */ false); implicitParam->setIsFromGenvar(true); block->arrayIndex = &implicitParam->getValue().integer(); @@ -734,19 +742,19 @@ GenerateBlockArraySymbol& GenerateBlockArraySymbol::fromSyntax(Compilation& comp }; // Bind the initialization expression. - auto& initial = Expression::bindRValue(compilation.getIntegerType(), *syntax.initialExpr, + auto& initial = Expression::bindRValue(comp.getIntegerType(), *syntax.initialExpr, syntax.equals.range(), context); ConstantValue initialVal = context.eval(initial); if (!initialVal) return *result; // Fabricate a local variable that will serve as the loop iteration variable. - auto& iterScope = *compilation.emplace(compilation, "", loc, - StatementBlockKind::Sequential, - VariableLifetime::Automatic); - auto& local = *compilation.emplace(genvar.valueText(), genvar.location(), - VariableLifetime::Automatic); - local.setType(compilation.getIntegerType()); + auto& iterScope = *comp.emplace(comp, "", loc, + StatementBlockKind::Sequential, + VariableLifetime::Automatic); + auto& local = *comp.emplace(genvar.valueText(), genvar.location(), + VariableLifetime::Automatic); + local.setType(comp.getIntegerType()); local.flags |= VariableFlags::CompilerGenerated; iterScope.setTemporaryParent(*context.scope, scopeIndex); @@ -820,7 +828,7 @@ GenerateBlockArraySymbol& GenerateBlockArraySymbol::fromSyntax(Compilation& comp createBlock(index, isUninstantiated); } - result->entries = entries.copy(compilation); + result->entries = entries.copy(comp); if (entries.empty()) { createBlock(SVInt(32, 0, true), true); } diff --git a/tests/unittests/ast/HierarchyTests.cpp b/tests/unittests/ast/HierarchyTests.cpp index 4ac5da76d..563363ab4 100644 --- a/tests/unittests/ast/HierarchyTests.cpp +++ b/tests/unittests/ast/HierarchyTests.cpp @@ -177,10 +177,10 @@ endmodule Compilation compilation; const auto& instance = evalModule(tree, compilation).body.memberAt(1); - REQUIRE(std::ranges::distance(instance.members()) == 10); + REQUIRE(std::ranges::distance(instance.members()) == 11); for (uint32_t i = 0; i < 10; i++) { - auto& leaf = instance.memberAt(i).memberAt(1).body; + auto& leaf = instance.memberAt(i + 1).memberAt(1).body; auto& foo = leaf.find("foo"); CHECK(foo.getValue().integer() == i); } diff --git a/tests/unittests/ast/MemberTests.cpp b/tests/unittests/ast/MemberTests.cpp index b74996be8..8d64f11d5 100644 --- a/tests/unittests/ast/MemberTests.cpp +++ b/tests/unittests/ast/MemberTests.cpp @@ -1052,7 +1052,7 @@ endmodule auto& foo = compilation.getRoot() .lookupName("top.m1[2][1][3].asdf") - .memberAt(1) + .memberAt(2) .memberAt(1) .find("foo"); diff --git a/tests/unittests/parsing/VisitorTests.cpp b/tests/unittests/parsing/VisitorTests.cpp index 9266272a5..4dfae7ca8 100644 --- a/tests/unittests/parsing/VisitorTests.cpp +++ b/tests/unittests/parsing/VisitorTests.cpp @@ -747,5 +747,5 @@ TEST_CASE("Visit all file") { v.visitDefault(elem); })); - CHECK(count == 1487); + CHECK(count == 1488); }