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] Verify main module is a module, and is public. #7439

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
12 changes: 10 additions & 2 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,16 @@ LogicalResult CircuitOp::verifyRegions() {

mlir::SymbolTable symtbl(getOperation());

if (!symtbl.lookup(main))
return emitOpError().append("Module with same name as circuit not found");
auto *mainModule = symtbl.lookup(main);
if (!mainModule)
return emitOpError().append(
"does not contain module with same name as circuit");
if (!isa<FModuleLike>(mainModule))
return mainModule->emitError(
"entity with name of circuit must be a module");
if (symtbl.getSymbolVisibility(mainModule) !=
mlir::SymbolTable::Visibility::Public)
return mainModule->emitError("main module must be public");

// Store a mapping of defname to either the first external module
// that defines it or, preferentially, the first external module
Expand Down
12 changes: 4 additions & 8 deletions test/Dialect/FIRRTL/annotations-errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -438,23 +438,19 @@ firrtl.circuit "Top" attributes {
// -----
// OutputDirAnnotation targeting a non-public module should fail.

// expected-error @below {{Unable to apply annotation: {class = "circt.OutputDirAnnotation", dirname = "foo", target = "~Top|Top"}}}
// expected-error @below {{Unable to apply annotation: {class = "circt.OutputDirAnnotation", dirname = "foo", target = "~Top|NotTop"}}}
firrtl.circuit "Top" attributes {
rawAnnotations = [
{
class = "circt.OutputDirAnnotation",
dirname = "foo",
target = "~Top|Top"
},
{
class = "circt.OutputDirAnnotation",
dirname = "foo",
target = "~Top|Top"
target = "~Top|NotTop"
}
]
} {
firrtl.module @Top() {}
// expected-error @below {{circt.OutputDirAnnotation must target a public module}}
firrtl.module private @Top() {}
firrtl.module private @NotTop() {}
}

// -----
Expand Down
20 changes: 20 additions & 0 deletions test/Dialect/FIRRTL/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -2554,3 +2554,23 @@ firrtl.module @LHSTypes() {
}
}

// -----

// expected-error @below {{op does not contain module with same name as circuit}}
firrtl.circuit "NoMain" { }

// -----

firrtl.circuit "PrivateMain" {
// expected-error @below {{main module must be public}}
firrtl.module private @PrivateMain() {}
}

// -----

firrtl.circuit "MainNotModule" {
// expected-error @below {{entity with name of circuit must be a module}}
func.func @MainNotModule() {
return
}
}
2 changes: 1 addition & 1 deletion test/Dialect/FIRRTL/lower-classes.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ firrtl.circuit "AnyCast" {
// CHECK-LABEL: firrtl.circuit "ModuleWithPropertySubmodule"
firrtl.circuit "ModuleWithPropertySubmodule" {
// CHECK: om.class @ModuleWithPropertySubmodule_Class
firrtl.module private @ModuleWithPropertySubmodule() {
firrtl.module @ModuleWithPropertySubmodule() {
%c0 = firrtl.integer 0
// CHECK: om.object @SubmoduleWithProperty_Class
%inst.prop = firrtl.instance inst @SubmoduleWithProperty(in prop: !firrtl.integer)
Expand Down
4 changes: 2 additions & 2 deletions test/Dialect/FIRRTL/lower-intmodules.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ firrtl.circuit "ProbeIntrinsicTest" {
// CHECK-NOT: firrtl.intmodule private @FPGAProbeIntrinsic
firrtl.intmodule private @FPGAProbeIntrinsic(in data: !firrtl.uint, in clock: !firrtl.clock) attributes {intrinsic = "circt_fpga_probe"}

// CHECK-LABEL: firrtl.module private @ProbeIntrinsicTest
firrtl.module private @ProbeIntrinsicTest(in %clock : !firrtl.clock, in %data : !firrtl.uint<32>) {
// CHECK-LABEL: firrtl.module @ProbeIntrinsicTest
firrtl.module @ProbeIntrinsicTest(in %clock : !firrtl.clock, in %data : !firrtl.uint<32>) {
// CHECK: [[DATA:%.+]] = firrtl.wire : !firrtl.uint
// CHECK-NEXT: [[CLOCK:%.+]] = firrtl.wire : !firrtl.clock
// CHECK-NEXT: firrtl.int.generic "circt_fpga_probe" [[DATA]], [[CLOCK]] : (!firrtl.uint, !firrtl.clock) -> ()
Expand Down
2 changes: 1 addition & 1 deletion test/Dialect/FIRRTL/lower-intrinsics-errors.mlir
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl.module(firrtl-lower-intrinsics)))' -verify-diagnostics --split-input-file %s

firrtl.circuit "UnknownIntrinsic" {
firrtl.module private @UnknownIntrinsic(in %data: !firrtl.uint<32>) {
firrtl.module @UnknownIntrinsic(in %data: !firrtl.uint<32>) {
%0 = firrtl.wire : !firrtl.uint<32>
// expected-error @below {{unknown intrinsic}}
// expected-error @below {{failed to legalize}}
Expand Down
Loading