Skip to content

Commit

Permalink
[FIRRTL] Verify enabled layers of module, extmodule.
Browse files Browse the repository at this point in the history
* Verify specified symbols resolve for extmodule
* Verify specified symbols resolve to LayerOp's
* Verify public modules don't enable circuit-disabled layers.
  (SpecializeLayers will delete if so)
  • Loading branch information
dtzSiFive committed Aug 26, 2024
1 parent 33c35ff commit 4cdd13c
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 23 deletions.
44 changes: 36 additions & 8 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1694,23 +1694,51 @@ static LogicalResult verifyPortSymbolUses(FModuleLike module,
return success();
}

static LogicalResult verifyEnabledLayers(FModuleLike module,
SymbolTableCollection &symbolTable) {
auto circuitOp = module->getParentOfType<CircuitOp>();
for (auto layer : module.getLayers()) {
auto *op =
symbolTable.lookupSymbolIn(circuitOp, cast<SymbolRefAttr>(layer));
if (!op)
return module->emitOpError() << "enables unknown layer '" << layer << "'";
if (!isa<LayerOp>(op))
return module->emitOpError()
.append("enables non-layer '", layer, "'")
.attachNote(op->getLoc())
.append("expected layer");
}

// Check public modules don't enable circuit-disabled layers.
if (auto disabledLayers = circuitOp.getDisableLayersAttr()) {
auto disabledLayersSymRefs = disabledLayers.getAsRange<SymbolRefAttr>();
if (module.isPublic()) {
for (auto layer : module.getLayers())
if (llvm::is_contained(disabledLayersSymRefs, layer))
return module->emitOpError(
"public module has circuit-disabled layer ")
<< layer << " enabled";
}
}

return success();
}

LogicalResult FModuleOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
if (failed(
verifyPortSymbolUses(cast<FModuleLike>(getOperation()), symbolTable)))
return failure();

auto circuitOp = (*this)->getParentOfType<CircuitOp>();
for (auto layer : getLayers()) {
if (!symbolTable.lookupSymbolIn(circuitOp, cast<SymbolRefAttr>(layer)))
return emitOpError() << "enables unknown layer '" << layer << "'";
}

return success();
return verifyEnabledLayers(cast<FModuleLike>(getOperation()), symbolTable);
}

LogicalResult
FExtModuleOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
return verifyPortSymbolUses(cast<FModuleLike>(getOperation()), symbolTable);
if (failed(
verifyPortSymbolUses(cast<FModuleLike>(getOperation()), symbolTable)))
return failure();

return verifyEnabledLayers(cast<FModuleLike>(getOperation()), symbolTable);
}

