From 30a0274a60bd8293ed73529d34920bdc01457cfd Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Fri, 24 Nov 2023 20:46:31 -0500 Subject: [PATCH] [firrtl] Add 4.0.0 public modules Add support for FIRRTL 4.0.0 feature, "public modules". This does not provide complete FIRRTL 4.0.0 support as this requires removing circuit names. This will be done in a later commit. Signed-off-by: Schuyler Eldridge --- lib/Dialect/FIRRTL/Import/FIRParser.cpp | 53 +++++++++++++-------- lib/Dialect/FIRRTL/Import/FIRTokenKinds.def | 1 + test/Dialect/FIRRTL/parse-basic.fir | 12 +++++ test/Dialect/FIRRTL/parse-errors.fir | 13 +++++ 4 files changed, 59 insertions(+), 20 deletions(-) diff --git a/lib/Dialect/FIRRTL/Import/FIRParser.cpp b/lib/Dialect/FIRRTL/Import/FIRParser.cpp index 562fa6d1c9fb..a41f8384b5a0 100644 --- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp @@ -4111,7 +4111,7 @@ struct FIRCircuitParser : public FIRParser { ParseResult parseExtClass(CircuitOp circuit, unsigned indent); ParseResult parseExtModule(CircuitOp circuit, unsigned indent); ParseResult parseIntModule(CircuitOp circuit, unsigned indent); - ParseResult parseModule(CircuitOp circuit, unsigned indent); + ParseResult parseModule(CircuitOp circuit, bool isPublic, unsigned indent); ParseResult parsePortList(SmallVectorImpl &resultPorts, SmallVectorImpl &resultPortLocs, @@ -4368,6 +4368,7 @@ ParseResult FIRCircuitParser::skipToModuleEnd(unsigned indent) { case FIRToken::kw_extmodule: case FIRToken::kw_intmodule: case FIRToken::kw_module: + case FIRToken::kw_public: case FIRToken::kw_layer: case FIRToken::kw_type: // All module declarations should have the same indentation @@ -4497,6 +4498,7 @@ ParseResult FIRCircuitParser::parseClass(CircuitOp circuit, unsigned indent) { // build it auto builder = circuit.getBodyBuilder(); auto classOp = builder.create(info.getLoc(), name, portList); + classOp.setPrivate(); deferredModules.emplace_back( DeferredModuleToParse{classOp, portLocs, getLexer().getCursor(), indent}); @@ -4569,13 +4571,20 @@ ParseResult FIRCircuitParser::parseExtModule(CircuitOp circuit, return failure(); auto builder = circuit.getBodyBuilder(); - auto convention = getConstants().options.scalarizeExtModules - ? Convention::Scalarized - : Convention::Internal; + auto isMainModule = (name == circuit.getName()); + auto convention = + (isMainModule && getConstants().options.scalarizeTopModule) || + getConstants().options.scalarizeExtModules + ? Convention::Scalarized + : Convention::Internal; auto conventionAttr = ConventionAttr::get(getContext(), convention); auto annotations = ArrayAttr::get(getContext(), {}); - builder.create(info.getLoc(), name, conventionAttr, portList, - defName, annotations, parameters, internalPaths); + auto extModuleOp = builder.create( + info.getLoc(), name, conventionAttr, portList, defName, annotations, + parameters, internalPaths); + auto visibility = isMainModule ? SymbolTable::Visibility::Public + : SymbolTable::Visibility::Private; + SymbolTable::setSymbolVisibility(extModuleOp, visibility); return success(); } @@ -4609,13 +4618,16 @@ ParseResult FIRCircuitParser::parseIntModule(CircuitOp circuit, ArrayAttr annotations = getConstants().emptyArrayAttr; auto builder = circuit.getBodyBuilder(); - builder.create(info.getLoc(), name, portList, intName, - annotations, parameters, internalPaths); + builder + .create(info.getLoc(), name, portList, intName, annotations, + parameters, internalPaths) + .setPrivate(); return success(); } /// module ::= 'module' id ':' info? INDENT portlist simple_stmt_block DEDENT -ParseResult FIRCircuitParser::parseModule(CircuitOp circuit, unsigned indent) { +ParseResult FIRCircuitParser::parseModule(CircuitOp circuit, bool isPublic, + unsigned indent) { StringAttr name; SmallVector portList; SmallVector portLocs; @@ -4636,8 +4648,9 @@ ParseResult FIRCircuitParser::parseModule(CircuitOp circuit, unsigned indent) { auto builder = circuit.getBodyBuilder(); auto moduleOp = builder.create(info.getLoc(), name, conventionAttr, portList, annotations); - auto visibility = isMainModule ? SymbolTable::Visibility::Public - : SymbolTable::Visibility::Private; + + auto visibility = isMainModule || isPublic ? SymbolTable::Visibility::Public + : SymbolTable::Visibility::Private; SymbolTable::setSymbolVisibility(moduleOp, visibility); // Parse the body of this module after all prototypes have been parsed. This @@ -4671,7 +4684,14 @@ ParseResult FIRCircuitParser::parseToplevelDefinition(CircuitOp circuit, return failure(); return parseLayer(circuit); case FIRToken::kw_module: - return parseModule(circuit, indent); + return parseModule(circuit, /*isPublic=*/false, indent); + case FIRToken::kw_public: + if (requireFeature({4, 0, 0}, "public modules")) + return failure(); + consumeToken(); + if (getToken().getKind() == FIRToken::kw_module) + return parseModule(circuit, /*isPublic=*/true, indent); + return emitError(getToken().getLoc(), "only modules may be public"); case FIRToken::kw_type: return parseTypeDecl(); default: @@ -4935,6 +4955,7 @@ ParseResult FIRCircuitParser::parseCircuit( case FIRToken::kw_intmodule: case FIRToken::kw_layer: case FIRToken::kw_module: + case FIRToken::kw_public: case FIRToken::kw_type: { auto indent = getIndentation(); if (!indent.has_value()) @@ -4986,14 +5007,6 @@ ParseResult FIRCircuitParser::parseCircuit( << "' must contain a module named '" << circuit.getName() << "'"; } - // If the circuit has an entry point that is not an external module, set the - // visibility of all non-main modules to private. - if (auto mainMod = dyn_cast(*main)) { - for (auto mod : circuit.getOps()) { - if (mod != main) - SymbolTable::setSymbolVisibility(mod, SymbolTable::Visibility::Private); - } - } return success(); } diff --git a/lib/Dialect/FIRRTL/Import/FIRTokenKinds.def b/lib/Dialect/FIRRTL/Import/FIRTokenKinds.def index be5fcf2c99d7..5140d67671f5 100644 --- a/lib/Dialect/FIRRTL/Import/FIRTokenKinds.def +++ b/lib/Dialect/FIRRTL/Import/FIRTokenKinds.def @@ -139,6 +139,7 @@ TOK_KEYWORD(old) TOK_KEYWORD(output) TOK_KEYWORD(parameter) TOK_KEYWORD(propassign) +TOK_KEYWORD(public) TOK_KEYWORD(rdwr) TOK_KEYWORD(read) TOK_KEYWORD(ref) diff --git a/test/Dialect/FIRRTL/parse-basic.fir b/test/Dialect/FIRRTL/parse-basic.fir index c36e64efd6ce..15835da9af4b 100644 --- a/test/Dialect/FIRRTL/parse-basic.fir +++ b/test/Dialect/FIRRTL/parse-basic.fir @@ -1863,3 +1863,15 @@ circuit AnyRef: ; CHECK-NEXT: %[[CAST:.+]] = firrtl.object.anyref_cast %[[OBJ]] ; CHECK-NEXT: %[[LIST:.+]] = firrtl.list.create %[[CAST]], %x ; CHECK-NEXT: propassign %listOfAny, %[[LIST]] + +;// ----- + +FIRRTL version 4.0.0 +; CHECK-LABEL: circuit "PublicModules" +circuit PublicModules: + ; CHECK: firrtl.module @Foo + public module Foo: + ; CHECK: firrtl.module private @Bar + module Bar: + ; CHECK: firrtl.module @PublicModules + public module PublicModules: diff --git a/test/Dialect/FIRRTL/parse-errors.fir b/test/Dialect/FIRRTL/parse-errors.fir index e40d50748f85..7454a62c18fb 100644 --- a/test/Dialect/FIRRTL/parse-errors.fir +++ b/test/Dialect/FIRRTL/parse-errors.fir @@ -1140,3 +1140,16 @@ circuit DoubleRadix: output d : Double ; expected-error @below {{expected floating point in Double expression}} propassign d, Double(0hABC) + +;// ----- +FIRRTL version 3.9.0 +circuit PublicModuleUnsupported: + ; expected-error @below {{public modules are a FIRRTL 4.0.0+ feature}} + public module PublicModuleUnsupported: + +;// ----- +FIRRTL version 4.0.0 +circuit PublicNonModule: + ; expected-error @below {{only modules may be public}} + public extmodule Foo: + module PublicNonModule: