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][FIRParser] Enforce 4.0.0 main module must be public. #6747

Merged
merged 2 commits into from
Feb 26, 2024
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
9 changes: 7 additions & 2 deletions lib/Dialect/FIRRTL/Import/FIRParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5037,7 +5037,8 @@ ParseResult FIRCircuitParser::parseModule(CircuitOp circuit, bool isPublic,
SmallVector<PortInfo, 8> portList;
SmallVector<SMLoc> portLocs;
ArrayAttr layers;
LocWithInfo info(getToken().getLoc(), this);
auto modLoc = getToken().getLoc();
LocWithInfo info(modLoc, this);
consumeToken(FIRToken::kw_module);
if (parseId(name, "expected module name") ||
parseOptionalEnabledLayers(layers) ||
Expand All @@ -5046,7 +5047,11 @@ ParseResult FIRCircuitParser::parseModule(CircuitOp circuit, bool isPublic,
return failure();

// The main module is implicitly public.
isPublic |= name == circuit.getName();
if (name == circuit.getName()) {
if (!isPublic && removedFeature({4, 0, 0}, "private main modules", modLoc))
return failure();
isPublic = true;
}

if (isPublic && version >= FIRVersion{4, 0, 0}) {
for (auto [pi, loc] : llvm::zip_equal(portList, portLocs)) {
Expand Down
8 changes: 4 additions & 4 deletions test/Dialect/FIRRTL/parse-basic.fir
Original file line number Diff line number Diff line change
Expand Up @@ -1682,7 +1682,7 @@ circuit Layers:
; CHECK: firrtl.module @Layers
; CHECK-SAME: out %b: !firrtl.probe<uint<1>, @A>
; CHECK-SAME: out %c: !firrtl.rwprobe<uint<1>, @A::@B>
module Layers:
public module Layers:
input a: UInt<1>
output b: Probe<UInt<1>, A>
output c: RWProbe<UInt<1>, A.B>
Expand Down Expand Up @@ -1858,7 +1858,7 @@ FIRRTL version 4.0.0

; CHECK-LABEL: firrtl.circuit "IntegerArithmetic"
circuit IntegerArithmetic :
module IntegerArithmetic :
public module IntegerArithmetic :
input a : Integer
input b : Integer
output c : Integer
Expand Down Expand Up @@ -1955,7 +1955,7 @@ circuit LayerEnabledModule:
layer C, bind:
; CHECK: firrtl.module @LayerEnabledModule
; CHECK-SAME: layers = [@A, @B::@C]
module LayerEnabledModule enablelayer A enablelayer B.C:
public module LayerEnabledModule enablelayer A enablelayer B.C:

; CHECK: firrtl.module private @UserOfLayerEnabledModule
; CHECK-SAME: layers = [@A, @B::@C]
Expand Down Expand Up @@ -2031,7 +2031,7 @@ FIRRTL version 4.0.0
; CHECK-LABEL: circuit "StaticShiftRight"
circuit StaticShiftRight:
; CHECK: firrtl.module @StaticShiftRight
module StaticShiftRight:
public module StaticShiftRight:
input a : UInt<8>
input b : UInt<0>
input c : SInt<8>
Expand Down
36 changes: 21 additions & 15 deletions test/Dialect/FIRRTL/parse-errors.fir
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ circuit UnsupportedVersionGroups:

FIRRTL version 4.1.0
circuit RemovedGroups:
module RemovedGroups:
public module RemovedGroups:
; expected-error @below {{optional groups were removed in FIRRTL 4.0.0, but the specified FIRRTL version was 4.1.0}}
group A:

Expand All @@ -817,7 +817,7 @@ circuit LayerAndCircuitShareName:
layer LayerAndCircuitShareName, bind :

; expected-error @below {{redefinition of symbol named 'LayerAndCircuitShareName'}}
module LayerAndCircuitShareName:
public module LayerAndCircuitShareName:

;// -----

Expand Down Expand Up @@ -1180,7 +1180,7 @@ FIRRTL version 4.0.0
circuit PublicNonModule:
; expected-error @below {{only modules may be public}}
public extmodule Foo:
module PublicNonModule:
public module PublicNonModule:

;// -----
circuit RWProbeExprOOB :
Expand Down Expand Up @@ -1228,7 +1228,7 @@ circuit Foo:
module FPGATarget:
input clock: UInt<1>

module Foo:
public module Foo:
; expected-error @below {{instance case port 'clock' type does not match the default module port}}
instchoice inst of DefaultTarget, Platform :
FPGA => FPGATarget
Expand All @@ -1246,7 +1246,7 @@ circuit Foo:
module FPGATarget:
input x: UInt<1>

module Foo:
public module Foo:
; expected-error @below {{instance case module port 'x' does not match the default module port 'clock'}}
instchoice inst of DefaultTarget, Platform :
FPGA => FPGATarget
Expand All @@ -1261,7 +1261,7 @@ circuit Foo:
module DefaultTarget:
module FPGATarget:

module Foo:
public module Foo:
; expected-error @below {{use of undefined option case 'ASIC'}}
instchoice inst of DefaultTarget, Platform :
ASIC => FPGATarget
Expand All @@ -1273,7 +1273,7 @@ circuit Foo:
module DefaultTarget:
module FPGATarget:

module Foo:
public module Foo:
; expected-error @below {{use of undefined option group 'Platform'}}
instchoice inst of DefaultTarget, Platform :
ASIC => FPGATarget
Expand All @@ -1299,52 +1299,52 @@ circuit Foo:
FIRRTL version 4.0.0
circuit Foo:
layer A bind:
module Foo:
public module Foo:
output a: Probe<
; expected-error @below {{expected probe data type}}
;// -----
FIRRTL version 4.0.0
circuit Foo:
layer A bind:
module Foo:
public module Foo:
; expected-error @below {{expected '<' in reference type}}
output a: Probe X

;// -----
FIRRTL version 4.0.0
circuit Foo:
layer A bind:
module Foo:
public module Foo:
output a: Probe
; expected-error @below {{expected '<' in reference type}}
;// -----
FIRRTL version 4.0.0
circuit Foo:
layer A bind:
module Foo:
public module Foo:
; expected-error @below {{expected probe data type}}
output a: Probe<>

;// -----
FIRRTL version 4.0.0
circuit Foo:
layer A bind:
module Foo:
public module Foo:
; expected-error @below {{expected '>' to end reference type}}
output a: Probe<UInt<1>
;// -----
FIRRTL version 4.0.0
circuit Foo:
layer A bind:
module Foo:
public module Foo:
; expected-error @below {{expected '>' to end reference type}}
output a: Probe<UInt<1> A

;// -----
FIRRTL version 4.0.0
circuit Foo:
layer A bind:
module Foo:
public module Foo:
; expected-error @below {{expected layer name}}
output a: Probe<UInt<1> A.>

Expand All @@ -1365,6 +1365,12 @@ circuit UnknownWidthExt:
;// -----
FIRRTL version 4.0.0
circuit AbstractResetPublic:
module AbstractResetPublic:
public module AbstractResetPublic:
; expected-error @below {{public module port must have concrete reset type}}
input r: Reset

;// -----
FIRRTL version 4.0.0
circuit PrivateMainModule:
; expected-error @below {{private main modules were removed in FIRRTL 4.0.0}}
module PrivateMainModule:
2 changes: 1 addition & 1 deletion test/firtool/chisel_assert.fir
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ circuit ChiselVerif:
parameter label = "label for cover"

; CHECK: module ChiselVerif
module ChiselVerif:
public module ChiselVerif:
input clock: Clock
input cond: UInt<1>
input enable: UInt<1>
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/instchoice.fir
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ circuit Foo:
module ASICTarget:
input clock: Clock

module Foo:
public module Foo:
input clock: Clock

; CHECK: %inst_clock = firrtl.instance_choice inst interesting_name @DefaultTarget
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/layers.fir
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ circuit Foo: %[[
layer A, bind:
layer B, bind:

module Foo:
public module Foo:
input in: UInt<1>

layerblock A:
Expand Down
2 changes: 1 addition & 1 deletion test/firtool/lower-layers2.fir
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ circuit TestHarness:
; CHECK: .b (b)
; CHECK: );
; CHECK: endmodule
module TestHarness:
public module TestHarness:
input clock: Clock
input reset: UInt<1>
input a: UInt<32>
Expand Down
Loading