From aeab8ed08eb4d604ed773bf6109530adbe3af6f3 Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Tue, 9 Jul 2024 12:01:33 -0700 Subject: [PATCH] Add support for SystemVerilog parameter passthroughs (#497) --- CHANGELOG.md | 2 +- lib/src/modules/bus.dart | 36 +++--- lib/src/synthesizers/systemverilog.dart | 163 ++++++++++++++++++++---- test/external_test.dart | 7 +- test/name_test.dart | 9 +- test/sv_param_passthrough.sv | 22 ++++ test/sv_param_passthrough_test.dart | 131 +++++++++++++++++++ 7 files changed, 319 insertions(+), 51 deletions(-) create mode 100644 test/sv_param_passthrough.sv create mode 100644 test/sv_param_passthrough_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index b2274dced..326a3a348 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## Next release - Added `LogicNet`, `inOut`s, and `TriStateBuffer` to enable multi-directional wires, ports, and drivers. -- Deprecated `CustomSystemVerilog` in favor of `SystemVerilog`, which has similar functionality but supports `inOut` ports and collapses all ports into a single `ports` argument. +- Deprecated `CustomSystemVerilog` in favor of `SystemVerilog`, which has similar functionality but supports `inOut` ports, and collapses all ports into a single `ports` argument, as well as some other new features like custom definitions and parameter passthroughs. - Breaking: `ExternalSystemVerilogModule` and `InlineSystemVerilog` now extend `SystemVerilog` instead of `CustomSystemVerilog`, meaning the `instantiationVerilog` API arguments have been modified. - Breaking: Increased minimum Dart SDK version to 3.0.0. - Breaking: `Interface.connectIO` has an additional optional named argument for `inOutTags`. Implementations of `Interface` which override `connectIO` will need to be updated. diff --git a/lib/src/modules/bus.dart b/lib/src/modules/bus.dart index 800f3930c..755d4c42b 100644 --- a/lib/src/modules/bus.dart +++ b/lib/src/modules/bus.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // bus.dart @@ -15,16 +15,16 @@ import 'package:rohd/rohd.dart'; /// The output [subset] will have width equal to `|endIndex - startIndex| + 1`. class BusSubset extends Module with InlineSystemVerilog { /// Name for the input port of this module. - late final String _original; + late final String _originalName; /// Name for the output port of this module. - late final String _subset; + late final String _subsetName; /// The input to get a subset of. - late final Logic original = input(_original); + late final Logic _original = input(_originalName); - /// The output, a subset of [original]. - late final Logic subset = output(_subset); + /// The output, a subset of [_original]. + late final Logic subset = output(_subsetName); /// Start index of the subset. final int startIndex; @@ -33,7 +33,7 @@ class BusSubset extends Module with InlineSystemVerilog { final int endIndex; @override - List get expressionlessInputs => [_original]; + List get expressionlessInputs => [_originalName]; /// Constructs a [Module] that accesses a subset from [bus] which ranges /// from [startIndex] to [endIndex] (inclusive of both). @@ -55,13 +55,13 @@ class BusSubset extends Module with InlineSystemVerilog { ' than ${bus.width}'); } - _original = Naming.unpreferredName('original_${bus.name}'); - _subset = + _originalName = Naming.unpreferredName('original_${bus.name}'); + _subsetName = Naming.unpreferredName('subset_${endIndex}_${startIndex}_${bus.name}'); - addInput(_original, bus, width: bus.width); + addInput(_originalName, bus, width: bus.width); final newWidth = (endIndex - startIndex).abs() + 1; - addOutput(_subset, width: newWidth); + addOutput(_subsetName, width: newWidth); // so that people can't do a slice assign, not (yet?) implemented subset.makeUnassignable(); @@ -72,22 +72,22 @@ class BusSubset extends Module with InlineSystemVerilog { /// Performs setup steps for custom functional behavior. void _setup() { _execute(); // for initial values - original.glitch.listen((args) { + _original.glitch.listen((args) { _execute(); }); } /// Executes the functional behavior of this gate. void _execute() { - if (original.width == 1) { - subset.put(original.value); + if (_original.width == 1) { + subset.put(_original.value); return; } if (endIndex < startIndex) { - subset.put(original.value.getRange(endIndex, startIndex + 1).reversed); + subset.put(_original.value.getRange(endIndex, startIndex + 1).reversed); } else { - subset.put(original.value.getRange(startIndex, endIndex + 1)); + subset.put(_original.value.getRange(startIndex, endIndex + 1)); } } @@ -99,13 +99,13 @@ class BusSubset extends Module with InlineSystemVerilog { if (inputs.length != 1) { throw Exception('BusSubset has exactly one input, but saw $inputs.'); } - final a = inputs[_original]!; + final a = inputs[_originalName]!; assert(!a.contains(_expressionRegex), 'Inputs to bus swizzle cannot contain any expressions.'); // When, input width is 1, ignore startIndex and endIndex - if (original.width == 1) { + if (_original.width == 1) { return a; } diff --git a/lib/src/synthesizers/systemverilog.dart b/lib/src/synthesizers/systemverilog.dart index fd50efc72..47a065b8c 100644 --- a/lib/src/synthesizers/systemverilog.dart +++ b/lib/src/synthesizers/systemverilog.dart @@ -7,6 +7,8 @@ // 2021 August 26 // Author: Max Korbel +import 'dart:collection'; + import 'package:collection/collection.dart'; import 'package:meta/meta.dart'; import 'package:rohd/rohd.dart'; @@ -23,7 +25,8 @@ class SystemVerilogSynthesizer extends Synthesizer { bool generatesDefinition(Module module) => // ignore: deprecated_member_use_from_same_package !((module is CustomSystemVerilog) || - (module is SystemVerilog && !module.generatesDefinition)); + (module is SystemVerilog && + module.generatedDefinitionType == DefinitionGenerationType.none)); /// Creates a line of SystemVerilog that instantiates [module]. /// @@ -37,6 +40,13 @@ class SystemVerilogSynthesizer extends Synthesizer { /// Based on this module definition: `c <= a & b` /// The values for [ports] should be: /// ports: `{ 'a' : 'sig_a', 'b' : 'sig_b', 'c' : 'sig_c'}` + /// + /// If [forceStandardInstantiation] is set, then the standard instantiation + /// for SystemVerilog modules will be used. + /// + /// If [parameters] is provided, then the module will be instantiated with + /// all of the keys as parameter names set to the corresponding values + /// provided. static String instantiationVerilogFor( {required Module module, required String instanceType, @@ -135,12 +145,19 @@ class SystemVerilogSynthesizer extends Synthesizer { ); @override - SynthesisResult synthesize(Module module, - String Function(Module module) getInstanceTypeOfModule) => - module is SystemVerilog && module.generatesDefinition - ? _SystemVerilogCustomDefinitionSynthesisResult( - module, getInstanceTypeOfModule) - : _SystemVerilogSynthesisResult(module, getInstanceTypeOfModule); + SynthesisResult synthesize( + Module module, String Function(Module module) getInstanceTypeOfModule) { + assert( + module is! SystemVerilog || + module.generatedDefinitionType != DefinitionGenerationType.none, + 'SystemVerilog modules synthesized must generate a definition.'); + + return module is SystemVerilog && + module.generatedDefinitionType == DefinitionGenerationType.custom + ? _SystemVerilogCustomDefinitionSynthesisResult( + module, getInstanceTypeOfModule) + : _SystemVerilogSynthesisResult(module, getInstanceTypeOfModule); + } } /// Allows a [Module] to define a custom implementation of SystemVerilog to be @@ -165,8 +182,26 @@ mixin CustomSystemVerilog on Module { final List expressionlessInputs = const []; } -/// Allows a [Module] to define a custom implementation of SystemVerilog to be -/// injected in generated output instead of instantiating a separate `module`. +/// Represents the definition of a SystemVerilog parameter at the time of +/// declaration of a module definition. +class SystemVerilogParameterDefinition { + /// The SystemVerilog type to use for declaring this parameter. + final String type; + + /// The default value for this parameter. + final String defaultValue; + + /// The name of the parameter. + final String name; + + /// Creates a new SystemVerilog parameter definition with [name] of the + /// provided [type] with the [defaultValue]. + const SystemVerilogParameterDefinition(this.name, + {required this.type, required this.defaultValue}); +} + +/// Allows a [Module] to control the instantiation and/or definition of +/// generated SystemVerilog for that module. mixin SystemVerilog on Module { /// Generates custom SystemVerilog to be injected in place of a `module` /// instantiation. @@ -179,12 +214,14 @@ mixin SystemVerilog on Module { /// /// If a standard instantiation is desired, either return `null` or use /// [SystemVerilogSynthesizer.instantiationVerilogFor] with - /// `forceStandardInstantiation` set to `true`. + /// `forceStandardInstantiation` set to `true`. By default, `null` is + /// returned and thus a standard instantiation is used. String? instantiationVerilog( String instanceType, String instanceName, Map ports, - ); + ) => + null; /// A list of names of [input]s which should not have any SystemVerilog /// expressions (including constants) in-lined into them. Only signal names @@ -194,16 +231,51 @@ mixin SystemVerilog on Module { /// A custom SystemVerilog definition to be produced for this [Module]. /// - /// If `null` is returned, then no definition will be generated. Otherwise, - /// this function should be a pure function, i.e. it should have no side - /// effects and always return the same thing for the same inputs. - String? definitionVerilog(String definitionType) => null; + /// If an empty string is returned (the default behavior), then no definition + /// will be generated. + /// + /// If `null` is returned, then a default definition will be generated. + /// + /// This function should have no side effects and always return the same thing + /// for the same inputs. + String? definitionVerilog(String definitionType) => ''; + + /// A collection of SystemVerilog [SystemVerilogParameterDefinition]s to be + /// declared on the definition when generating SystemVerilog for this [Module] + /// if [generatedDefinitionType] is [DefinitionGenerationType.standard]. + /// + /// If `null` is returned (the default), then no parameters will be generated. + /// Otherwise, this function should have no side effects and always return the + /// same thing for the same inputs. + List? get definitionParameters => null; - /// Whether or not this [Module] generates a SystemVerilog definition. + /// What kind of SystemVerilog definition this [Module] generates, or whether + /// it does at all. /// - /// By default, this is automatically calculated by whether or not - /// [definitionVerilog] provides a definition. - bool get generatesDefinition => definitionVerilog('*PLACEHOLDER*') != null; + /// By default, this is automatically calculated based on the return value of + /// [definitionVerilog]. + DefinitionGenerationType get generatedDefinitionType { + final def = definitionVerilog('*PLACEHOLDER*'); + if (def == null) { + return DefinitionGenerationType.standard; + } else if (def.isNotEmpty) { + return DefinitionGenerationType.custom; + } else { + return DefinitionGenerationType.none; + } + } +} + +/// A type of generation for generated outputs. +enum DefinitionGenerationType { + /// No definition will be generated. + none, + + /// A standard definition will be generated. + standard, + + /// A custom definition will be generated. + custom, } /// Allows a [Module] to define a special type of [SystemVerilog] which can be @@ -246,10 +318,14 @@ mixin InlineSystemVerilog on Module implements SystemVerilog { final List expressionlessInputs = const []; @override - String? definitionVerilog(String definitionType) => null; + String? definitionVerilog(String definitionType) => ''; + + @override + DefinitionGenerationType get generatedDefinitionType => + DefinitionGenerationType.none; @override - bool get generatesDefinition => definitionVerilog('*PLACEHOLDER*') != null; + List? get definitionParameters => null; } /// A [SynthesisResult] representing a [Module] that provides a custom @@ -257,7 +333,10 @@ mixin InlineSystemVerilog on Module implements SystemVerilog { class _SystemVerilogCustomDefinitionSynthesisResult extends SynthesisResult { _SystemVerilogCustomDefinitionSynthesisResult( super.module, super.getInstanceTypeOfModule) - : assert(module is SystemVerilog && module.generatesDefinition, + : assert( + module is SystemVerilog && + module.generatedDefinitionType == + DefinitionGenerationType.custom, 'This should only be used for custom system verilog definitions.'); @override @@ -278,12 +357,15 @@ class _SystemVerilogCustomDefinitionSynthesisResult extends SynthesisResult { /// A [SynthesisResult] representing a conversion of a [Module] to /// SystemVerilog. class _SystemVerilogSynthesisResult extends SynthesisResult { - /// A cached copy of the generated ports + /// A cached copy of the generated ports. late final String _portsString; - /// A cached copy of the generated contents of the module + /// A cached copy of the generated contents of the module. late final String _moduleContentsString; + /// A cached copy of the generated parameters. + late final String? _parameterString; + /// The main [_SynthModuleDefinition] for this. final _SynthModuleDefinition _synthModuleDefinition; @@ -295,17 +377,21 @@ class _SystemVerilogSynthesisResult extends SynthesisResult { : _synthModuleDefinition = _SynthModuleDefinition(module) { _portsString = _verilogPorts(); _moduleContentsString = _verilogModuleContents(getInstanceTypeOfModule); + _parameterString = _verilogParameters(module); } @override bool matchesImplementation(SynthesisResult other) => other is _SystemVerilogSynthesisResult && other._portsString == _portsString && + other._parameterString == _parameterString && other._moduleContentsString == _moduleContentsString; @override int get matchHashCode => - _portsString.hashCode ^ _moduleContentsString.hashCode; + _portsString.hashCode ^ + _moduleContentsString.hashCode ^ + _parameterString.hashCode; @override String toFileContents() => _toVerilog(getInstanceTypeOfModule); @@ -396,11 +482,34 @@ class _SystemVerilogSynthesisResult extends SynthesisResult { ..._verilogInOuts(), ].join(',\n'); + String? _verilogParameters(Module module) { + if (module is SystemVerilog) { + final defParams = module.definitionParameters; + if (defParams == null) { + return null; + } + + return [ + '#(', + defParams + .map((p) => 'parameter ${p.type} ${p.name} = ${p.defaultValue}') + .join(',\n'), + ')', + ].join('\n'); + } + + return null; + } + /// The full SV representation of this module. String _toVerilog(String Function(Module module) getInstanceTypeOfModule) { final verilogModuleName = getInstanceTypeOfModule(module); return [ - 'module $verilogModuleName(', + [ + 'module $verilogModuleName', + _parameterString, + '(', + ].nonNulls.join(' '), _portsString, ');', _moduleContentsString, @@ -626,7 +735,7 @@ class _SynthModuleDefinition { /// A mapping from original [Logic]s to the [_SynthLogic]s that represent /// them. - final Map logicToSynthMap = {}; + final Map logicToSynthMap = HashMap(); /// A mapping from the original [Module]s to the /// [_SynthSubModuleInstantiation]s that represent them. diff --git a/test/external_test.dart b/test/external_test.dart index e7a554bdd..09ac84c87 100644 --- a/test/external_test.dart +++ b/test/external_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2022-2023 Intel Corporation +// Copyright (C) 2022-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // external_test.dart @@ -32,9 +32,14 @@ void main() { final mod = TopModule(Logic(width: 2)); await mod.build(); final sv = mod.generateSynth(); + + // make sure we instantiate the external module properly expect( sv, contains( 'external_module_name #(.WIDTH(2)) external_module(.a(a),.b(b));')); + + // make sure we don't generate the external module SV definition + expect(RegExp(r'module\s+external_module_name').hasMatch(sv), isFalse); }); } diff --git a/test/name_test.dart b/test/name_test.dart index 27e739b7d..2742c0ec8 100644 --- a/test/name_test.dart +++ b/test/name_test.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2023 Intel Corporation +// Copyright (C) 2023-2024 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // definition_name_test.dart @@ -191,14 +191,15 @@ void main() { test('respected with no conflicts', () async { final mod = SpeciallyNamedModule(Logic(), false, false); await mod.build(); - expect(mod.generateSynth(), contains('module specialName(')); + final sv = mod.generateSynth(); + expect(sv, contains('module specialName (')); }); test('uniquified with conflicts', () async { final mod = TopModule(Logic(), false, false); await mod.build(); final sv = mod.generateSynth(); - expect(sv, contains('module specialName(')); - expect(sv, contains('module specialName_0(')); + expect(sv, contains('module specialName (')); + expect(sv, contains('module specialName_0 (')); }); test('reserved throws exception with conflicts', () async { final mod = TopModule(Logic(), true, false); diff --git a/test/sv_param_passthrough.sv b/test/sv_param_passthrough.sv new file mode 100644 index 000000000..ddb0566dd --- /dev/null +++ b/test/sv_param_passthrough.sv @@ -0,0 +1,22 @@ +// Copyright (C) 2024 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// sv_param_passthrough.sv +// Module useful for testing parameter passthrough +// +// 2024 June 25 +// Author: Max Korbel + +module leaf_node #( + parameter int A = 0, + parameter int B = 0, + parameter bit[3:0] C = 0, + parameter logic D = 1'b0 +) ( + input logic [7:0] a, + output logic [7:0] b +); + +assign b = a + B; + +endmodule : leaf_node diff --git a/test/sv_param_passthrough_test.dart b/test/sv_param_passthrough_test.dart new file mode 100644 index 000000000..a92b000bc --- /dev/null +++ b/test/sv_param_passthrough_test.dart @@ -0,0 +1,131 @@ +// Copyright (C) 2024 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// sv_param_passthrough_test.dart +// Unit tests for pass-through SystemVerilog parameters +// +// 2024 June 25 +// Author: Max Korbel + +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/utilities/simcompare.dart'; +import 'package:test/test.dart'; + +const _bParam = 7; + +abstract class ModWithParamPassthrough extends Module with SystemVerilog { + ModWithParamPassthrough(this.definitionParameters, + {required this.instantiationParameters, + super.definitionName, + super.name}); + + @override + final List definitionParameters; + + final Map instantiationParameters; + + @override + String? definitionVerilog(String definitionType) => null; + + @override + String instantiationVerilog( + String instanceType, + String instanceName, + Map ports, + ) => + SystemVerilogSynthesizer.instantiationVerilogFor( + module: this, + instanceType: definitionName, + instanceName: instanceName, + ports: ports, + parameters: instantiationParameters, + forceStandardInstantiation: true, + ); +} + +class Top extends ModWithParamPassthrough { + Logic get b => output('b'); + Top(Logic a, {super.instantiationParameters = const {}, super.name = 'top'}) + : super([ + const SystemVerilogParameterDefinition('A', + type: 'int', defaultValue: '3'), + const SystemVerilogParameterDefinition('B', + type: 'int', defaultValue: '$_bParam'), + const SystemVerilogParameterDefinition('C', + type: 'bit[3:0]', defaultValue: '2'), + ]) { + a = addInput('a', a, width: 8); + addOutput('b', width: 8); + b <= + Mid( + a, + instantiationParameters: Map.fromEntries( + definitionParameters.map((e) => MapEntry(e.name, e.name))), + ).b; + } +} + +class Mid extends ModWithParamPassthrough { + Logic get b => output('b'); + Mid(Logic a, {super.instantiationParameters = const {}, super.name = 'mid'}) + : super([ + const SystemVerilogParameterDefinition('A', + type: 'int', defaultValue: '3'), + const SystemVerilogParameterDefinition('B', + type: 'int', defaultValue: '0'), + const SystemVerilogParameterDefinition('C', + type: 'bit[3:0]', defaultValue: '0'), + const SystemVerilogParameterDefinition('D', + type: 'logic', defaultValue: "1'b0"), + ]) { + a = addInput('a', a, width: 8); + addOutput('b', width: 8); + b <= + LeafNodeExternal( + a, + instantiationParameters: Map.fromEntries( + definitionParameters.map((e) => MapEntry(e.name, e.name))), + ).b; + } +} + +class LeafNodeExternal extends ModWithParamPassthrough { + Logic get b => output('b'); + LeafNodeExternal(Logic a, + {super.definitionName = 'leaf_node', + super.instantiationParameters = const {}, + super.name = 'leaf'}) + : super([ + const SystemVerilogParameterDefinition('A', + type: 'int', defaultValue: '0'), + const SystemVerilogParameterDefinition('B', + type: 'int', defaultValue: '0'), + const SystemVerilogParameterDefinition('C', + type: 'bit[3:0]', defaultValue: '0'), + const SystemVerilogParameterDefinition('D', + type: 'logic', defaultValue: '0'), + ]) { + a = addInput('a', a, width: 8); + addOutput('b', width: 8); + } + + // leaf node should not generate any SV, like external + @override + String? definitionVerilog(String definitionType) => ''; +} + +void main() { + test('passthrough params custom system verilog', () async { + final mod = Top(Logic(width: 8)); + await mod.build(); + + final vectors = [ + Vector({'a': 1}, {'b': 1 + _bParam}), + Vector({'a': 3}, {'b': 3 + _bParam}), + ]; + + SimCompare.checkIverilogVector(mod, vectors, iverilogExtraArgs: [ + 'test/sv_param_passthrough.sv', // include external SV + ]); + }); +}