LogicalResult
Expand Down
34 changes: 34 additions & 0 deletions test/Dialect/FIRRTL/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ firrtl.circuit "InstanceCannotHavePortSymbols" {
// -----

firrtl.circuit "InstanceMissingLayers" {
firrtl.layer @A bind {}
// expected-note @below {{original module declared here}}
firrtl.extmodule @Ext(in in : !firrtl.uint<1>) attributes {layers = [@A]}
firrtl.module @InstanceMissingLayers() {
Expand Down Expand Up @@ -1977,6 +1978,38 @@ firrtl.circuit "UnknownEnabledLayer" {

// -----

// https://github.com/llvm/circt/issues/7448

firrtl.circuit "NonLayerEnabledLayer" {
// expected-note @below {{expected layer}}
firrtl.extmodule @A()
// expected-error @below {{'firrtl.module' op enables non-layer '@A'}}
firrtl.module @NonLayerEnabledLayer() attributes {layers = [@A]} {}
}

// -----

// Disable layer enabled by main.

firrtl.circuit "MainEnablesDisabledLayer" attributes {disable_layers = [@A]} {
firrtl.layer @A bind { }
// expected-error @below {{public module has circuit-disabled layer @A enabled}}
firrtl.module @MainEnablesDisabledLayer() attributes {layers = [@A]} { }
}

// -----

// Disable layer enabled by non-main public.

firrtl.circuit "PublicEnablesDisabledLayer" attributes {disable_layers = [@A]} {
firrtl.layer @A bind { }
// expected-error @below {{public module has circuit-disabled layer @A enabled}}
firrtl.module @M() attributes {layers = [@A]} { }
firrtl.module @PublicEnablesDisabledLayer() { }
}

// -----

firrtl.circuit "RWProbeRemote" {
firrtl.module @Other() {
%w = firrtl.wire sym @x : !firrtl.uint<1>
Expand Down Expand Up @@ -2561,3 +2594,4 @@ firrtl.circuit "MainNotModule" {
return
}
}

30 changes: 15 additions & 15 deletions test/Dialect/FIRRTL/specialize-layers.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -298,21 +298,21 @@ attributes {
}
}

// CHECK: firrtl.extmodule @ExtModule() attributes {layers = [@A, @A, @A::@B_C]}
firrtl.extmodule @ExtModule() attributes {layers = [@A, @A::@B, @A::@B::@C]}
// CHECK: firrtl.extmodule private @ExtModule() attributes {layers = [@A, @A, @A::@B_C]}
firrtl.extmodule private @ExtModule() attributes {layers = [@A, @A::@B, @A::@B::@C]}
}

// CHECK-LABEL: firrtl.circuit "DisableLayerA"
firrtl.circuit "DisableLayerA" attributes {
disable_layers = [@A]
} {
firrtl.layer @A bind { }
// CHECK-NOT: firrtl.extmodule @Test0()
firrtl.extmodule @Test0() attributes {layers = [@A]}
// CHECK-NOT: firrtl.extmodule @Test1() attributes {layers = [@A]}
firrtl.extmodule @Test1() attributes {layers = [@A]}
// CHECK-NOT: firrtl.extmodule @Test2() attributes {layers = [@A]}
firrtl.extmodule @Test2() attributes {layers = [@A]}
// CHECK-NOT: firrtl.extmodule private @Test0()
firrtl.extmodule private @Test0() attributes {layers = [@A]}
// CHECK-NOT: firrtl.extmodule private @Test1() attributes {layers = [@A]}
firrtl.extmodule private @Test1() attributes {layers = [@A]}
// CHECK-NOT: firrtl.extmodule private @Test2() attributes {layers = [@A]}
firrtl.extmodule private @Test2() attributes {layers = [@A]}

// Top level module, which can't be deleted by the pass.
firrtl.extmodule @DisableLayerA()
Expand Down Expand Up @@ -368,7 +368,7 @@ firrtl.circuit "ProbeOpsEnableA" attributes {
(out out : !firrtl.probe<uint<1>, @A>)
}

firrtl.extmodule @ExtModule(out out : !firrtl.probe<uint<1>, @A>)
firrtl.extmodule private @ExtModule(out out : !firrtl.probe<uint<1>, @A>)
}

// CHECK-LABEL: firrtl.circuit "ProbeOpsDisableA"
Expand Down Expand Up @@ -416,8 +416,8 @@ firrtl.circuit "ProbeOpsDisableA" attributes {
(out out : !firrtl.probe<uint<1>, @A>)
}

// CHECK: firrtl.extmodule @ExtModule()
firrtl.extmodule @ExtModule(out out : !firrtl.probe<uint<1>, @A>)
// CHECK: firrtl.extmodule private @ExtModule()
firrtl.extmodule private @ExtModule(out out : !firrtl.probe<uint<1>, @A>)
}

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -451,11 +451,11 @@ firrtl.circuit "HierPathDelete" attributes {
}
}

firrtl.extmodule @Leaf()
firrtl.extmodule private @Leaf()

// CHECK-NOT: hw.hierpath private @DeletedPath [@Deleted]
hw.hierpath private @DeletedPath [@Deleted]
firrtl.extmodule @Deleted() attributes {layers = [@Layer]}
firrtl.extmodule private @Deleted() attributes {layers = [@Layer]}
}

// CHECK-LABEL: firrtl.circuit "HierPathDelete2"
Expand Down Expand Up @@ -485,11 +485,11 @@ firrtl.circuit "HierPathDelete2" attributes {
}
}

firrtl.extmodule @Leaf()
firrtl.extmodule private @Leaf()

// CHECK-NOT: hw.hierpath private @DeletedPath [@Deleted]
hw.hierpath private @DeletedPath [@Deleted]
firrtl.extmodule @Deleted() attributes {layers = [@Layer]}
firrtl.extmodule private @Deleted() attributes {layers = [@Layer]}
}


Expand Down

0 comments on commit 4cdd13c

Please sign in to comment.