Skip to content

Commit a49f464

Browse files
seldridgeseldridge
committed
[FIRRTL] Add layer verifier, no bind-under-inline
Add a verifier to the `firrtl.layer` op to disallow bind layers under inline layers. We can potentially support this in the future, however, it does not easily work now. The plan here is to add basic support for inline layers and then turn on more support as is needed. To support this in the future would require making the `` `ifdef `` logic be placed inside the bind file. To support arbitrary bind-under-inline-under-bind requires additional thought. (I do not know how to handle this latter case today.) Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
1 parent cfc8425 commit a49f464

File tree

4 files changed

+65
-42
lines changed

4 files changed

+65
-42
lines changed

‎include/circt/Dialect/FIRRTL/FIRRTLStructure.td

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,7 @@ def LayerOp : FIRRTLOp<
459459
let assemblyFormat = [{
460460
$sym_name `` $convention attr-dict-with-keyword $body
461461
}];
462+
let hasVerifier = 1;
462463
}
463464

464465
def OptionOp : FIRRTLOp<"option", [

‎lib/Dialect/FIRRTL/FIRRTLOps.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2179,6 +2179,38 @@ bool ExtClassOp::canDiscardOnUseEmpty() {
21792179
return false;
21802180
}
21812181

2182+
//===----------------------------------------------------------------------===//
2183+
// LayerOp
2184+
//===----------------------------------------------------------------------===//
2185+
2186+
LogicalResult LayerOp::verify() {
2187+
2188+
// A Bind Convention layer may not exist under an Inline Convention layer
2189+
// because we haven't implemented a lowering for it. A lowering should be
2190+
// possible, but it gets weird. This also transitively disallows a Bind
2191+
// Convention under an Inline Convention under a Bind Convention. We don't
2192+
// have a lowering for this either. Consequently, just reject this for now as
2193+
// it's a niche use case.
2194+
//
2195+
// TODO: Remove this restriction by defining a lowering for bind-under-inline
2196+
// and bind-under-inline-under-bind.
2197+
if (getConvention() == LayerConvention::Bind) {
2198+
Operation *parentOp = (*this)->getParentOp();
2199+
while (auto parentLayer = dyn_cast<LayerOp>(parentOp)) {
2200+
if (parentLayer.getConvention() == LayerConvention::Inline) {
2201+
auto diag = emitOpError() << "has bind convention and cannot be nested "
2202+
"under a layer with inline convention";
2203+
diag.attachNote(parentLayer.getLoc())
2204+
<< "layer with inline convention here";
2205+
return failure();
2206+
}
2207+
parentOp = parentOp->getParentOp();
2208+
}
2209+
}
2210+
2211+
return success();
2212+
}
2213+
21822214
//===----------------------------------------------------------------------===//
21832215
// InstanceOp
21842216
//===----------------------------------------------------------------------===//

‎test/Dialect/FIRRTL/errors.mlir

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,6 +1836,18 @@ firrtl.circuit "ClassCannotHavePortSymbols" {
18361836

18371837
// -----
18381838

1839+
// A bind layer cannot be nested under an inline layer as we can't lower it.
1840+
firrtl.circuit "BindUnderInline" {
1841+
// expected-note @below {{layer with inline convention here}}
1842+
firrtl.layer @A inline {
1843+
// expected-error @below {{has bind convention and cannot be nested under a layer with inline convention}}
1844+
firrtl.layer @B bind {}
1845+
}
1846+
firrtl.module @BindUnderInline() {}
1847+
}
1848+
1849+
// -----
1850+
18391851
// A layer block, "@A::@B", is missing an outer nesting of a layer block
18401852
// definition with symbol "@A".
18411853
firrtl.circuit "LayerBlockMissingNesting" {

‎test/Dialect/FIRRTL/lower-layers.mlir

Lines changed: 20 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -378,70 +378,48 @@ firrtl.circuit "Test" {
378378
// Inline Layers
379379
//===--------------------------------------------------------------------===//
380380

381-
// CHECK: sv.macro.decl @[[INLINE:.*]]["Inline"]
381+
// CHECK: sv.macro.decl @[[INLINE:.*]]["Inline"]
382+
// CHECK-NEXT: sv.macro.decl @Inline_Inline
383+
// CHECK-NEXT: sv.macro.decl @Bound_Inline
382384
firrtl.layer @Inline inline {
383-
// CHECK: sv.macro.decl @Inline_Inline
384385
firrtl.layer @Inline inline {}
385-
firrtl.layer @Bound bind {
386-
// CHECK: sv.macro.decl @Inline_Bound_Inline
387-
firrtl.layer @Inline inline {}
388-
firrtl.layer @Bound bind {
389-
// CHECK: sv.macro.decl @Inline_Bound_Bound_Inline
390-
firrtl.layer @Inline inline {}
391-
}
392-
}
393386
}
394387

395388
firrtl.layer @Bound bind {
396-
// CHECK: sv.macro.decl @Bound_Inline
397389
firrtl.layer @Inline inline {}
398390
}
399391

400-
// CHECK: firrtl.module private @ModuleWithInlineLayerBlocks_Inline_Bound() {
401-
// CHECK: %w3 = firrtl.wire : !firrtl.uint<3>
402-
// CHECK: sv.ifdef @Inline_Bound_Inline {
403-
// CHECK: %w4 = firrtl.wire : !firrtl.uint<4>
404-
// CHECK: }
405-
// CHECK: }
406-
407-
// CHECK: firrtl.module private @ModuleWithInlineLayerBlocks_Bound() {
408-
// CHECK: %w5 = firrtl.wire : !firrtl.uint<5>
409-
// CHECK: sv.ifdef @Bound_Inline {
410-
// CHECK: %w6 = firrtl.wire : !firrtl.uint<6>
411-
// CHECK: }
412-
// CHECK: }
392+
// CHECK: firrtl.module private @ModuleWithInlineLayerBlocks_Bound() {
393+
// CHECK-NEXT: %w3 = firrtl.wire
394+
// CHECK-NEXT: sv.ifdef @Bound_Inline {
395+
// CHECK-NEXT: %w4 = firrtl.wire
396+
// CHECK-NEXT: }
397+
// CHECK-NEXT: }
413398

414-
// CHECK: firrtl.module @ModuleWithInlineLayerBlocks() {
415-
// CHECK: sv.ifdef @[[INLINE]] {
416-
// CHECK: %w1 = firrtl.wire : !firrtl.uint<1>
417-
// CHECK: sv.ifdef @Inline_Inline {
418-
// CHECK: %w2 = firrtl.wire : !firrtl.uint<2>
419-
// CHECK: }
420-
// CHECK: firrtl.instance inline_bound {lowerToBind, output_file = #hw.output_file<"layers-Test-Inline-Bound.sv", excludeFromFileList>} @ModuleWithInlineLayerBlocks_Inline_Bound()
421-
// CHECK: }
422-
// CHECK: firrtl.instance bound {lowerToBind, output_file = #hw.output_file<"layers-Test-Bound.sv", excludeFromFileList>} @ModuleWithInlineLayerBlocks_Bound()
423-
// CHECK: }
399+
// CHECK-NEXT: firrtl.module @ModuleWithInlineLayerBlocks() {
400+
// CHECK-NEXT: sv.ifdef @[[INLINE]] {
401+
// CHECK-NEXT: %w1 = firrtl.wire
402+
// CHECK-NEXT: sv.ifdef @Inline_Inline {
403+
// CHECK-NEXT: %w2 = firrtl.wire
404+
// CHECK-NEXT: }
405+
// CHECK-NEXT: }
406+
// CHECK-NEXT: }
424407
firrtl.module @ModuleWithInlineLayerBlocks() {
425408
firrtl.layerblock @Inline {
426409
%w1 = firrtl.wire : !firrtl.uint<1>
427410
firrtl.layerblock @Inline::@Inline {
428411
%w2 = firrtl.wire : !firrtl.uint<2>
429412
}
430-
firrtl.layerblock @Inline::@Bound {
431-
%w3 = firrtl.wire : !firrtl.uint<3>
432-
firrtl.layerblock @Inline::@Bound::@Inline {
433-
%w4 = firrtl.wire : !firrtl.uint<4>
434-
}
435-
}
436413
}
437414

438415
firrtl.layerblock @Bound {
439-
%w5 = firrtl.wire : !firrtl.uint<5>
416+
%w3 = firrtl.wire : !firrtl.uint<3>
440417
firrtl.layerblock @Bound::@Inline {
441-
%w6 = firrtl.wire : !firrtl.uint<6>
418+
%w4 = firrtl.wire : !firrtl.uint<4>
442419
}
443420
}
444421
}
422+
445423
}
446424

447425
// -----

0 commit comments

Comments
 (0)