Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[firrtl] Add 4.0.0 public modules #6448

Merged
merged 2 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/circt/Dialect/FIRRTL/FIRParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct FIRParserOptions {
/// This, along with numOMIRFiles provides structure to the buffers in the
/// source manager.
unsigned numAnnotationFiles;
bool scalarizeTopModule = false;
bool scalarizePublicModules = false;
bool scalarizeExtModules = false;
};

Expand Down
60 changes: 37 additions & 23 deletions lib/Dialect/FIRRTL/Import/FIRParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<PortInfo> &resultPorts,
SmallVectorImpl<SMLoc> &resultPortLocs,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -4497,6 +4498,7 @@ ParseResult FIRCircuitParser::parseClass(CircuitOp circuit, unsigned indent) {
// build it
auto builder = circuit.getBodyBuilder();
auto classOp = builder.create<ClassOp>(info.getLoc(), name, portList);
classOp.setPrivate();
deferredModules.emplace_back(
DeferredModuleToParse{classOp, portLocs, getLexer().getCursor(), indent});

Expand Down Expand Up @@ -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.scalarizePublicModules) ||
getConstants().options.scalarizeExtModules
? Convention::Scalarized
: Convention::Internal;
auto conventionAttr = ConventionAttr::get(getContext(), convention);
auto annotations = ArrayAttr::get(getContext(), {});
builder.create<FExtModuleOp>(info.getLoc(), name, conventionAttr, portList,
defName, annotations, parameters, internalPaths);
auto extModuleOp = builder.create<FExtModuleOp>(
info.getLoc(), name, conventionAttr, portList, defName, annotations,
parameters, internalPaths);
auto visibility = isMainModule ? SymbolTable::Visibility::Public
seldridge marked this conversation as resolved.
Show resolved Hide resolved
: SymbolTable::Visibility::Private;
SymbolTable::setSymbolVisibility(extModuleOp, visibility);
return success();
}

Expand Down Expand Up @@ -4609,13 +4618,16 @@ ParseResult FIRCircuitParser::parseIntModule(CircuitOp circuit,

ArrayAttr annotations = getConstants().emptyArrayAttr;
auto builder = circuit.getBodyBuilder();
builder.create<FIntModuleOp>(info.getLoc(), name, portList, intName,
annotations, parameters, internalPaths);
builder
.create<FIntModuleOp>(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<PortInfo, 8> portList;
SmallVector<SMLoc> portLocs;
Expand All @@ -4626,18 +4638,20 @@ ParseResult FIRCircuitParser::parseModule(CircuitOp circuit, unsigned indent) {
info.parseOptionalInfo() || parsePortList(portList, portLocs, indent))
return failure();

auto circuitName = circuit.getName();
auto isMainModule = (name == circuitName);
// The main module is implicitly public.
isPublic |= name == circuit.getName();

ArrayAttr annotations = getConstants().emptyArrayAttr;
auto convention = Convention::Internal;
if (isMainModule && getConstants().options.scalarizeTopModule)
if (isPublic && getConstants().options.scalarizePublicModules)
convention = Convention::Scalarized;
auto conventionAttr = ConventionAttr::get(getContext(), convention);
auto builder = circuit.getBodyBuilder();
auto moduleOp = builder.create<FModuleOp>(info.getLoc(), name, conventionAttr,
portList, annotations);
auto visibility = isMainModule ? SymbolTable::Visibility::Public
: SymbolTable::Visibility::Private;

auto visibility = isPublic ? SymbolTable::Visibility::Public
: SymbolTable::Visibility::Private;
SymbolTable::setSymbolVisibility(moduleOp, visibility);

// Parse the body of this module after all prototypes have been parsed. This
Expand Down Expand Up @@ -4671,7 +4685,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();
seldridge marked this conversation as resolved.
Show resolved Hide resolved
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:
Expand Down Expand Up @@ -4935,6 +4956,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())
Expand Down Expand Up @@ -4986,14 +5008,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<FModuleOp>(*main)) {
for (auto mod : circuit.getOps<FModuleLike>()) {
if (mod != main)
SymbolTable::setSymbolVisibility(mod, SymbolTable::Visibility::Private);
}
}
return success();
}

Expand Down
1 change: 1 addition & 0 deletions lib/Dialect/FIRRTL/Import/FIRTokenKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions test/Dialect/FIRRTL/parse-basic.fir
Original file line number Diff line number Diff line change
Expand Up @@ -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:
13 changes: 13 additions & 0 deletions test/Dialect/FIRRTL/parse-errors.fir
Original file line number Diff line number Diff line change
Expand Up @@ -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:
16 changes: 8 additions & 8 deletions test/firtool/convention.fir
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
; RUN: firtool %s --format=fir --parse-only --scalarize-top-module=false --scalarize-ext-modules=false --preserve-aggregate=all | FileCheck --check-prefix=SCALARIZE_NONE %s
; RUN: firtool %s --format=fir --parse-only --scalarize-top-module=true --scalarize-ext-modules=false --preserve-aggregate=all | FileCheck --check-prefix=SCALARIZE_TOP %s
; RUN: firtool %s --format=fir --parse-only --scalarize-top-module=false --scalarize-ext-modules=true --preserve-aggregate=all | FileCheck --check-prefix=SCALARIZE_EXT %s
; RUN: firtool %s --format=fir --parse-only --scalarize-top-module=true --scalarize-ext-modules=true --preserve-aggregate=all | FileCheck --check-prefix=SCALARIZE_BOTH %s
; RUN: firtool %s --format=fir --parse-only --scalarize-public-modules=false --scalarize-ext-modules=false --preserve-aggregate=all | FileCheck --check-prefix=SCALARIZE_NONE %s
; RUN: firtool %s --format=fir --parse-only --scalarize-public-modules=true --scalarize-ext-modules=false --preserve-aggregate=all | FileCheck --check-prefix=SCALARIZE_PUB %s
; RUN: firtool %s --format=fir --parse-only --scalarize-public-modules=false --scalarize-ext-modules=true --preserve-aggregate=all | FileCheck --check-prefix=SCALARIZE_EXT %s
; RUN: firtool %s --format=fir --parse-only --scalarize-public-modules=true --scalarize-ext-modules=true --preserve-aggregate=all | FileCheck --check-prefix=SCALARIZE_BOTH %s

; Ensure that top module and ext modules are marked scalarized.

circuit Top :
; SCALARIZE_NONE-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_TOP: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_PUB: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_EXT-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_BOTH: attributes {convention = #firrtl<convention scalarized>}
module Top :
output port: UInt<8>[2]
port[0] <= UInt<8>(0)
port[1] <= UInt<8>(0)

; SCALARIZE_NONE-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_TOP-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_PUB-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_EXT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_BOTH: attributes {convention = #firrtl<convention scalarized>}
extmodule External :
output port: UInt<8>[2]

; SCALARIZE_NONE-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_TOP-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_PUB-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_EXT-NOT: attributes {convention = #firrtl<convention scalarized>}
; SCALARIZE_BOTH-NOT: attributes {convention = #firrtl<convention scalarized>}
module Internal :
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/refs-in-aggs.fir
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
; Check this design makes it through firtool.
; RUN: firtool %s --no-dedup
; RUN: firtool %s --no-dedup --preserve-aggregate=all --scalarize-top-module=false
; RUN: firtool %s --no-dedup --preserve-aggregate=all --scalarize-public-modules=false

; Check parsing into open aggregates.
; RUN: circt-translate -import-firrtl -verify-diagnostics -split-input-file %s | circt-opt | FileCheck %s
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/define-flip-to-passive.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0
circuit Foo :
; SPEC EXAMPLE BEGIN
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/define-subelement.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0
circuit Foo:
; SPEC EXAMPLE BEGIN
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/define.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0
circuit Refs:
; SPEC EXAMPLE BEGIN
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/endpoints.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0
circuit Top : %[[
{ "class": "firrtl.transforms.DontTouchAnnotation", "target": "~Top|Top>producer_debug"},
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/ext.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
circuit MyExternalModuleWithRefs :
; SPEC EXAMPLE BEGIN
extmodule MyExternalModuleWithRefs :
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/force_and_release.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0
circuit ForceAndRelease:
; SPEC EXAMPLE BEGIN
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/force_initial.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0

circuit ForceAndRelease:
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/force_nonpassive.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s -verify-diagnostics
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false -verify-diagnostics
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false -verify-diagnostics
FIRRTL version 3.0.0

circuit Top :
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/forwarding_refs_upwards.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
circuit Forward:
; SPEC EXAMPLE BEGIN
extmodule Foo : ; XXX: module -> extmodule
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/forwarding_refs_upwards_narrow.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
circuit Forward :
; SPEC EXAMPLE BEGIN
extmodule Foo :
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/in_ref_context.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s -verify-diagnostics
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false -verify-diagnostics
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false -verify-diagnostics
FIRRTL version 3.0.0
; expected-error @+27 {{input probes cannot be used}}
; expected-note @+19 {{input probe here}}
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/inference_reset_bad.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s -verify-diagnostics
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false -verify-diagnostics
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false -verify-diagnostics
; XFAIL: *
; https://github.com/llvm/circt/issues/4813
FIRRTL version 3.0.0
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/inference_reset_good.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0
circuit ResetInferGood :
; SPEC EXAMPLE BEGIN
Expand Down
3 changes: 1 addition & 2 deletions test/firtool/spec/refs/inference_works.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
; NOT SPEC EXAMPLE
FIRRTL version 3.0.0

Expand All @@ -16,4 +16,3 @@ circuit MinimumWidth :
inst u of Uninferred
connect u.x, x
connect y, read(u.r)

2 changes: 1 addition & 1 deletion test/firtool/spec/refs/invalid_in.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s --verify-diagnostics
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false --verify-diagnostics
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false --verify-diagnostics
FIRRTL version 3.0.0
circuit FooUser:
; Invalid, per spec
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/nested_refproducer.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0
circuit RefProducer :
; SPEC EXAMPLE BEGIN
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/spec/refs/nosubaccess.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
circuit NoSubAccessesWithProbes :
extmodule Ext :
output x : {a : Probe<UInt<2>[2]>, b : UInt<2>}[3]
Expand Down
3 changes: 1 addition & 2 deletions test/firtool/spec/refs/probe_export_simple.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
circuit MyModule :
; SPEC EXAMPLE BEGIN
module MyModule :
Expand All @@ -8,4 +8,3 @@ circuit MyModule :

define r = probe(in)
; SPEC EXAMPLE END

2 changes: 1 addition & 1 deletion test/firtool/spec/refs/read.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0

circuit Foo :
Expand Down
3 changes: 1 addition & 2 deletions test/firtool/spec/refs/read_subelement.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0

circuit Foo :
Expand All @@ -17,4 +17,3 @@ circuit Foo :
inst f of Foo
connect x, read(f.p.b) ; indirectly access the probed data
; SPEC EXAMPLE END

2 changes: 1 addition & 1 deletion test/firtool/spec/refs/read_subelement_add.fir
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: firtool %s
; RUN: firtool %s -preserve-aggregate=all -scalarize-top-module=false
; RUN: firtool %s -preserve-aggregate=all -scalarize-public-modules=false
FIRRTL version 3.0.0

circuit Foo :
Expand Down
Loading