From 50e3f9fafffce414f9882c3452333ec4e898a6d5 Mon Sep 17 00:00:00 2001 From: John Demme Date: Tue, 17 Dec 2024 18:14:27 +0000 Subject: [PATCH 01/27] [circt-cocotb] Refactor slightly and add Questa support --- .../circt-cocotb-driver.py.in | 92 ++++++++++++++----- 1 file changed, 67 insertions(+), 25 deletions(-) diff --git a/tools/circt-cocotb-driver/circt-cocotb-driver.py.in b/tools/circt-cocotb-driver/circt-cocotb-driver.py.in index 844f8fbdd276..0a579f9a1f7d 100755 --- a/tools/circt-cocotb-driver/circt-cocotb-driver.py.in +++ b/tools/circt-cocotb-driver/circt-cocotb-driver.py.in @@ -5,7 +5,7 @@ import os import subprocess import sys import re -from cocotb_test.simulator import run +import cocotb_test.simulator as csim def parseArgs(args): @@ -22,7 +22,7 @@ def parseArgs(args): help="Name of the top level verilog module.") argparser.add_argument("--simulator", - choices=['icarus'], + choices=['icarus', 'questa'], default="icarus", help="Name of the simulator to use.") @@ -31,10 +31,16 @@ def parseArgs(args): required=True, help="Name of the python module.") - argparser.add_argument("--pythonFolders", - type=str, - default="", - help="The folders where cocotb should include from, separated by commas.") + argparser.add_argument( + "--pythonFolders", + type=str, + default="", + help="The folders where cocotb should include from, separated by commas.") + + argparser.add_argument( + "--gui", + action="store_true", + help="Run the simulator with a GUI. Only supported by Questa.") argparser.add_argument( "sources", @@ -44,10 +50,12 @@ def parseArgs(args): return argparser.parse_args(args[1:]) -class _IVerilogHandler: +class Icarus(csim.Icarus): """ Class for handling icarus-verilog specific commands and patching.""" - def __init__(self): + def __init__(self, **kwargs): + super().__init__(**kwargs) + # Ensure that iverilog is available in path and it is at least iverilog v11 try: out = subprocess.check_output(["iverilog", "-V"]) @@ -64,7 +72,42 @@ class _IVerilogHandler: if float(ver) < 11: raise Exception(f"Icarus Verilog version must be >= 11, got {ver}") - def extra_compile_args(self, objDir): + def run(self, objDir, gui=False): + if gui: + raise Exception("GUI is not supported by Icarus Verilog") + + # If no timescale is defined in the source code, icarus assumes a + # timescale of '1'. This prevents cocotb from creating small timescale clocks. + # Since a timescale is not emitted by default from export-verilog, make our + # lives easier and create a minimum timescale through the command-line. + cmd_file = os.path.join(objDir, "cmds.f") + with open(cmd_file, "w+") as f: + f.write("+timescale+1ns/1ps") + + self.compile_args.append(f"-f{cmd_file}") + return super().run() + + +class Questa(csim.Questa): + """ Class for handling icarus-verilog specific commands and patching.""" + + def __init__(self, **kwargs): + super().__init__(**kwargs) + + # Ensure that iverilog is available in path and it is at least iverilog v11 + try: + out = subprocess.check_output(["vlog", "-version"]) + except subprocess.CalledProcessError: + raise Exception("vlog not found in path") + + # find the 'Icarus Verilog version #' string and extract the version number + # using a regex + ver_re = r"QuestaSim-64 vlog (\d+\.\d+)" + ver_match = re.search(ver_re, out.decode("utf-8")) + if ver_match is None: + raise Exception("Could not find Questa version") + + def run(self, objDir, gui=False): # If no timescale is defined in the source code, icarus assumes a # timescale of '1'. This prevents cocotb from creating small timescale clocks. # Since a timescale is not emitted by default from export-verilog, make our @@ -73,7 +116,9 @@ class _IVerilogHandler: with open(cmd_file, "w+") as f: f.write("+timescale+1ns/1ps") - return [f"-f{cmd_file}"] + self.gui = gui + self.extra_compile_args = ["-f", f"{cmd_file}"] + return super().run() def main(): @@ -96,28 +141,25 @@ def main(): except subprocess.CalledProcessError: raise Exception( "'make' is not available, and is required to run cocotb tests.") - + kwargs = { + "module": args.pythonModule, + "toplevel": args.topLevel, + "toplevel_lang": "verilog", + "verilog_sources": sources, + "python_search": [f.strip() for f in args.pythonFolders.split(",")], + "work_dir": objDir, + } try: if args.simulator == "icarus": - simhandler = _IVerilogHandler() + sim = Icarus(**kwargs) + elif args.simulator == "questa": + sim = Questa(**kwargs) else: raise Exception(f"Unknown simulator: {args.simulator}") except Exception as e: raise Exception(f"Failed to initialize simulator handler: {e}") - # Simulator-specific extra compile args. - compileArgs = [] - if simhandler: - compileArgs += simhandler.extra_compile_args(objDir) - - run(simulator=args.simulator, - module=args.pythonModule, - toplevel=args.topLevel, - toplevel_lang="verilog", - verilog_sources=sources, - python_search=[f.strip() for f in args.pythonFolders.split(",")], - work_dir=objDir, - compile_args=compileArgs) + sim.run(objDir, gui=args.gui) if __name__ == "__main__": From 9775df2cb438c50907c3c46e659aca500eacbce5 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Tue, 17 Dec 2024 13:11:34 -0700 Subject: [PATCH 02/27] [FIRRTL] Support MarkDUTAnnotation on extmodules. (#8001) In some cases, the "DUT" might be an extmodule from a separate compilation unit, and we still want all the old legacy "is DUT" logic to work. To support this, we need applyDUTAnno to allow the annotation to be applied to an extmodule, and we need extractDUT and its users to work with FModuleLikes instead of FModuleOp. The only other thing that appears to be using this "is DUT" logic is the newer InstanceInfo helper, which already works with igraph::ModuleOpInterfaces and seems to work fine with extmodules as the "DUT". --- .../circt/Dialect/FIRRTL/FIRRTLAnnotations.h | 2 +- lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp | 3 +- .../FIRRTL/Transforms/BlackBoxReader.cpp | 4 +- .../FIRRTL/Transforms/GrandCentral.cpp | 4 +- .../FIRRTL/Transforms/LowerAnnotations.cpp | 4 +- test/Analysis/firrtl-test-instance-info.mlir | 37 +++++++++++++++++++ test/firtool/mark-dut.fir | 29 +++++++++++++++ 7 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 test/firtool/mark-dut.fir diff --git a/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h b/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h index 4682a05ff613..29709bdff81d 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLAnnotations.h @@ -448,7 +448,7 @@ struct PortAnnoTarget : public AnnoTarget { /// found or if the DUT was found and a previous DUT was not set (if `dut` is /// null). This returns failure if a DUT was found and a previous DUT was set. /// This function generates an error message in the failure case. -LogicalResult extractDUT(FModuleOp mod, FModuleOp &dut); +LogicalResult extractDUT(FModuleLike mod, FModuleLike &dut); } // namespace firrtl } // namespace circt diff --git a/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp b/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp index d5dc2b29f05a..a733d32d3a40 100644 --- a/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLAnnotations.cpp @@ -564,7 +564,8 @@ FIRRTLType PortAnnoTarget::getType() const { // TODO: Remove these in favor of first-class annotations. //===----------------------------------------------------------------------===// -LogicalResult circt::firrtl::extractDUT(const FModuleOp mod, FModuleOp &dut) { +LogicalResult circt::firrtl::extractDUT(const FModuleLike mod, + FModuleLike &dut) { if (!AnnotationSet(mod).hasAnnotation(dutAnnoClass)) return success(); diff --git a/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp b/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp index 0cfa24d1b9bd..42907d951186 100644 --- a/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp +++ b/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp @@ -104,7 +104,7 @@ struct BlackBoxReaderPass /// The design-under-test (DUT) as indicated by the presence of a /// "sifive.enterprise.firrtl.MarkDUTAnnotation". This will be null if no /// annotation is present. - FModuleOp dut; + FModuleLike dut; /// The file list file name (sic) for black boxes. If set, generates a file /// that lists all non-header source files for black boxes. Can be changed @@ -207,7 +207,7 @@ void BlackBoxReaderPass::runOnOperation() { // Do a shallow walk of the circuit to collect information necessary before we // do real work. for (auto &op : *circuitOp.getBodyBlock()) { - FModuleOp module = dyn_cast(op); + FModuleLike module = dyn_cast(op); // Find the DUT if it exists or error if there are multiple DUTs. if (module) if (failed(extractDUT(module, dut))) diff --git a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp index 92d448691a3e..fdd4853ac5f7 100644 --- a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp +++ b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp @@ -613,7 +613,7 @@ struct GrandCentralPass /// The design-under-test (DUT) as determined by the presence of a /// "sifive.enterprise.firrtl.MarkDUTAnnotation". This will be null if no DUT /// was found. - FModuleOp dut; + FModuleLike dut; /// An optional directory for testbench-related files. This is null if no /// "TestBenchDirAnnotation" is found. @@ -1577,7 +1577,7 @@ void GrandCentralPass::runOnOperation() { // Find the DUT if it exists. This needs to be known before the circuit is // walked. - for (auto mod : circuitOp.getOps()) { + for (auto mod : circuitOp.getOps()) { if (failed(extractDUT(mod, dut))) removalError = true; } diff --git a/lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp b/lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp index 178a257f6bba..d8ae6b45b9b6 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerAnnotations.cpp @@ -246,10 +246,10 @@ static LogicalResult applyDUTAnno(const AnnoPathValue &target, if (!target.isLocal()) return mlir::emitError(loc) << "must be local"; - if (!isa(target.ref) || !isa(op)) + if (!isa(target.ref) || !isa(op)) return mlir::emitError(loc) << "can only target to a module"; - auto moduleOp = cast(op); + auto moduleOp = cast(op); // DUT has public visibility. moduleOp.setPublic(); diff --git a/test/Analysis/firrtl-test-instance-info.mlir b/test/Analysis/firrtl-test-instance-info.mlir index ad3802907716..ec38da6a2418 100644 --- a/test/Analysis/firrtl-test-instance-info.mlir +++ b/test/Analysis/firrtl-test-instance-info.mlir @@ -487,3 +487,40 @@ firrtl.circuit "Foo" { firrtl.instance a {lowerToBind} @Foo_A() } } + +// ----- + +// Test that the DUT can be an extmodule +// CHECK: - operation: firrtl.circuit "Testharness" +// CHECK-NEXT: hasDut: true +// CHECK-NEXT: dut: firrtl.extmodule private @DUT +// CHECK-NEXT: effectiveDut: firrtl.extmodule private @DUT +firrtl.circuit "Testharness" { + // CHECK: - operation: firrtl.module @Testharness + // CHECK-NEXT: isDut: false + // CHECK-NEXT: anyInstanceUnderDut: false + // CHECK-NEXT: allInstancesUnderDut: false + firrtl.module @Testharness() { + firrtl.instance dut @DUT() + firrtl.instance foo @Foo() + } + + // CHECK: - operation: firrtl.extmodule private @DUT + // CHECK-NEXT: isDut: true + // CHECK-NEXT: anyInstanceUnderDut: true + // CHECK-NEXT: allInstancesUnderDut: true + firrtl.extmodule private @DUT() attributes { + annotations = [ + { + class = "sifive.enterprise.firrtl.MarkDUTAnnotation" + } + ] + } + + // CHECK: - operation: firrtl.module private @Foo + // CHECK-NEXT: isDut: false + // CHECK-NEXT: anyInstanceUnderDut: false + // CHECK-NEXT: allInstancesUnderDut: false + firrtl.module private @Foo() { + } +} diff --git a/test/firtool/mark-dut.fir b/test/firtool/mark-dut.fir new file mode 100644 index 000000000000..3161a3ad472e --- /dev/null +++ b/test/firtool/mark-dut.fir @@ -0,0 +1,29 @@ +; RUN: firtool %s -ir-verilog | FileCheck %s + +FIRRTL version 4.1.0 + +; COM: Check that we can even target an extmodule. +circuit Test : %[[ + { + "class": "sifive.enterprise.firrtl.MarkDUTAnnotation", + "target": "~Test|DUT" + } +]] + ; CHECK: hw.hierpath private [[DUT_NLA:@.+]] [@Test::[[DUT_SYM:@.+]]] + public module Test : + input in : UInt<1> + output out : UInt<1> + + ; CHECK: hw.instance "dut" sym [[DUT_SYM]] + inst dut of DUT + + connect dut.in, in + connect out, dut.out + + extmodule DUT : + input in : UInt<1> + output out : UInt<1> + + ; COM: Check that metadata includes the dutModulePath pointing to Test::dut. + ; CHECK: om.class @SiFive_Metadata(%basepath: !om.basepath) -> (dutModulePath + ; CHECK-NEXT: om.path_create instance %basepath [[DUT_NLA]] From 6f20eee285db0b7606ba9118b7acf29d3e1602e4 Mon Sep 17 00:00:00 2001 From: Martin Erhart Date: Tue, 17 Dec 2024 23:10:56 +0100 Subject: [PATCH 03/27] [RTG] Add operation and types to represent labels (#7964) --- include/circt-c/Dialect/RTG.h | 6 ++ include/circt/Dialect/RTG/IR/CMakeLists.txt | 5 ++ include/circt/Dialect/RTG/IR/RTGDialect.h | 3 + include/circt/Dialect/RTG/IR/RTGOps.td | 57 +++++++++++++++++++ include/circt/Dialect/RTG/IR/RTGTypes.td | 12 ++++ include/circt/Dialect/RTG/IR/RTGVisitors.h | 16 ++++-- .../Bindings/Python/dialects/rtg.py | 5 +- lib/Bindings/Python/RTGModule.cpp | 8 +++ lib/CAPI/Dialect/RTG.cpp | 9 +++ lib/Dialect/RTG/IR/CMakeLists.txt | 1 + lib/Dialect/RTG/IR/RTGDialect.cpp | 2 + test/CAPI/rtg.c | 10 ++++ test/Dialect/RTG/IR/basic.mlir | 11 ++++ test/Dialect/RTG/IR/cse.mlir | 23 ++++++++ 14 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 test/Dialect/RTG/IR/cse.mlir diff --git a/include/circt-c/Dialect/RTG.h b/include/circt-c/Dialect/RTG.h index 86a0e5a5bc51..c33afa9411a5 100644 --- a/include/circt-c/Dialect/RTG.h +++ b/include/circt-c/Dialect/RTG.h @@ -31,6 +31,12 @@ MLIR_CAPI_EXPORTED bool rtgTypeIsASequence(MlirType type); /// Creates an RTG sequence type in the context. MLIR_CAPI_EXPORTED MlirType rtgSequenceTypeGet(MlirContext ctxt); +/// If the type is an RTG label. +MLIR_CAPI_EXPORTED bool rtgTypeIsALabel(MlirType type); + +/// Creates an RTG mode type in the context. +MLIR_CAPI_EXPORTED MlirType rtgLabelTypeGet(MlirContext ctxt); + /// If the type is an RTG set. MLIR_CAPI_EXPORTED bool rtgTypeIsASet(MlirType type); diff --git a/include/circt/Dialect/RTG/IR/CMakeLists.txt b/include/circt/Dialect/RTG/IR/CMakeLists.txt index d06b8e760cf0..072a9d3c6dbb 100644 --- a/include/circt/Dialect/RTG/IR/CMakeLists.txt +++ b/include/circt/Dialect/RTG/IR/CMakeLists.txt @@ -2,6 +2,11 @@ add_circt_dialect(RTG rtg) add_circt_doc(RTG Dialects/RTGOps -gen-op-doc) add_circt_doc(RTG Dialects/RTGTypes -gen-typedef-doc -dialect rtg) +set(LLVM_TARGET_DEFINITIONS RTG.td) +mlir_tablegen(RTGEnums.h.inc -gen-enum-decls) +mlir_tablegen(RTGEnums.cpp.inc -gen-enum-defs) +add_public_tablegen_target(CIRCTRTGEnumsIncGen) + set(LLVM_TARGET_DEFINITIONS RTGInterfaces.td) mlir_tablegen(RTGOpInterfaces.h.inc -gen-op-interface-decls) mlir_tablegen(RTGOpInterfaces.cpp.inc -gen-op-interface-defs) diff --git a/include/circt/Dialect/RTG/IR/RTGDialect.h b/include/circt/Dialect/RTG/IR/RTGDialect.h index beb8e47a4ffe..ec12d965acce 100644 --- a/include/circt/Dialect/RTG/IR/RTGDialect.h +++ b/include/circt/Dialect/RTG/IR/RTGDialect.h @@ -16,6 +16,9 @@ #include "circt/Support/LLVM.h" #include "mlir/IR/Dialect.h" +// Pull in all enum type definitions and utility function declarations. +#include "circt/Dialect/RTG/IR/RTGEnums.h.inc" + // Pull in the Dialect definition. #include "circt/Dialect/RTG/IR/RTGDialect.h.inc" diff --git a/include/circt/Dialect/RTG/IR/RTGOps.td b/include/circt/Dialect/RTG/IR/RTGOps.td index 943d9d0dbb36..a1077c6a5c7b 100644 --- a/include/circt/Dialect/RTG/IR/RTGOps.td +++ b/include/circt/Dialect/RTG/IR/RTGOps.td @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +include "mlir/IR/EnumAttr.td" include "mlir/IR/CommonTypeConstraints.td" include "mlir/IR/CommonAttrConstraints.td" include "mlir/IR/Properties.td" @@ -97,6 +98,62 @@ def InvokeSequenceOp : RTGOp<"invoke_sequence", []> { let assemblyFormat = "$sequence attr-dict"; } +//===- Label Operations ---------------------------------------------------===// + +class LabelDeclBase traits> : RTGOp { + let description = [{ + Declares a label that can then be placed by an `rtg.label` operation in an + instruction sequence, passed on to sequences via their arguments, and used + by instructions (e.g., as jump targets) by allowing ISA dialects to use them + directly as an operand of an instruction or by casting it to a value + representing an immediate. + + The format string may contain placeholders of the form `{i}` where `i` + refers to the i-th element in `args`. + The declared label is uniqued by the compiler to no collide with any other + label declarations. + }]; + + // TODO: 'args' can be generalized to more types + let arguments = (ins StrAttr:$formatString, Variadic:$args); + + let assemblyFormat = [{ + $formatString (`,` $args^)? attr-dict + }]; +} + +def LabelDeclOp : LabelDeclBase<"label_decl", [Pure]> { + let summary = "declares a label for an instruction sequence"; + let results = (outs LabelType:$label); +} + +def LabelUniqueDeclOp : LabelDeclBase<"label_unique_decl", []> { + let summary = "declares a unique label for an instruction sequence"; + let results = (outs Res:$label); +} + +def LabelVisibilityAttr : I32EnumAttr<"LabelVisibility", + "visibility specifiers for labels", [ + I32EnumAttrCase<"local", 0>, + I32EnumAttrCase<"global", 1>, + I32EnumAttrCase<"external", 2>, +]> { + let cppNamespace = "::circt::rtg"; +} + +def LabelOp : RTGOp<"label", []> { + let summary = "places a label in an instruction sequence"; + let description = [{ + Any declared label must only be placed at most once in any fully elaborated + instruction sequence. + }]; + + let arguments = (ins LabelVisibilityAttr:$visibility, LabelType:$label); + + let assemblyFormat = "$visibility $label attr-dict"; +} + //===- Set Operations ------------------------------------------------------===// def SetCreateOp : RTGOp<"set_create", [Pure, SameTypeOperands]> { diff --git a/include/circt/Dialect/RTG/IR/RTGTypes.td b/include/circt/Dialect/RTG/IR/RTGTypes.td index e2c8c83ef933..1a4946a14613 100644 --- a/include/circt/Dialect/RTG/IR/RTGTypes.td +++ b/include/circt/Dialect/RTG/IR/RTGTypes.td @@ -29,6 +29,18 @@ def SequenceType : RTGTypeDef<"Sequence"> { let assemblyFormat = ""; } +def LabelType : RTGTypeDef<"Label"> { + let summary = "a reference to a label"; + let description = [{ + This type represents a label. Payload dialects can add operations to cast + from this type to an immediate type they can use as an operand to an + instruction. + }]; + + let mnemonic = "label"; + let assemblyFormat = ""; +} + def SetType : RTGTypeDef<"Set"> { let summary = "a set of values"; let description = [{ diff --git a/include/circt/Dialect/RTG/IR/RTGVisitors.h b/include/circt/Dialect/RTG/IR/RTGVisitors.h index aea845f5abe2..00a62ec8ca1f 100644 --- a/include/circt/Dialect/RTG/IR/RTGVisitors.h +++ b/include/circt/Dialect/RTG/IR/RTGVisitors.h @@ -35,10 +35,10 @@ class RTGOpVisitor { SetSelectRandomOp, SetDifferenceOp, SetUnionOp, SetSizeOp, TestOp, InvokeSequenceOp, BagCreateOp, BagSelectRandomOp, BagDifferenceOp, BagUnionOp, - BagUniqueSizeOp, TargetOp, YieldOp>( - [&](auto expr) -> ResultType { - return thisCast->visitOp(expr, args...); - }) + BagUniqueSizeOp, LabelDeclOp, LabelUniqueDeclOp, LabelOp, + TargetOp, YieldOp>([&](auto expr) -> ResultType { + return thisCast->visitOp(expr, args...); + }) .template Case([&](auto expr) -> ResultType { return thisCast->visitRegisterOp(expr, args...); }) @@ -97,6 +97,9 @@ class RTGOpVisitor { HANDLE(BagDifferenceOp, Unhandled); HANDLE(BagUnionOp, Unhandled); HANDLE(BagUniqueSizeOp, Unhandled); + HANDLE(LabelDeclOp, Unhandled); + HANDLE(LabelUniqueDeclOp, Unhandled); + HANDLE(LabelOp, Unhandled); HANDLE(TestOp, Unhandled); HANDLE(TargetOp, Unhandled); HANDLE(YieldOp, Unhandled); @@ -111,8 +114,8 @@ class RTGTypeVisitor { ResultType dispatchTypeVisitor(Type type, ExtraArgs... args) { auto *thisCast = static_cast(this); return TypeSwitch(type) - .template Case([&](auto expr) -> ResultType { + .template Case([&](auto expr) -> ResultType { return thisCast->visitType(expr, args...); }) .template Case( @@ -160,6 +163,7 @@ class RTGTypeVisitor { HANDLE(DictType, Unhandled); HANDLE(IndexType, Unhandled); HANDLE(IntegerType, Unhandled); + HANDLE(LabelType, Unhandled); #undef HANDLE }; diff --git a/integration_test/Bindings/Python/dialects/rtg.py b/integration_test/Bindings/Python/dialects/rtg.py index b863760e86c8..32000f14172d 100644 --- a/integration_test/Bindings/Python/dialects/rtg.py +++ b/integration_test/Bindings/Python/dialects/rtg.py @@ -84,11 +84,12 @@ with InsertionPoint(m.body): indexTy = IndexType.get() sequenceTy = rtg.SequenceType.get() + labelTy = rtg.LabelType.get() setTy = rtg.SetType.get(indexTy) bagTy = rtg.BagType.get(indexTy) seq = rtg.SequenceOp('seq') - Block.create_at_start(seq.bodyRegion, [sequenceTy, setTy, bagTy]) + Block.create_at_start(seq.bodyRegion, [sequenceTy, labelTy, setTy, bagTy]) # CHECK: rtg.sequence @seq - # CHECK: (%{{.*}}: !rtg.sequence, %{{.*}}: !rtg.set, %{{.*}}: !rtg.bag): + # CHECK: (%{{.*}}: !rtg.sequence, %{{.*}}: !rtg.label, %{{.*}}: !rtg.set, %{{.*}}: !rtg.bag): print(m) diff --git a/lib/Bindings/Python/RTGModule.cpp b/lib/Bindings/Python/RTGModule.cpp index e6b25523e8af..d5729868ab2b 100644 --- a/lib/Bindings/Python/RTGModule.cpp +++ b/lib/Bindings/Python/RTGModule.cpp @@ -32,6 +32,14 @@ void circt::python::populateDialectRTGSubmodule(py::module &m) { }, py::arg("self"), py::arg("ctxt") = nullptr); + mlir_type_subclass(m, "LabelType", rtgTypeIsALabel) + .def_classmethod( + "get", + [](py::object cls, MlirContext ctxt) { + return cls(rtgLabelTypeGet(ctxt)); + }, + py::arg("self"), py::arg("ctxt") = nullptr); + mlir_type_subclass(m, "SetType", rtgTypeIsASet) .def_classmethod( "get", diff --git a/lib/CAPI/Dialect/RTG.cpp b/lib/CAPI/Dialect/RTG.cpp index 7f8bf120a302..e5cf0ee4ff6b 100644 --- a/lib/CAPI/Dialect/RTG.cpp +++ b/lib/CAPI/Dialect/RTG.cpp @@ -36,6 +36,15 @@ MlirType rtgSequenceTypeGet(MlirContext ctxt) { return wrap(SequenceType::get(unwrap(ctxt))); } +// LabelType +//===----------------------------------------------------------------------===// + +bool rtgTypeIsALabel(MlirType type) { return isa(unwrap(type)); } + +MlirType rtgLabelTypeGet(MlirContext ctxt) { + return wrap(LabelType::get(unwrap(ctxt))); +} + // SetType //===----------------------------------------------------------------------===// diff --git a/lib/Dialect/RTG/IR/CMakeLists.txt b/lib/Dialect/RTG/IR/CMakeLists.txt index 327e5ff3457b..2466e6fa79f1 100644 --- a/lib/Dialect/RTG/IR/CMakeLists.txt +++ b/lib/Dialect/RTG/IR/CMakeLists.txt @@ -11,6 +11,7 @@ add_circt_dialect_library(CIRCTRTGDialect DEPENDS MLIRRTGIncGen + CIRCTRTGEnumsIncGen CIRCTRTGOpInterfacesIncGen CIRCTRTGISAAssemblyOpInterfacesIncGen CIRCTRTGTypeInterfacesIncGen diff --git a/lib/Dialect/RTG/IR/RTGDialect.cpp b/lib/Dialect/RTG/IR/RTGDialect.cpp index e517c13d574d..98541ade057b 100644 --- a/lib/Dialect/RTG/IR/RTGDialect.cpp +++ b/lib/Dialect/RTG/IR/RTGDialect.cpp @@ -33,4 +33,6 @@ void RTGDialect::initialize() { >(); } +#include "circt/Dialect/RTG/IR/RTGEnums.cpp.inc" + #include "circt/Dialect/RTG/IR/RTGDialect.cpp.inc" diff --git a/test/CAPI/rtg.c b/test/CAPI/rtg.c index 272263f1a849..f36afac66b2e 100644 --- a/test/CAPI/rtg.c +++ b/test/CAPI/rtg.c @@ -24,6 +24,15 @@ static void testSequenceType(MlirContext ctx) { mlirTypeDump(sequenceTy); } +static void testLabelType(MlirContext ctx) { + MlirType labelTy = rtgLabelTypeGet(ctx); + + // CHECK: is_label + fprintf(stderr, rtgTypeIsALabel(labelTy) ? "is_label\n" : "isnot_label\n"); + // CHECK: !rtg.label + mlirTypeDump(labelTy); +} + static void testSetType(MlirContext ctx) { MlirType elTy = mlirIntegerTypeGet(ctx, 32); MlirType setTy = rtgSetTypeGet(elTy); @@ -71,6 +80,7 @@ int main(int argc, char **argv) { mlirDialectHandleLoadDialect(mlirGetDialectHandle__rtg__(), ctx); testSequenceType(ctx); + testLabelType(ctx); testSetType(ctx); testBagType(ctx); testDictType(ctx); diff --git a/test/Dialect/RTG/IR/basic.mlir b/test/Dialect/RTG/IR/basic.mlir index d3c29199a962..29c817b0e41a 100644 --- a/test/Dialect/RTG/IR/basic.mlir +++ b/test/Dialect/RTG/IR/basic.mlir @@ -3,6 +3,17 @@ // CHECK-LABEL: rtg.sequence @seq // CHECK-SAME: attributes {rtg.some_attr} { rtg.sequence @seq0 attributes {rtg.some_attr} { + %arg = arith.constant 1 : index + // CHECK: [[LBL0:%.*]] = rtg.label_decl "label_string_{0}_{1}", %{{.*}}, %{{.*}} + %0 = rtg.label_decl "label_string_{0}_{1}", %arg, %arg + // CHECK: [[LBL1:%.+]] = rtg.label_unique_decl "label_string" + %1 = rtg.label_unique_decl "label_string" + // CHECK: rtg.label local [[LBL0]] + rtg.label local %0 + // CHECK: rtg.label global [[LBL1]] + rtg.label global %1 + // CHECK: rtg.label external [[LBL0]] + rtg.label external %0 } // CHECK-LABEL: rtg.sequence @seq1 diff --git a/test/Dialect/RTG/IR/cse.mlir b/test/Dialect/RTG/IR/cse.mlir new file mode 100644 index 000000000000..49e7a3f736af --- /dev/null +++ b/test/Dialect/RTG/IR/cse.mlir @@ -0,0 +1,23 @@ +// RUN: circt-opt --cse %s | FileCheck %s + +// CHECK-LABEL: rtg.sequence @seq +// CHECK-SAME: attributes {rtg.some_attr} { +rtg.sequence @seq0 attributes {rtg.some_attr} { + // CHECK-NEXT: arith.constant + %arg = arith.constant 1 : index + // CHECK-NEXT: rtg.label_decl "label_string_{0}_{1}", %{{.*}}, %{{.*}} + // They are CSE'd and DCE'd + %0 = rtg.label_decl "label_string_{0}_{1}", %arg, %arg + %1 = rtg.label_decl "label_string_{0}_{1}", %arg, %arg + // CHECK-NEXT: rtg.label_unique_decl "label_string" + // CHECK-NEXT: rtg.label_unique_decl "label_string" + // They are DCE'd but not CSE'd + %2 = rtg.label_unique_decl "label_string" + %3 = rtg.label_unique_decl "label_string" + %4 = rtg.label_unique_decl "label_string" + // CHECK-NEXT: rtg.label global + rtg.label global %0 + rtg.label global %1 + rtg.label global %2 + rtg.label global %3 +} From 9ba70e089dbd227534787c0cf1da66d2abbb7032 Mon Sep 17 00:00:00 2001 From: John Demme Date: Tue, 17 Dec 2024 22:26:29 +0000 Subject: [PATCH 04/27] [circt-cocotb] Bumping iverilog to 12, fixing cocotb-driver - Icarus Verilog 12 doesn't like lines without endings - Add flag to ignore assertions which Icarus doesn't support. --- tools/circt-cocotb-driver/circt-cocotb-driver.py.in | 7 +++++-- utils/get-iverilog.sh | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) mode change 100644 => 100755 utils/get-iverilog.sh diff --git a/tools/circt-cocotb-driver/circt-cocotb-driver.py.in b/tools/circt-cocotb-driver/circt-cocotb-driver.py.in index 0a579f9a1f7d..0d6ccfbd3ae5 100755 --- a/tools/circt-cocotb-driver/circt-cocotb-driver.py.in +++ b/tools/circt-cocotb-driver/circt-cocotb-driver.py.in @@ -82,9 +82,12 @@ class Icarus(csim.Icarus): # lives easier and create a minimum timescale through the command-line. cmd_file = os.path.join(objDir, "cmds.f") with open(cmd_file, "w+") as f: - f.write("+timescale+1ns/1ps") + f.write("+timescale+1ns/1ps\n") - self.compile_args.append(f"-f{cmd_file}") + self.compile_args.extend([ + f"-f{cmd_file}", + "-gsupported-assertions", # Enable SV assertions which are supported. + ]) return super().run() diff --git a/utils/get-iverilog.sh b/utils/get-iverilog.sh old mode 100644 new mode 100755 index a3d27c498b50..a8e0c7778c02 --- a/utils/get-iverilog.sh +++ b/utils/get-iverilog.sh @@ -14,7 +14,7 @@ mkdir -p "$(dirname "$BASH_SOURCE[0]")/../ext" EXT_DIR=$(cd "$(dirname "$BASH_SOURCE[0]")/../ext" && pwd) -IVERILOG_VER=11_0 +IVERILOG_VER=12_0 echo $EXT_DIR cd $EXT_DIR From e508c74252284cbf5d1a058ad1c007594f267636 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Wed, 18 Dec 2024 08:17:36 -0700 Subject: [PATCH 05/27] [CD] Install nanobind when building wheels. (#8005) We recently started supporting nanobind as a dependency in upstream MLIR. This is installed in our CI images, but not the Python wheel CI environment. This adds that dependency to the package used for the Python wheel build. --- lib/Bindings/Python/pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Bindings/Python/pyproject.toml b/lib/Bindings/Python/pyproject.toml index fe4456b1b92a..2b65896e0214 100644 --- a/lib/Bindings/Python/pyproject.toml +++ b/lib/Bindings/Python/pyproject.toml @@ -7,6 +7,7 @@ requires = [ # MLIR build depends. "numpy", "pybind11>=2.11,<=2.12", + "nanobind==2.4.0", "PyYAML", ] build-backend = "setuptools.build_meta" From 44ea9a8f447ad078bd60e8572eea42029ed466ab Mon Sep 17 00:00:00 2001 From: John Demme Date: Wed, 18 Dec 2024 12:38:45 -0500 Subject: [PATCH 06/27] [Seq] Fix FIFO lowering to correct depth and pointer increments (#8003) Fixes two bugs: - Incorrect depth constant led to FIFO never reaching capacity. - Increment pointers only worked for depth which were powers of two. Updates integration test to test for these two conditions. --- include/circt/Dialect/Seq/SeqPasses.td | 6 +- integration_test/Dialect/Seq/fifo/fifo.mlir | 4 +- integration_test/Dialect/Seq/fifo/fifo.py | 38 +++---- lib/Dialect/Seq/Transforms/CMakeLists.txt | 1 + lib/Dialect/Seq/Transforms/LowerSeqFIFO.cpp | 44 ++++++-- test/Dialect/Seq/lower-fifo.mlir | 105 +++++--------------- 6 files changed, 89 insertions(+), 109 deletions(-) diff --git a/include/circt/Dialect/Seq/SeqPasses.td b/include/circt/Dialect/Seq/SeqPasses.td index 1790ebbc1208..f6b88bc4693d 100644 --- a/include/circt/Dialect/Seq/SeqPasses.td +++ b/include/circt/Dialect/Seq/SeqPasses.td @@ -18,7 +18,11 @@ include "mlir/Pass/PassBase.td" def LowerSeqFIFO : Pass<"lower-seq-fifo", "hw::HWModuleOp"> { let summary = "Lower seq.fifo ops"; let constructor = "circt::seq::createLowerSeqFIFOPass()"; - let dependentDialects = ["circt::hw::HWDialect", "circt::comb::CombDialect"]; + let dependentDialects = [ + "circt::hw::HWDialect", + "circt::comb::CombDialect", + "circt::verif::VerifDialect" + ]; } def LowerSeqHLMem: Pass<"lower-seq-hlmem", "hw::HWModuleOp"> { diff --git a/integration_test/Dialect/Seq/fifo/fifo.mlir b/integration_test/Dialect/Seq/fifo/fifo.mlir index 54c85cd3b40d..4193a089d152 100644 --- a/integration_test/Dialect/Seq/fifo/fifo.mlir +++ b/integration_test/Dialect/Seq/fifo/fifo.mlir @@ -1,6 +1,6 @@ // REQUIRES: iverilog,cocotb -// RUN: circt-opt %s --lower-seq-fifo --lower-seq-hlmem --lower-seq-to-sv --sv-trace-iverilog --export-verilog -o %t.mlir > %t.sv +// RUN: circt-opt %s --lower-seq-fifo --lower-seq-hlmem --lower-seq-to-sv --lower-verif-to-sv --sv-trace-iverilog --export-verilog -o %t.mlir > %t.sv // RUN: circt-cocotb-driver.py --objdir=%T --topLevel=fifo \ // RUN: --pythonModule=fifo --pythonFolder="%S,%S/.." %t.sv 2>&1 @@ -8,6 +8,6 @@ // CHECK: ** TESTS=[[N:.*]] PASS=[[N]] FAIL=0 SKIP=0 hw.module @fifo(in %clk : !seq.clock, in %rst : i1, in %inp : i32, in %rdEn : i1, in %wrEn : i1, out out: i32, out empty: i1, out full: i1, out almost_empty : i1, out almost_full : i1) { - %out, %full, %empty, %almostFull, %almostEmpty = seq.fifo depth 4 almost_full 2 almost_empty 1 in %inp rdEn %rdEn wrEn %wrEn clk %clk rst %rst : i32 + %out, %full, %empty, %almostFull, %almostEmpty = seq.fifo depth 6 almost_full 2 almost_empty 1 in %inp rdEn %rdEn wrEn %wrEn clk %clk rst %rst : i32 hw.output %out, %empty, %full, %almostEmpty, %almostFull : i32, i1, i1, i1, i1 } diff --git a/integration_test/Dialect/Seq/fifo/fifo.py b/integration_test/Dialect/Seq/fifo/fifo.py index 8a6d8dbe44d9..66304798938e 100644 --- a/integration_test/Dialect/Seq/fifo/fifo.py +++ b/integration_test/Dialect/Seq/fifo/fifo.py @@ -20,6 +20,7 @@ async def initDut(dut): async def write(dut, value): dut.inp.value = value + assert dut.full.value == 0 dut.wrEn.value = 1 await clock(dut) dut.wrEn.value = 0 @@ -52,14 +53,21 @@ async def readWrite(dut, value): return data -FIFO_DEPTH = 4 +FIFO_DEPTH = 6 FIFO_ALMOST_FULL = 2 FIFO_ALMOST_EMPTY = 1 +@cocotb.test() async def test_separate_read_write(dut): - # Run a test where we incrementally write and read values from 1 to FIFO_DEPTH values - for i in range(1, FIFO_DEPTH): + """Run a test where we incrementally write and read values from 1 to + FIFO_DEPTH values.""" + + dut.inp.value = 0 + dut.rdEn.value = 0 + dut.wrEn.value = 0 + await initDut(dut) + for i in range(1, FIFO_DEPTH + 1): for j in range(i): await write(dut, 42 + j) @@ -78,9 +86,16 @@ async def test_separate_read_write(dut): assert dut.empty.value == 1 +@cocotb.test() async def test_concurrent_read_write(dut): - # Fill up the FIFO halfway and concurrently read and write. Should be able - # to do this continuously. + """Fill up the FIFO halfway and concurrently read and write. Should be able + to do this continuously.""" + + dut.inp.value = 0 + dut.rdEn.value = 0 + dut.wrEn.value = 0 + await initDut(dut) + counter = 0 for i in range(FIFO_DEPTH // 2): await write(dut, counter) @@ -88,19 +103,8 @@ async def test_concurrent_read_write(dut): for i in range(FIFO_DEPTH * 2): expected_value = counter - FIFO_DEPTH // 2 - print("expected_value: ", expected_value) + print(f"expected_value: {expected_value}") assert await readWrite(dut, counter) == expected_value assert dut.full.value == 0 assert dut.empty.value == 0 counter += 1 - - -@cocotb.test() -async def test1(dut): - dut.inp.value = 0 - dut.rdEn.value = 0 - dut.wrEn.value = 0 - await initDut(dut) - - await test_separate_read_write(dut) - await test_concurrent_read_write(dut) diff --git a/lib/Dialect/Seq/Transforms/CMakeLists.txt b/lib/Dialect/Seq/Transforms/CMakeLists.txt index 63f574d5b57b..9a8f0ae83127 100644 --- a/lib/Dialect/Seq/Transforms/CMakeLists.txt +++ b/lib/Dialect/Seq/Transforms/CMakeLists.txt @@ -15,6 +15,7 @@ add_circt_dialect_library(CIRCTSeqTransforms CIRCTSeq CIRCTSupport CIRCTSV + CIRCTVerif MLIRIR MLIRPass MLIRTransformUtils diff --git a/lib/Dialect/Seq/Transforms/LowerSeqFIFO.cpp b/lib/Dialect/Seq/Transforms/LowerSeqFIFO.cpp index a0deefb8aa3c..9141086cd619 100644 --- a/lib/Dialect/Seq/Transforms/LowerSeqFIFO.cpp +++ b/lib/Dialect/Seq/Transforms/LowerSeqFIFO.cpp @@ -10,6 +10,7 @@ #include "circt/Dialect/SV/SVOps.h" #include "circt/Dialect/Seq/SeqOps.h" #include "circt/Dialect/Seq/SeqPasses.h" +#include "circt/Dialect/Verif/VerifOps.h" #include "circt/Support/BackedgeBuilder.h" #include "mlir/Pass/Pass.h" #include "mlir/Transforms/DialectConversion.h" @@ -34,10 +35,10 @@ struct FIFOLowering : public OpConversionPattern { LogicalResult matchAndRewrite(seq::FIFOOp mem, OpAdaptor adaptor, ConversionPatternRewriter &rewriter) const final { + Location loc = mem.getLoc(); Type eltType = adaptor.getInput().getType(); Value clk = adaptor.getClk(); Value rst = adaptor.getRst(); - Location loc = mem.getLoc(); BackedgeBuilder bb(rewriter, loc); size_t depth = mem.getDepth(); Type countType = rewriter.getIntegerType(llvm::Log2_64_Ceil(depth + 1)); @@ -47,11 +48,12 @@ struct FIFOLowering : public OpConversionPattern { Backedge nextCount = bb.get(countType); // ====== Some constants ====== - Value countTcFull = - rewriter.create(loc, countType, depth - 1); + Value countTcFull = rewriter.create(loc, countType, depth); Value countTc1 = rewriter.create(loc, countType, 1); Value countTc0 = rewriter.create(loc, countType, 0); + Value ptrTc0 = rewriter.create(loc, ptrType, 0); Value ptrTc1 = rewriter.create(loc, ptrType, 1); + Value ptrTcFull = rewriter.create(loc, ptrType, depth); // ====== Hardware units ====== Value count = rewriter.create( @@ -123,8 +125,13 @@ struct FIFOLowering : public OpConversionPattern { Value wrAndNotFull = rewriter.create( loc, adaptor.getWrEn(), comb::createOrFoldNot(loc, fifoFull, rewriter)); auto addWrAddrPtrTc1 = rewriter.create(loc, wrAddr, ptrTc1); - wrAddrNext.setValue(rewriter.create(loc, wrAndNotFull, - addWrAddrPtrTc1, wrAddr)); + + auto wrAddrNextNoRollover = rewriter.create( + loc, wrAndNotFull, addWrAddrPtrTc1, wrAddr); + auto isMaxAddrWr = rewriter.create( + loc, comb::ICmpPredicate::eq, wrAddrNextNoRollover, ptrTcFull); + wrAddrNext.setValue(rewriter.create(loc, isMaxAddrWr, ptrTc0, + wrAddrNextNoRollover)); static_cast(wrAddrNext) .getDefiningOp() ->setAttr("sv.namehint", rewriter.getStringAttr("fifo_wr_addr_next")); @@ -133,8 +140,12 @@ struct FIFOLowering : public OpConversionPattern { Value rdAndNotEmpty = rewriter.create(loc, adaptor.getRdEn(), notFifoEmpty); auto addRdAddrPtrTc1 = rewriter.create(loc, rdAddr, ptrTc1); - rdAddrNext.setValue(rewriter.create(loc, rdAndNotEmpty, - addRdAddrPtrTc1, rdAddr)); + auto rdAddrNextNoRollover = rewriter.create( + loc, rdAndNotEmpty, addRdAddrPtrTc1, rdAddr); + auto isMaxAddrRd = rewriter.create( + loc, comb::ICmpPredicate::eq, rdAddrNextNoRollover, ptrTcFull); + rdAddrNext.setValue(rewriter.create(loc, isMaxAddrRd, ptrTc0, + rdAddrNextNoRollover)); static_cast(rdAddrNext) .getDefiningOp() ->setAttr("sv.namehint", rewriter.getStringAttr("fifo_rd_addr_next")); @@ -168,6 +179,22 @@ struct FIFOLowering : public OpConversionPattern { ->setAttr("sv.namehint", rewriter.getStringAttr("fifo_almost_empty")); } + // ====== Protocol checks ===== + Value clkI1 = rewriter.create(loc, clk); + Value notEmptyAndRden = comb::createOrFoldNot( + loc, rewriter.create(loc, adaptor.getRdEn(), fifoEmpty), + rewriter); + rewriter.create( + loc, notEmptyAndRden, verif::ClockEdge::Pos, clkI1, /*enable=*/Value(), + rewriter.getStringAttr("FIFO empty when read enabled")); + Value notFullAndWren = comb::createOrFoldNot( + loc, rewriter.create(loc, adaptor.getWrEn(), fifoFull), + rewriter); + rewriter.create( + loc, notFullAndWren, verif::ClockEdge::Pos, clkI1, + /*enable=*/Value(), + rewriter.getStringAttr("FIFO full when write enabled")); + rewriter.replaceOp(mem, results); return success(); } @@ -186,7 +213,8 @@ void LowerSeqFIFOPass::runOnOperation() { // Lowering patterns must lower away all HLMem-related operations. target.addIllegalOp(); - target.addLegalDialect(); + target.addLegalDialect(); RewritePatternSet patterns(&ctxt); patterns.add(&ctxt); diff --git a/test/Dialect/Seq/lower-fifo.mlir b/test/Dialect/Seq/lower-fifo.mlir index 8d3fdebca2d9..1b3c695e9ea8 100644 --- a/test/Dialect/Seq/lower-fifo.mlir +++ b/test/Dialect/Seq/lower-fifo.mlir @@ -4,43 +4,21 @@ // RUN: circt-opt --lower-seq-fifo --canonicalize %s | FileCheck %s --implicit-check-not=seq.fifo -// CHECK: hw.module @fifo1(in %[[CLOCK:.*]] : !seq.clock, in %[[VAL_1:.*]] : i1, in %[[VAL_2:.*]] : i32, in %[[VAL_3:.*]] : i1, in %[[VAL_4:.*]] : i1, out out : i32) { -// CHECK: %[[VAL_6:.*]] = hw.constant -1 : i2 -// CHECK: %[[VAL_5:.*]] = hw.constant true -// CHECK: %[[VAL_7:.*]] = hw.constant -2 : i2 -// CHECK: %[[VAL_8:.*]] = hw.constant 1 : i2 -// CHECK: %[[VAL_9:.*]] = hw.constant 0 : i2 -// CHECK: %[[VAL_10:.*]] = seq.compreg sym @fifo_count %[[VAL_11:.*]], %[[CLOCK]] reset %[[VAL_1]], %[[VAL_9]] : i2 -// CHECK: %[[VAL_12:.*]] = seq.hlmem @fifo_mem %[[CLOCK]], %[[VAL_1]] : <3xi32> -// CHECK: %[[VAL_13:.*]] = seq.compreg sym @fifo_rd_addr %[[VAL_14:.*]], %[[CLOCK]] reset %[[VAL_1]], %[[VAL_9]] : i2 -// CHECK: %[[VAL_15:.*]] = seq.compreg sym @fifo_wr_addr %[[VAL_16:.*]], %[[CLOCK]] reset %[[VAL_1]], %[[VAL_9]] : i2 -// CHECK: %[[VAL_17:.*]] = seq.read %[[VAL_12]]{{\[}}%[[VAL_13]]] rden %[[VAL_3]] {latency = 0 : i64} : !seq.hlmem<3xi32> -// CHECK: seq.write %[[VAL_12]]{{\[}}%[[VAL_15]]] %[[VAL_2]] wren %[[VAL_4]] {latency = 1 : i64} : !seq.hlmem<3xi32> -// CHECK: %[[VAL_18:.*]] = comb.xor %[[VAL_3]], %[[VAL_5]] : i1 -// CHECK: %[[VAL_19:.*]] = comb.xor %[[VAL_4]], %[[VAL_5]] : i1 -// CHECK: %[[VAL_20:.*]] = comb.and %[[VAL_3]], %[[VAL_19]] : i1 -// CHECK: %[[VAL_21:.*]] = comb.and %[[VAL_4]], %[[VAL_18]] : i1 -// CHECK: %[[VAL_22:.*]] = comb.icmp eq %[[VAL_10]], %[[VAL_7]] : i2 -// CHECK: %[[VAL_23:.*]] = comb.add %[[VAL_10]], %[[VAL_8]] : i2 -// CHECK: %[[VAL_24:.*]] = comb.mux %[[VAL_22]], %[[VAL_10]], %[[VAL_23]] : i2 -// CHECK: %[[VAL_25:.*]] = comb.icmp eq %[[VAL_10]], %[[VAL_9]] : i2 -// CHECK: %[[VAL_26:.*]] = comb.add %[[VAL_10]], %[[VAL_6]] : i2 -// CHECK: %[[VAL_27:.*]] = comb.xor %[[VAL_20]], %[[VAL_5]] : i1 -// CHECK: %[[VAL_28:.*]] = comb.or %[[VAL_27]], %[[VAL_25]] : i1 -// CHECK: %[[VAL_29:.*]] = comb.mux %[[VAL_28]], %[[VAL_10]], %[[VAL_26]] : i2 -// CHECK: %[[VAL_30:.*]] = comb.mux %[[VAL_21]], %[[VAL_24]], %[[VAL_29]] : i2 -// CHECK: %[[VAL_31:.*]] = comb.or %[[VAL_3]], %[[VAL_4]] : i1 -// CHECK: %[[VAL_11]] = comb.mux %[[VAL_31]], %[[VAL_30]], %[[VAL_10]] {sv.namehint = "fifo_count_next"} : i2 -// CHECK: %[[VAL_32:.*]] = comb.icmp ne %[[VAL_10]], %[[VAL_7]] : i2 -// CHECK: %[[VAL_33:.*]] = comb.and %[[VAL_4]], %[[VAL_32]] : i1 -// CHECK: %[[VAL_34:.*]] = comb.add %[[VAL_15]], %[[VAL_8]] : i2 -// CHECK: %[[VAL_16]] = comb.mux %[[VAL_33]], %[[VAL_34]], %[[VAL_15]] {sv.namehint = "fifo_wr_addr_next"} : i2 -// CHECK: %[[VAL_35:.*]] = comb.icmp ne %[[VAL_10]], %[[VAL_9]] : i2 -// CHECK: %[[VAL_36:.*]] = comb.and %[[VAL_3]], %[[VAL_35]] : i1 -// CHECK: %[[VAL_37:.*]] = comb.add %[[VAL_13]], %[[VAL_8]] : i2 -// CHECK: %[[VAL_14]] = comb.mux %[[VAL_36]], %[[VAL_37]], %[[VAL_13]] {sv.namehint = "fifo_rd_addr_next"} : i2 -// CHECK: hw.output %[[VAL_17]] : i32 -// CHECK: } +// CHECK: hw.module @fifo1(in %[[CLOCK:.*]] : !seq.clock, in %[[VAL_1:.*]] : i1, in %[[VAL_2:.*]] : i32, in %[[VAL_3:.*]] : i1, in %[[VAL_4:.*]] : i1, out out : i32) { +// CHECK: %fifo_count = seq.compreg sym @fifo_count %{{.+}}, %clk reset %rst, %c0_i2 : i2 +// CHECK: %fifo_mem = seq.hlmem @fifo_mem %clk, %rst : <3xi32> +// CHECK: %fifo_rd_addr = seq.compreg sym @fifo_rd_addr %{{.+}}, %clk reset %rst, %c0_i2 : i2 +// CHECK: %fifo_wr_addr = seq.compreg sym @fifo_wr_addr %{{.+}}, %clk reset %rst, %c0_i2 : i2 +// CHECK: %fifo_mem_rdata = seq.read %fifo_mem[%fifo_rd_addr] rden %rdEn {latency = 0 : i64} : !seq.hlmem<3xi32> +// CHECK: seq.write %fifo_mem[%fifo_wr_addr] %in wren %wrEn {latency = 1 : i64} : !seq.hlmem<3xi32> +// All of the comb logic which would be here is pretty much impossible +// to verify visually. As a rule, if it's difficult to verify +// visually, that's a candidate for a new op: #8002. Since I haven't +// verified it visually, I'm not including it here. There are +// integration tests for it. +// CHECK: verif.clocked_assert %{{.+}}, posedge %{{.+}} label "FIFO empty when read enabled" : i1 +// CHECK: verif.clocked_assert %{{.+}}, posedge %{{.+}} label "FIFO full when write enabled" : i1 +// CHECK: hw.output %fifo_mem_rdata : i32 hw.module @fifo1(in %clk : !seq.clock, in %rst : i1, in %in : i32, in %rdEn : i1, in %wrEn : i1, out out : i32) { %out, %full, %empty = seq.fifo depth 3 in %in rdEn %rdEn wrEn %wrEn clk %clk rst %rst : i32 hw.output %out : i32 @@ -48,50 +26,15 @@ hw.module @fifo1(in %clk : !seq.clock, in %rst : i1, in %in : i32, in %rdEn : i1 // CHECK: hw.module @fifo2(in %[[CLOCK:.*]] : !seq.clock, in %[[VAL_1:.*]] : i1, in %[[VAL_2:.*]] : [[TY:.+]], in %[[VAL_3:.*]] : i1, in %[[VAL_4:.*]] : i1, out out : [[TY]], out empty : i1, out full : i1, out almost_empty : i1, out almost_full : i1) { -// CHECK: %[[VAL_5:.*]] = hw.constant 2 : i3 -// CHECK: %[[VAL_8:.*]] = hw.constant -1 : i3 -// CHECK: %[[VAL_7:.*]] = hw.constant true -// CHECK: %[[VAL_6:.*]] = hw.constant 0 : i2 -// CHECK: %[[VAL_9:.*]] = hw.constant 3 : i3 -// CHECK: %[[VAL_10:.*]] = hw.constant 1 : i3 -// CHECK: %[[VAL_11:.*]] = hw.constant 0 : i3 -// CHECK: %[[VAL_12:.*]] = hw.constant 1 : i2 -// CHECK: %[[VAL_13:.*]] = seq.compreg sym @fifo_count %[[VAL_14:.*]], %[[CLOCK]] reset %[[VAL_1]], %[[VAL_11]] : i3 -// CHECK: %[[VAL_15:.*]] = seq.hlmem @fifo_mem %[[CLOCK]], %[[VAL_1]] : <4x[[TY]]> -// CHECK: %[[VAL_16:.*]] = seq.compreg sym @fifo_rd_addr %[[VAL_17:.*]], %[[CLOCK]] reset %[[VAL_1]], %[[VAL_6]] : i2 -// CHECK: %[[VAL_18:.*]] = seq.compreg sym @fifo_wr_addr %[[VAL_19:.*]], %[[CLOCK]] reset %[[VAL_1]], %[[VAL_6]] : i2 -// CHECK: %[[VAL_20:.*]] = seq.read %[[VAL_15]]{{\[}}%[[VAL_16]]] rden %[[VAL_3]] {latency = 1 : i64} : !seq.hlmem<4x[[TY]]> -// CHECK: seq.write %[[VAL_15]]{{\[}}%[[VAL_18]]] %[[VAL_2]] wren %[[VAL_4]] {latency = 1 : i64} : !seq.hlmem<4x[[TY]]> -// CHECK: %[[VAL_21:.*]] = comb.icmp eq %[[VAL_13]], %[[VAL_9]] {sv.namehint = "fifo_full"} : i3 -// CHECK: %[[VAL_22:.*]] = comb.icmp eq %[[VAL_13]], %[[VAL_11]] {sv.namehint = "fifo_empty"} : i3 -// CHECK: %[[VAL_23:.*]] = comb.xor %[[VAL_3]], %[[VAL_7]] : i1 -// CHECK: %[[VAL_24:.*]] = comb.xor %[[VAL_4]], %[[VAL_7]] : i1 -// CHECK: %[[VAL_25:.*]] = comb.and %[[VAL_3]], %[[VAL_24]] : i1 -// CHECK: %[[VAL_26:.*]] = comb.and %[[VAL_4]], %[[VAL_23]] : i1 -// CHECK: %[[VAL_27:.*]] = comb.icmp eq %[[VAL_13]], %[[VAL_9]] : i3 -// CHECK: %[[VAL_28:.*]] = comb.add %[[VAL_13]], %[[VAL_10]] : i3 -// CHECK: %[[VAL_29:.*]] = comb.mux %[[VAL_27]], %[[VAL_13]], %[[VAL_28]] : i3 -// CHECK: %[[VAL_30:.*]] = comb.icmp eq %[[VAL_13]], %[[VAL_11]] : i3 -// CHECK: %[[VAL_31:.*]] = comb.add %[[VAL_13]], %[[VAL_8]] : i3 -// CHECK: %[[VAL_32:.*]] = comb.xor %[[VAL_25]], %[[VAL_7]] : i1 -// CHECK: %[[VAL_33:.*]] = comb.or %[[VAL_32]], %[[VAL_30]] : i1 -// CHECK: %[[VAL_34:.*]] = comb.mux %[[VAL_33]], %[[VAL_13]], %[[VAL_31]] : i3 -// CHECK: %[[VAL_35:.*]] = comb.mux %[[VAL_26]], %[[VAL_29]], %[[VAL_34]] : i3 -// CHECK: %[[VAL_36:.*]] = comb.or %[[VAL_3]], %[[VAL_4]] : i1 -// CHECK: %[[VAL_14]] = comb.mux %[[VAL_36]], %[[VAL_35]], %[[VAL_13]] {sv.namehint = "fifo_count_next"} : i3 -// CHECK: %[[VAL_37:.*]] = comb.xor %[[VAL_21]], %[[VAL_7]] : i1 -// CHECK: %[[VAL_38:.*]] = comb.and %[[VAL_4]], %[[VAL_37]] : i1 -// CHECK: %[[VAL_39:.*]] = comb.add %[[VAL_18]], %[[VAL_12]] : i2 -// CHECK: %[[VAL_19]] = comb.mux %[[VAL_38]], %[[VAL_39]], %[[VAL_18]] {sv.namehint = "fifo_wr_addr_next"} : i2 -// CHECK: %[[VAL_40:.*]] = comb.xor %[[VAL_22]], %[[VAL_7]] : i1 -// CHECK: %[[VAL_41:.*]] = comb.and %[[VAL_3]], %[[VAL_40]] : i1 -// CHECK: %[[VAL_42:.*]] = comb.add %[[VAL_16]], %[[VAL_12]] : i2 -// CHECK: %[[VAL_17]] = comb.mux %[[VAL_41]], %[[VAL_42]], %[[VAL_16]] {sv.namehint = "fifo_rd_addr_next"} : i2 -// CHECK: %[[VAL_43:.*]] = comb.extract %[[VAL_13]] from 1 : (i3) -> i2 -// CHECK: %[[VAL_44:.*]] = comb.icmp ne %[[VAL_43]], %[[VAL_6]] {sv.namehint = "fifo_almost_full"} : i2 -// CHECK: %[[VAL_45:.*]] = comb.icmp ult %[[VAL_13]], %[[VAL_5]] {sv.namehint = "fifo_almost_empty"} : i3 - // CHECK: hw.output %[[VAL_20]], %[[VAL_22]], %[[VAL_21]], %[[VAL_45]], %[[VAL_44]] : [[TY]], i1, i1, i1, i1 -// CHECK: } +// CHECK: %fifo_count = seq.compreg sym @fifo_count %16, %clk reset %rst, %c0_i3 : i3 +// CHECK: %fifo_mem = seq.hlmem @fifo_mem %clk, %rst : <4x!hw.array<2xi32>> +// CHECK: %fifo_rd_addr = seq.compreg sym @fifo_rd_addr %28, %clk reset %rst, %c0_i2 : i2 +// CHECK: %fifo_wr_addr = seq.compreg sym @fifo_wr_addr %22, %clk reset %rst, %c0_i2 : i2 +// CHECK: %fifo_mem_rdata = seq.read %fifo_mem[%fifo_rd_addr] rden %rdEn {latency = 1 : i64} : !seq.hlmem<4x!hw.array<2xi32>> +// CHECK: seq.write %fifo_mem[%fifo_wr_addr] %in wren %wrEn {latency = 1 : i64} : !seq.hlmem<4x!hw.array<2xi32>> +// See comment above. +// CHECK: verif.clocked_assert %{{.+}}, posedge %{{.+}} label "FIFO empty when read enabled" : i1 +// CHECK: verif.clocked_assert %{{.+}}, posedge %{{.+}} label "FIFO full when write enabled" : i1 !testType = !hw.array<2xi32> hw.module @fifo2(in %clk : !seq.clock, in %rst : i1, in %in : !testType, in %rdEn : i1, in %wrEn : i1, out out: !testType, out empty: i1, out full: i1, out almost_empty : i1, out almost_full : i1) { %out, %full, %empty, %almostFull, %almostEmpty = seq.fifo depth 4 rd_latency 1 almost_full 2 almost_empty 1 in %in rdEn %rdEn wrEn %wrEn clk %clk rst %rst : !testType From 6cabe3cc22c35a01af236deb1b4689ac30d73dde Mon Sep 17 00:00:00 2001 From: John Demme Date: Wed, 18 Dec 2024 13:18:29 -0500 Subject: [PATCH 07/27] [ESI] FIFO with ESI channels (#8004) Adds an op which provides a FIFO with ESI channels (FIFO signaling semantics only for now) on both ends. --- include/circt/Dialect/ESI/ESIChannels.td | 21 +++ .../ESI/{runtime => }/lit.local.cfg.py | 0 .../Dialect/ESI/widgets/esi_widgets.mlir | 13 ++ .../Dialect/ESI/widgets/esi_widgets.py | 151 ++++++++++++++++++ lib/Dialect/ESI/ESIOps.cpp | 16 ++ lib/Dialect/ESI/Passes/ESILowerPhysical.cpp | 67 +++++++- test/Dialect/ESI/errors.mlir | 8 + test/Dialect/ESI/lowering.mlir | 14 ++ 8 files changed, 286 insertions(+), 4 deletions(-) rename integration_test/Dialect/ESI/{runtime => }/lit.local.cfg.py (100%) create mode 100644 integration_test/Dialect/ESI/widgets/esi_widgets.mlir create mode 100644 integration_test/Dialect/ESI/widgets/esi_widgets.py diff --git a/include/circt/Dialect/ESI/ESIChannels.td b/include/circt/Dialect/ESI/ESIChannels.td index 8c9d41e71f56..3bef93462b2d 100644 --- a/include/circt/Dialect/ESI/ESIChannels.td +++ b/include/circt/Dialect/ESI/ESIChannels.td @@ -476,6 +476,27 @@ def PipelineStageOp : ESI_Physical_Op<"stage", [ // Misc operations //===----------------------------------------------------------------------===// +def FIFOOp : ESI_Physical_Op<"fifo", []> { + let summary = "A FIFO with ESI channel connections"; + let description = [{ + A FIFO is a first-in-first-out buffer. This operation is a simple FIFO + which can be used to connect two ESI channels. The ESI channels MUST have + FIFO signaling semantics. + }]; + + let arguments = (ins + ClockType:$clk, I1:$rst, ChannelType:$input, + ConfinedAttr]>:$depth + ); + let results = (outs ChannelType:$output); + + let assemblyFormat = [{ + `in` $input `clk` $clk `rst` $rst `depth` $depth attr-dict + `:` type($input) `->` type($output) + }]; + let hasVerifier = 1; +} + def CosimToHostEndpointOp : ESI_Physical_Op<"cosim.to_host", []> { let summary = "Co-simulation endpoint sending data to the host."; let description = [{ diff --git a/integration_test/Dialect/ESI/runtime/lit.local.cfg.py b/integration_test/Dialect/ESI/lit.local.cfg.py similarity index 100% rename from integration_test/Dialect/ESI/runtime/lit.local.cfg.py rename to integration_test/Dialect/ESI/lit.local.cfg.py diff --git a/integration_test/Dialect/ESI/widgets/esi_widgets.mlir b/integration_test/Dialect/ESI/widgets/esi_widgets.mlir new file mode 100644 index 000000000000..2a87add26544 --- /dev/null +++ b/integration_test/Dialect/ESI/widgets/esi_widgets.mlir @@ -0,0 +1,13 @@ +// REQUIRES: iverilog,cocotb + +// Test the original HandshakeToHW flow. + +// RUN: circt-opt %s --lower-esi-to-physical --lower-esi-ports --lower-esi-to-hw --lower-seq-fifo --lower-seq-hlmem --lower-seq-to-sv --lower-verif-to-sv --sv-trace-iverilog --prettify-verilog --export-verilog -o %t.hw.mlir > %t.sv +// RUN: circt-cocotb-driver.py --objdir=%T --topLevel=fifo1 --pythonModule=esi_widgets --pythonFolder="%S" %t.sv %esi_prims + +module attributes {circt.loweringOptions = "disallowLocalVariables"} { + hw.module @fifo1(in %clk: !seq.clock, in %rst: i1, in %in: !esi.channel, out out: !esi.channel) { + %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel -> !esi.channel + hw.output %fifo : !esi.channel + } +} diff --git a/integration_test/Dialect/ESI/widgets/esi_widgets.py b/integration_test/Dialect/ESI/widgets/esi_widgets.py new file mode 100644 index 000000000000..e84fb76d1d14 --- /dev/null +++ b/integration_test/Dialect/ESI/widgets/esi_widgets.py @@ -0,0 +1,151 @@ +from typing import List, Optional +import cocotb +from cocotb.clock import Clock +from cocotb.triggers import RisingEdge + + +async def init(dut, timeout: Optional[int] = None): + # Create a 10ns period (100MHz) clock on port clock + clk = Clock(dut.clk, 10, units="ns") + cocotb.start_soon(clk.start()) # Start the clock + + # Reset + dut.rst.value = 1 + for i in range(10): + await RisingEdge(dut.clk) + dut.rst.value = 0 + await RisingEdge(dut.clk) + + if timeout is None: + return + + async def timeout_fn(): + for i in range(timeout): + await RisingEdge(dut.clk) + assert False, "Timeout" + + cocotb.start_soon(timeout_fn()) + + +class ESIInputPort: + + async def write(self, data): + raise RuntimeError("Must be implemented by subclass") + + +class ESIFifoInputPort(ESIInputPort): + + def __init__(self, dut, name): + self.dut = dut + self.name = name + self.data = getattr(dut, name) + self.rden = getattr(dut, f"{name}_rden") + self.empty = getattr(dut, f"{name}_empty") + # Configure initial state + self.empty.value = 1 + + async def write(self, data: int): + self.data.value = data + self.empty.value = 0 + await RisingEdge(self.dut.clk) + while self.rden.value == 0: + await RisingEdge(self.dut.clk) + self.empty.value = 1 + + +class ESIOutputPort: + + async def read(self) -> Optional[int]: + raise RuntimeError("Must be implemented by subclass") + + async def cmd_read(self): + raise RuntimeError("Must be implemented by subclass") + + +class ESIFifoOutputPort(ESIOutputPort): + + def __init__(self, dut, name: str, latency: int = 0): + self.dut = dut + self.name = name + self.data = getattr(dut, name) + self.rden = getattr(dut, f"{name}_rden") + self.empty = getattr(dut, f"{name}_empty") + self.latency = latency + # Configure initial state + self.rden.value = 0 + self.running = 0 + self.q: List[int] = [] + + async def init_read(self): + + async def read_after_latency(): + for i in range(self.latency): + await RisingEdge(self.dut.clk) + self.q.append(self.data.value) + + self.running = 1 + self.empty.value = 0 + while await RisingEdge(self.dut.clk): + if self.rden.value == 1: + cocotb.start_soon(read_after_latency()) + + async def read(self) -> Optional[int]: + if len(self.q) == 0: + await RisingEdge(self.dut.clk) + return self.q.pop(0) + + async def cmd_read(self): + if self.running == 0: + cocotb.start_soon(self.init_read()) + + while self.empty.value == 1: + await RisingEdge(self.dut.clk) + self.rden.value = 1 + await RisingEdge(self.dut.clk) + self.rden.value = 0 + + +@cocotb.test() +async def fillAndDrain(dut): + in_port = ESIFifoInputPort(dut, "in") + out_port = ESIFifoOutputPort(dut, "out", 2) + await init(dut, timeout=10000) + + for i in range(10): + for i in range(12): + await in_port.write(i) + for i in range(12): + await out_port.cmd_read() + for i in range(12): + data = await out_port.read() + # print(f"{i:2}: 0x{int(data):016x}") + assert data == i + + for i in range(4): + await RisingEdge(dut.clk) + + +@cocotb.test() +async def backToBack(dut): + in_port = ESIFifoInputPort(dut, "in") + out_port = ESIFifoOutputPort(dut, "out", 2) + await init(dut) + + NUM_ITERS = 500 + + async def write(): + for i in range(NUM_ITERS): + await in_port.write(i) + + cocotb.start_soon(write()) + + for i in range(NUM_ITERS): + await out_port.cmd_read() + + for i in range(NUM_ITERS): + data = await out_port.read() + # print(f"{i:2}: 0x{int(data):016x}") + assert data == i + + for i in range(4): + await RisingEdge(dut.clk) diff --git a/lib/Dialect/ESI/ESIOps.cpp b/lib/Dialect/ESI/ESIOps.cpp index 80d43662e696..4c94935b14eb 100644 --- a/lib/Dialect/ESI/ESIOps.cpp +++ b/lib/Dialect/ESI/ESIOps.cpp @@ -72,6 +72,22 @@ LogicalResult ChannelBufferOp::verify() { return success(); } +//===----------------------------------------------------------------------===// +// FIFO functions. +//===----------------------------------------------------------------------===// + +LogicalResult FIFOOp::verify() { + ChannelType inputType = getInput().getType(); + if (inputType.getSignaling() != ChannelSignaling::FIFO) + return emitOpError("only supports FIFO signaling on input"); + ChannelType outputType = getOutput().getType(); + if (outputType.getSignaling() != ChannelSignaling::FIFO) + return emitOpError("only supports FIFO signaling on output"); + if (outputType.getInner() != inputType.getInner()) + return emitOpError("input and output types must match"); + return success(); +} + //===----------------------------------------------------------------------===// // PipelineStageOp functions. //===----------------------------------------------------------------------===// diff --git a/lib/Dialect/ESI/Passes/ESILowerPhysical.cpp b/lib/Dialect/ESI/Passes/ESILowerPhysical.cpp index 5185c9bb81d4..172ddb82b277 100644 --- a/lib/Dialect/ESI/Passes/ESILowerPhysical.cpp +++ b/lib/Dialect/ESI/Passes/ESILowerPhysical.cpp @@ -12,8 +12,10 @@ #include "../PassDetails.h" +#include "circt/Dialect/Comb/CombOps.h" #include "circt/Dialect/ESI/ESIOps.h" #include "circt/Dialect/HW/HWOps.h" +#include "circt/Dialect/Seq/SeqOps.h" #include "circt/Support/BackedgeBuilder.h" #include "circt/Support/LLVM.h" @@ -78,6 +80,64 @@ LogicalResult ChannelBufferLowering::matchAndRewrite( return success(); } +namespace { +/// Lower `ChannelBufferOp`s, breaking out the various options. For now, just +/// replace with the specified number of pipeline stages (since that's the only +/// option). +struct FIFOLowering : public OpConversionPattern { +public: + using OpConversionPattern::OpConversionPattern; + + LogicalResult + matchAndRewrite(FIFOOp, OpAdaptor adaptor, + ConversionPatternRewriter &rewriter) const final; +}; +} // anonymous namespace + +LogicalResult +FIFOLowering::matchAndRewrite(FIFOOp op, OpAdaptor adaptor, + ConversionPatternRewriter &rewriter) const { + auto loc = op.getLoc(); + auto outputType = op.getType(); + BackedgeBuilder bb(rewriter, loc); + auto i1 = rewriter.getI1Type(); + auto c1 = rewriter.create(loc, rewriter.getI1Type(), + rewriter.getBoolAttr(true)); + if (op.getInput().getType().getDataDelay() != 0) + return op.emitOpError( + "currently only supports input channels with zero data delay"); + + Backedge inputEn = bb.get(i1); + auto unwrapPull = rewriter.create(loc, op.getInput(), inputEn); + + Backedge outputRdEn = bb.get(i1); + auto seqFifo = rewriter.create( + loc, outputType.getInner(), i1, i1, Type(), Type(), unwrapPull.getData(), + outputRdEn, inputEn, op.getClk(), op.getRst(), op.getDepthAttr(), + rewriter.getI64IntegerAttr(outputType.getDataDelay()), IntegerAttr(), + IntegerAttr()); + auto inputNotEmpty = + rewriter.create(loc, unwrapPull.getEmpty(), c1); + inputNotEmpty->setAttr("sv.namehint", + rewriter.getStringAttr("inputNotEmpty")); + auto seqFifoNotFull = + rewriter.create(loc, seqFifo.getFull(), c1); + seqFifoNotFull->setAttr("sv.namehint", + rewriter.getStringAttr("seqFifoNotFull")); + inputEn.setValue( + rewriter.create(loc, inputNotEmpty, seqFifoNotFull)); + static_cast(inputEn).getDefiningOp()->setAttr( + "sv.namehint", rewriter.getStringAttr("inputEn")); + + auto output = + rewriter.create(loc, mlir::TypeRange{outputType, i1}, + seqFifo.getOutput(), seqFifo.getEmpty()); + outputRdEn.setValue(output.getRden()); + + rewriter.replaceOp(op, output.getChanOutput()); + return success(); +} + namespace { /// Lower pure modules into hw.modules. struct PureModuleLowering : public OpConversionPattern { @@ -186,13 +246,12 @@ void ESIToPhysicalPass::runOnOperation() { // Set up a conversion and give it a set of laws. ConversionTarget target(getContext()); target.markUnknownOpDynamicallyLegal([](Operation *) { return true; }); - target.addIllegalOp(); - target.addIllegalOp(); + target.addIllegalOp(); // Add all the conversion patterns. RewritePatternSet patterns(&getContext()); - patterns.insert(&getContext()); - patterns.insert(&getContext()); + patterns.insert( + &getContext()); // Run the conversion. if (failed( diff --git a/test/Dialect/ESI/errors.mlir b/test/Dialect/ESI/errors.mlir index 52f9bbff5a54..a98368265f57 100644 --- a/test/Dialect/ESI/errors.mlir +++ b/test/Dialect/ESI/errors.mlir @@ -46,6 +46,14 @@ hw.module @test(in %m : !sv.modport<@IData::@Noexist>) { // ----- +hw.module @testFifoTypes(in %clk: !seq.clock, in %rst: i1, in %a: !esi.channel, in %b: !esi.channel) { + // expected-error @+1 {{input and output types must match}} + %fifo = esi.fifo in %a clk %clk rst %rst depth 12 : !esi.channel -> !esi.channel + hw.output %fifo : !esi.channel +} + +// ----- + esi.service.decl @HostComms { esi.service.port @Send : !esi.bundle<[!esi.channel from "send"]> } diff --git a/test/Dialect/ESI/lowering.mlir b/test/Dialect/ESI/lowering.mlir index 7daed7d729c3..50b018603610 100644 --- a/test/Dialect/ESI/lowering.mlir +++ b/test/Dialect/ESI/lowering.mlir @@ -197,3 +197,17 @@ hw.module @i3LoopbackOddNames(in %in: !esi.channel, out out: !esi.channel } + +// CHECK-LABEL: hw.module @fifo1(in %clk : !seq.clock, in %rst : i1, in %in : !esi.channel, out out : !esi.channel) { +// CHECK-NEXT: %true = hw.constant true +// CHECK-NEXT: %data, %empty = esi.unwrap.fifo %in, [[R2:%.+]] : !esi.channel +// CHECK-NEXT: %out, %full, %empty_0 = seq.fifo depth 12 rd_latency 2 in %data rdEn %rden wrEn [[R2]] clk %clk rst %rst : i32 +// CHECK-NEXT: [[R0:%.+]] = comb.xor %empty, %true {sv.namehint = "inputNotEmpty"} : i1 +// CHECK-NEXT: [[R1:%.+]] = comb.xor %full, %true {sv.namehint = "seqFifoNotFull"} : i1 +// CHECK-NEXT: [[R2]] = comb.and [[R0]], [[R1]] {sv.namehint = "inputEn"} : i1 +// CHECK-NEXT: %chanOutput, %rden = esi.wrap.fifo %out, %empty_0 : !esi.channel +// CHECK-NEXT: hw.output %chanOutput : !esi.channel +hw.module @fifo1(in %clk: !seq.clock, in %rst: i1, in %in: !esi.channel, out out: !esi.channel) { + %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel -> !esi.channel + hw.output %fifo : !esi.channel +} From 00a562525206311c8283c6de8b58164f5116aa07 Mon Sep 17 00:00:00 2001 From: Bea Healy <57840981+TaoBi22@users.noreply.github.com> Date: Wed, 18 Dec 2024 19:41:08 +0000 Subject: [PATCH 08/27] [LowerToBMC] Topologically sort module body before inlining to BMC op (#8007) --- lib/Tools/circt-bmc/LowerToBMC.cpp | 6 +++ test/Tools/circt-bmc/lower-to-bmc-errors.mlir | 12 ++++++ test/Tools/circt-bmc/lower-to-bmc.mlir | 37 +++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/lib/Tools/circt-bmc/LowerToBMC.cpp b/lib/Tools/circt-bmc/LowerToBMC.cpp index 7cc43259082d..ddf5d970d7a5 100644 --- a/lib/Tools/circt-bmc/LowerToBMC.cpp +++ b/lib/Tools/circt-bmc/LowerToBMC.cpp @@ -15,6 +15,7 @@ #include "circt/Support/LLVM.h" #include "circt/Support/Namespace.h" #include "circt/Tools/circt-bmc/Passes.h" +#include "mlir/Analysis/TopologicalSortUtils.h" #include "mlir/Dialect/Func/IR/FuncOps.h" #include "mlir/Dialect/LLVMIR/FunctionCallUtils.h" #include "mlir/Dialect/LLVMIR/LLVMDialect.h" @@ -60,6 +61,11 @@ void LowerToBMCPass::runOnOperation() { return signalPassFailure(); } + if (!sortTopologically(&hwModule.getBodyRegion().front())) { + hwModule->emitError("could not resolve cycles in module"); + return signalPassFailure(); + } + // Create necessary function declarations and globals auto *ctx = &getContext(); OpBuilder builder(ctx); diff --git a/test/Tools/circt-bmc/lower-to-bmc-errors.mlir b/test/Tools/circt-bmc/lower-to-bmc-errors.mlir index 86694def20fd..ed08549619eb 100644 --- a/test/Tools/circt-bmc/lower-to-bmc-errors.mlir +++ b/test/Tools/circt-bmc/lower-to-bmc-errors.mlir @@ -79,3 +79,15 @@ hw.module @testModule(in %clk0 : !seq.clock, in %clkStruct : !hw.struct Date: Wed, 18 Dec 2024 18:49:18 -0500 Subject: [PATCH 09/27] [PyCDE] Update build flow (#8008) Fix the PyCDE build flow. Added nanobind and fixed dll install on Windows. --- frontends/PyCDE/pyproject.toml | 3 ++- frontends/PyCDE/setup.py | 2 ++ frontends/PyCDE/src/CMakeLists.txt | 19 +++++++------------ 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/frontends/PyCDE/pyproject.toml b/frontends/PyCDE/pyproject.toml index f4847d12b622..72462cda5ab1 100644 --- a/frontends/PyCDE/pyproject.toml +++ b/frontends/PyCDE/pyproject.toml @@ -7,7 +7,8 @@ requires = [ # MLIR build depends. "numpy", - "pybind11>=2.9", + "pybind11>=2.11,<=2.12", + "nanobind==2.4.0", "PyYAML", # PyCDE depends diff --git a/frontends/PyCDE/setup.py b/frontends/PyCDE/setup.py index 356ad47cd055..697a6f1ba97e 100644 --- a/frontends/PyCDE/setup.py +++ b/frontends/PyCDE/setup.py @@ -56,6 +56,8 @@ def run(self): if "BUILD_TYPE" in os.environ: cfg = os.environ["BUILD_TYPE"] cmake_args = [ + "-Wno-dev", + "-GNinja", "-DCMAKE_INSTALL_PREFIX={}".format(os.path.abspath(cmake_install_dir)), "-DPython3_EXECUTABLE={}".format(sys.executable.replace("\\", "/")), "-DCMAKE_BUILD_TYPE={}".format(cfg), # not used on MSVC, but no harm diff --git a/frontends/PyCDE/src/CMakeLists.txt b/frontends/PyCDE/src/CMakeLists.txt index fad232cb8301..a347ab73e585 100644 --- a/frontends/PyCDE/src/CMakeLists.txt +++ b/frontends/PyCDE/src/CMakeLists.txt @@ -87,6 +87,13 @@ add_mlir_python_modules(PyCDE PyCDE_CIRCTPythonCAPI ) +install(TARGETS PyCDE_CIRCTPythonCAPI + DESTINATION python_packages/pycde/circt/_mlir_libs + RUNTIME_DEPENDENCIES + PRE_EXCLUDE_REGEXES ".*" + PRE_INCLUDE_REGEXES ".*zlib.*" + COMPONENT PyCDE +) add_dependencies(PyCDE PyCDE_CIRCTPythonModules) add_dependencies(install-PyCDE install-PyCDE_CIRCTPythonModules) @@ -103,15 +110,3 @@ install(FILES ${esiprims} DESTINATION python_packages/pycde COMPONENT PyCDE ) - -install(IMPORTED_RUNTIME_ARTIFACTS PyCDE_CIRCTPythonCAPI - RUNTIME_DEPENDENCY_SET PyCDE_RUNTIME_DEPS - DESTINATION python_packages/pycde/circt/_mlir_libs - COMPONENT PyCDE -) -install(RUNTIME_DEPENDENCY_SET PyCDE_RUNTIME_DEPS - DESTINATION python_packages/pycde/circt/_mlir_libs - PRE_EXCLUDE_REGEXES .* - PRE_INCLUDE_REGEXES zlib1 - COMPONENT PyCDE -) From 6118d46081e9bbdc9ccdf5485936ad55942c5177 Mon Sep 17 00:00:00 2001 From: John Demme Date: Thu, 19 Dec 2024 00:36:53 +0000 Subject: [PATCH 10/27] [cocotb] Add option to dump waves and increase timescale - Adds a `--waves` option to tell cocotb to dump a waveform. - Increases the timescale --- tools/circt-cocotb-driver/circt-cocotb-driver.py.in | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/circt-cocotb-driver/circt-cocotb-driver.py.in b/tools/circt-cocotb-driver/circt-cocotb-driver.py.in index 0d6ccfbd3ae5..6fdedaa57344 100755 --- a/tools/circt-cocotb-driver/circt-cocotb-driver.py.in +++ b/tools/circt-cocotb-driver/circt-cocotb-driver.py.in @@ -31,6 +31,10 @@ def parseArgs(args): required=True, help="Name of the python module.") + argparser.add_argument("--waves", + action="store_true", + help="Enable waveform dumping.") + argparser.add_argument( "--pythonFolders", type=str, @@ -82,7 +86,7 @@ class Icarus(csim.Icarus): # lives easier and create a minimum timescale through the command-line. cmd_file = os.path.join(objDir, "cmds.f") with open(cmd_file, "w+") as f: - f.write("+timescale+1ns/1ps\n") + f.write("+timescale+1ns/100ps\n") self.compile_args.extend([ f"-f{cmd_file}", @@ -117,7 +121,7 @@ class Questa(csim.Questa): # lives easier and create a minimum timescale through the command-line. cmd_file = os.path.join(objDir, "cmds.f") with open(cmd_file, "w+") as f: - f.write("+timescale+1ns/1ps") + f.write("+timescale+1ns/100ps") self.gui = gui self.extra_compile_args = ["-f", f"{cmd_file}"] @@ -151,6 +155,7 @@ def main(): "verilog_sources": sources, "python_search": [f.strip() for f in args.pythonFolders.split(",")], "work_dir": objDir, + "waves": args.waves, } try: if args.simulator == "icarus": From 99826b849987b38b8590ad70cc45efd02dce2c35 Mon Sep 17 00:00:00 2001 From: John Demme Date: Thu, 19 Dec 2024 00:57:08 +0000 Subject: [PATCH 11/27] [HWArith] Fix lowering to HW with type aliases Closes #7961 --- lib/Conversion/HWArithToHW/HWArithToHW.cpp | 6 ++++++ test/Conversion/HWArithToHW/test_basic.mlir | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/lib/Conversion/HWArithToHW/HWArithToHW.cpp b/lib/Conversion/HWArithToHW/HWArithToHW.cpp index 2311f0c0f054..4af3a313aa2e 100644 --- a/lib/Conversion/HWArithToHW/HWArithToHW.cpp +++ b/lib/Conversion/HWArithToHW/HWArithToHW.cpp @@ -119,6 +119,8 @@ static bool isSignednessType(Type type) { }) .Case( [](auto type) { return isSignednessType(type.getElementType()); }) + .Case( + [](auto type) { return isSignednessType(type.getInnerType()); }) .Default([](auto type) { return false; }); return match; @@ -378,6 +380,10 @@ Type HWArithToHWTypeConverter::removeSignedness(Type type) { .Case([this](auto type) { return hw::InOutType::get(removeSignedness(type.getElementType())); }) + .Case([this](auto type) { + return hw::TypeAliasType::get( + type.getRef(), removeSignedness(type.getInnerType())); + }) .Default([](auto type) { return type; }); return convertedType; diff --git a/test/Conversion/HWArithToHW/test_basic.mlir b/test/Conversion/HWArithToHW/test_basic.mlir index 11d4c5282ac6..a75986b8d951 100644 --- a/test/Conversion/HWArithToHW/test_basic.mlir +++ b/test/Conversion/HWArithToHW/test_basic.mlir @@ -360,3 +360,24 @@ hw.module @wires () { sv.assign %r52, %c0_ui2 : ui2 sv.assign %r53, %c0_ui2 : ui2 } + +// ----- + +// CHECK: hw.type_scope @pycde { +// CHECK: hw.typedecl @MMIOIntermediateCmd : !hw.struct +// CHECK: } +// CHECK: hw.module @MMIOAxiReadWriteMux(out cmd : !hw.typealias<@pycde::@MMIOIntermediateCmd, !hw.struct>) { +// CHECK: %c0_i32 = hw.constant 0 : i32 +// CHECK: [[R0:%.+]] = hw.struct_create (%c0_i32) : !hw.typealias<@pycde::@MMIOIntermediateCmd, !hw.struct> +// CHECK: hw.output [[R0]] : !hw.typealias<@pycde::@MMIOIntermediateCmd, !hw.struct> +// CHECK: } + +hw.type_scope @pycde { + hw.typedecl @MMIOIntermediateCmd : !hw.struct +} +hw.module @MMIOAxiReadWriteMux(out cmd : !hw.typealias<@pycde::@MMIOIntermediateCmd, !hw.struct>) { + %2 = hw.constant 0 : i32 + %3 = hwarith.cast %2 : (i32) -> ui32 + %4 = hw.struct_create (%3) : !hw.typealias<@pycde::@MMIOIntermediateCmd, !hw.struct> + hw.output %4: !hw.typealias<@pycde::@MMIOIntermediateCmd, !hw.struct> +} From bcc1e01cdba00c985f4913de4c05ea150d262fc9 Mon Sep 17 00:00:00 2001 From: John Demme Date: Thu, 19 Dec 2024 01:21:14 +0000 Subject: [PATCH 12/27] [DC CAPI][PyCDE] Add cmerge and demux handshake ops - Adds cmerge and demux functions to the handshake pycde module. - Lowering them requires fixes to the conversion pass and the CAPI code. --- frontends/PyCDE/src/pycde/handshake.py | 32 ++++++++++++++++++++++---- frontends/PyCDE/src/pycde/system.py | 3 +-- frontends/PyCDE/test/test_handshake.py | 32 ++++++++++++++++---------- include/circt/Conversion/Passes.td | 7 +++++- lib/CAPI/Dialect/DC.cpp | 6 ++++- 5 files changed, 59 insertions(+), 21 deletions(-) diff --git a/frontends/PyCDE/src/pycde/handshake.py b/frontends/PyCDE/src/pycde/handshake.py index 80ca25f6b983..14397890aac6 100644 --- a/frontends/PyCDE/src/pycde/handshake.py +++ b/frontends/PyCDE/src/pycde/handshake.py @@ -3,13 +3,14 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception from __future__ import annotations -from typing import List, Optional, Dict +from typing import List, Optional, Dict, Tuple from .module import Module, ModuleLikeBuilderBase, PortError -from .signals import BitsSignal, ChannelSignal, ClockSignal, Signal +from .signals import (BitsSignal, ChannelSignal, ClockSignal, Signal, + _FromCirctValue) from .system import System -from .support import get_user_loc, obj_to_typed_attribute -from .types import Channel +from .support import clog2, get_user_loc +from .types import Bits, Channel from .circt.dialects import handshake as raw_handshake from .circt import ir @@ -82,7 +83,7 @@ def instantiate(self, module_inst, inputs, instance_name: str): # If the input is a channel signal, the types must match. if signal.type.inner_type != port.type: raise ValueError( - f"Wrong type on input signal '{name}'. Got '{signal.type}'," + f"Wrong type on input signal '{name}'. Got '{signal.type.inner_type}'," f" expected '{port.type}'") assert port.idx is not None circt_inputs[port.idx] = signal.value @@ -124,3 +125,24 @@ class Func(Module): BuilderType: type[ModuleLikeBuilderBase] = FuncBuilder _builder: FuncBuilder + + +def demux(cond: BitsSignal, data: Signal) -> Tuple[Signal, Signal]: + """Demux a signal based on a condition.""" + condbr = raw_handshake.ConditionalBranchOp(cond.value, data.value) + return (_FromCirctValue(condbr.trueResult), + _FromCirctValue(condbr.falseResult)) + + +def cmerge(*args: Signal) -> Tuple[Signal, BitsSignal]: + """Merge multiple signals into one and the index of the signal.""" + if len(args) == 0: + raise ValueError("cmerge must have at least one argument") + first = args[0] + for a in args[1:]: + if a.type != first.type: + raise ValueError("All arguments to cmerge must have the same type") + idx_type = Bits(clog2(len(args))) + cm = raw_handshake.ControlMergeOp(a.type._type, idx_type._type, + [a.value for a in args]) + return (_FromCirctValue(cm.result), BitsSignal(cm.index, idx_type)) diff --git a/frontends/PyCDE/src/pycde/system.py b/frontends/PyCDE/src/pycde/system.py index 5ba2be1fbf34..ca5775fac0fc 100644 --- a/frontends/PyCDE/src/pycde/system.py +++ b/frontends/PyCDE/src/pycde/system.py @@ -264,8 +264,8 @@ def get_instance(self, # Then run all the passes to lower dialects which produce `hw.module`s. "builtin.module(lower-handshake-to-dc)", "builtin.module(dc-materialize-forks-sinks)", - "builtin.module(canonicalize)", "builtin.module(lower-dc-to-hw)", + "builtin.module(map-arith-to-comb)", # Run ESI manifest passes. "builtin.module(esi-appid-hier{{top={tops} }}, esi-build-manifest{{top={tops} }})", @@ -275,7 +275,6 @@ def get_instance(self, # Instaniate hlmems, which could produce new esi connections. "builtin.module(hw.module(lower-seq-hlmem))", "builtin.module(lower-esi-to-physical)", - # TODO: support more than just cosim. "builtin.module(lower-esi-bundles, lower-esi-ports)", "builtin.module(lower-esi-to-hw{{platform={platform}}})", "builtin.module(convert-fsm-to-sv)", diff --git a/frontends/PyCDE/test/test_handshake.py b/frontends/PyCDE/test/test_handshake.py index cf3c3ffe8974..d8fc37ac24d4 100644 --- a/frontends/PyCDE/test/test_handshake.py +++ b/frontends/PyCDE/test/test_handshake.py @@ -1,42 +1,50 @@ # RUN: %PYTHON% %s | FileCheck %s from pycde import (Clock, Output, Input, generator, types, Module) -from pycde.handshake import Func +from pycde.handshake import Func, cmerge, demux from pycde.testing import unittestmodule from pycde.types import Bits, Channel -# CHECK: hw.module @Top(in %clk : !seq.clock, in %rst : i1, in %a : !esi.channel, out x : !esi.channel) -# CHECK: [[R0:%.+]] = handshake.esi_instance @TestFunc "TestFunc" clk %clk rst %rst(%a) : (!esi.channel) -> !esi.channel -# CHECK: hw.output [[R0]] : !esi.channel -# CHECK: } -# CHECK: handshake.func @TestFunc(%arg0: i8, ...) -> i8 +# CHECK: hw.module @Top(in %clk : !seq.clock, in %rst : i1, in %a : !esi.channel, in %b : !esi.channel, out x : !esi.channel) +# CHECK: %0:2 = handshake.esi_instance @TestFunc "TestFunc" clk %clk rst %rst(%a, %b) : (!esi.channel, !esi.channel) -> (!esi.channel, !esi.channel) +# CHECK: hw.output %0#0 : !esi.channel + +# CHECK: handshake.func @TestFunc(%arg0: i8, %arg1: i8, ...) -> (i8, i8) +# CHECK: %result, %index = control_merge %arg0, %arg1 : i8, i1 # CHECK: %c15_i8 = hw.constant 15 : i8 -# CHECK: %0 = comb.and bin %arg0, %c15_i8 : i8 -# CHECK: return %0 : i8 -# CHECK: } +# CHECK: [[R0:%.+]] = comb.and bin %result, %c15_i8 : i8 +# CHECK: %trueResult, %falseResult = cond_br %index, [[R0]] : i8 +# CHECK: return %trueResult, %falseResult : i8, i8 class TestFunc(Func): a = Input(Bits(8)) + b = Input(Bits(8)) x = Output(Bits(8)) + y = Output(Bits(8)) @generator def build(ports): - ports.x = ports.a & Bits(8)(0xF) + c, sel = cmerge(ports.a, ports.b) + z = c & Bits(8)(0xF) + x, y = demux(sel, z) + ports.x = x + ports.y = y BarType = types.struct({"foo": types.i12}, "bar") -@unittestmodule(print=True, run_passes=True) +@unittestmodule(print=True) class Top(Module): clk = Clock() rst = Input(Bits(1)) a = Input(Channel(Bits(8))) + b = Input(Channel(Bits(8))) x = Output(Channel(Bits(8))) @generator def build(ports): - test = TestFunc(clk=ports.clk, rst=ports.rst, a=ports.a) + test = TestFunc(clk=ports.clk, rst=ports.rst, a=ports.a, b=ports.b) ports.x = test.x diff --git a/include/circt/Conversion/Passes.td b/include/circt/Conversion/Passes.td index 0f3dc53a7b64..5f6ab39d4c23 100644 --- a/include/circt/Conversion/Passes.td +++ b/include/circt/Conversion/Passes.td @@ -471,7 +471,12 @@ def HandshakeToDC : Pass<"lower-handshake-to-dc", "mlir::ModuleOp"> { function with graph region behaviour. Thus, for now, we just use `hw.module` as a container operation. }]; - let dependentDialects = ["dc::DCDialect", "mlir::func::FuncDialect", "hw::HWDialect"]; + let dependentDialects = [ + "dc::DCDialect", + "mlir::arith::ArithDialect", + "mlir::func::FuncDialect", + "hw::HWDialect" + ]; let options = [ Option<"clkName", "clk-name", "std::string", "\"clk\"", "Name of the clock signal to use in the generated DC module">, diff --git a/lib/CAPI/Dialect/DC.cpp b/lib/CAPI/Dialect/DC.cpp index 40dd6e04d4a0..a5285e28e4c3 100644 --- a/lib/CAPI/Dialect/DC.cpp +++ b/lib/CAPI/Dialect/DC.cpp @@ -10,9 +10,13 @@ #include "circt/Conversion/Passes.h" #include "circt/Dialect/DC/DCDialect.h" #include "circt/Dialect/DC/DCPasses.h" +#include "circt/Transforms/Passes.h" #include "mlir/CAPI/IR.h" #include "mlir/CAPI/Registration.h" #include "mlir/CAPI/Support.h" -void registerDCPasses() { circt::dc::registerPasses(); } +void registerDCPasses() { + circt::registerMapArithToCombPass(); + circt::dc::registerPasses(); +} MLIR_DEFINE_CAPI_DIALECT_REGISTRATION(DC, dc, circt::dc::DCDialect) From 348e82a5318d8426b2523027390bb86944b9b626 Mon Sep 17 00:00:00 2001 From: John Demme Date: Thu, 19 Dec 2024 01:28:59 +0000 Subject: [PATCH 13/27] [PyCDE] Fix cocotb runner Icarus 3.12 doesn't like unterminated lines in command files. --- frontends/PyCDE/src/pycde/testing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontends/PyCDE/src/pycde/testing.py b/frontends/PyCDE/src/pycde/testing.py index 2dd27935fcfd..19cfe773afc9 100644 --- a/frontends/PyCDE/src/pycde/testing.py +++ b/frontends/PyCDE/src/pycde/testing.py @@ -123,7 +123,7 @@ def extra_compile_args(self, pycde_system: System): # lives easier and create a minimum timescale through the command-line. cmd_file = os.path.join(pycde_system.output_directory, "cmds.f") with open(cmd_file, "w+") as f: - f.write("+timescale+1ns/1ps") + f.write("+timescale+1ns/1ps\n") return [f"-f{cmd_file}"] From c31f2072d7e5e91a72db801c41b2c5e717ef86a4 Mon Sep 17 00:00:00 2001 From: John Demme Date: Thu, 19 Dec 2024 01:38:47 +0000 Subject: [PATCH 14/27] [ESI Runtime][Verilator] Option to slow down the sim Add a 'DEBUG_PERIOD' option to slow down the simulation. Useful in waveform debugging a fast simulation. --- lib/Dialect/ESI/runtime/cosim_dpi_server/driver.cpp | 11 +++++++++++ lib/Dialect/ESI/runtime/cosim_dpi_server/esi-cosim.py | 2 ++ 2 files changed, 13 insertions(+) diff --git a/lib/Dialect/ESI/runtime/cosim_dpi_server/driver.cpp b/lib/Dialect/ESI/runtime/cosim_dpi_server/driver.cpp index a94bd761b6c7..155edf84cd39 100644 --- a/lib/Dialect/ESI/runtime/cosim_dpi_server/driver.cpp +++ b/lib/Dialect/ESI/runtime/cosim_dpi_server/driver.cpp @@ -34,6 +34,7 @@ #include "signal.h" #include +#include vluint64_t timeStamp; @@ -54,6 +55,14 @@ int main(int argc, char **argv) { auto &dut = *new CLASSNAME(V, TOP_MODULE)(); char *waveformFile = getenv("SAVE_WAVE"); + char *periodStr = getenv("DEBUG_PERIOD"); + unsigned debugPeriod = 0; + if (periodStr) { + debugPeriod = std::stoi(periodStr); + std::cout << "[driver] Setting debug period to " << debugPeriod + << std::endl; + } + VerilatedVcdC *tfp = nullptr; if (waveformFile) { #ifdef TRACE @@ -97,6 +106,8 @@ int main(int argc, char **argv) { dut.clk = !dut.clk; if (tfp) tfp->dump(timeStamp); + if (debugPeriod) + std::this_thread::sleep_for(std::chrono::milliseconds(debugPeriod)); } // Tell the simulator that we're going to exit. This flushes the output(s) and diff --git a/lib/Dialect/ESI/runtime/cosim_dpi_server/esi-cosim.py b/lib/Dialect/ESI/runtime/cosim_dpi_server/esi-cosim.py index 61c742c74c6b..d4f611cf7f14 100755 --- a/lib/Dialect/ESI/runtime/cosim_dpi_server/esi-cosim.py +++ b/lib/Dialect/ESI/runtime/cosim_dpi_server/esi-cosim.py @@ -157,6 +157,8 @@ def run(self, inner_command: str, gui: bool = False) -> int: simEnv = Simulator.get_env() if self.debug: simEnv["COSIM_DEBUG_FILE"] = "cosim_debug.log" + # Slow the simulation down to one tick per millisecond. + simEnv["DEBUG_PERIOD"] = "1" simProc = subprocess.Popen(self.run_command(gui), stdout=simStdout, stderr=simStderr, From a91463e62e7da9cfb53bc068aa22fe0e77025c73 Mon Sep 17 00:00:00 2001 From: John Demme Date: Thu, 19 Dec 2024 01:43:32 +0000 Subject: [PATCH 15/27] [ESI Runtime] Remove an errant debug print One of my debug prints slipped through. Fix it. --- lib/Dialect/ESI/runtime/python/esiaccel/types.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Dialect/ESI/runtime/python/esiaccel/types.py b/lib/Dialect/ESI/runtime/python/esiaccel/types.py index cab40c66af2d..9cd4f2d11662 100644 --- a/lib/Dialect/ESI/runtime/python/esiaccel/types.py +++ b/lib/Dialect/ESI/runtime/python/esiaccel/types.py @@ -47,7 +47,6 @@ def supports_host(self) -> Tuple[bool, Optional[str]]: """Does this type support host communication via Python? Returns either '(True, None)' if it is, or '(False, reason)' if it is not.""" - print(f"supports_host: {self.cpp_type} {type(self)}") if self.bit_width % 8 != 0: return (False, "runtime only supports types with multiple of 8 bits") return (True, None) From 8f388d1af9f3078c9fc78e499b4a3f46bca5574e Mon Sep 17 00:00:00 2001 From: Bea Healy <57840981+TaoBi22@users.noreply.github.com> Date: Thu, 19 Dec 2024 14:06:47 +0000 Subject: [PATCH 16/27] [NFC] HWToBTOR2 comment typos --- lib/Conversion/HWToBTOR2/HWToBTOR2.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Conversion/HWToBTOR2/HWToBTOR2.cpp b/lib/Conversion/HWToBTOR2/HWToBTOR2.cpp index 1ac6748e6134..2ecd16cfa8d9 100644 --- a/lib/Conversion/HWToBTOR2/HWToBTOR2.cpp +++ b/lib/Conversion/HWToBTOR2/HWToBTOR2.cpp @@ -765,7 +765,7 @@ struct ConvertHWToBTOR2Pass genUnaryOp(op, expr, "not", 1); } - // Genrate the bad btor2 intruction + // Generate the bad btor2 instruction genBad(op); } // Assumptions are converted to a btor2 constraint instruction @@ -802,7 +802,7 @@ struct ConvertHWToBTOR2Pass assertLID = genUnaryOp(prop.getDefiningOp(), "not", 1); } - // Genrate the bad btor2 intruction + // Generate the bad btor2 instruction genBad(assertLID); } @@ -821,7 +821,7 @@ struct ConvertHWToBTOR2Pass assumeLID = genImplies(op, en, prop); } - // Genrate the bad btor2 intruction + // Generate the bad btor2 instruction genConstraint(assumeLID); } From b9fdb9199d41f61868b5bb79368c9506dfef8608 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Wed, 18 Dec 2024 17:40:15 -0800 Subject: [PATCH 17/27] [Python] Remove multiple inheritance from Comb helper class. As we move towards nanobind, the main issue is that nanobind explicitly does not want to participate in multiple inheritance, which means we can't use multiple inheritance in OpViews. For the comparator comb ops, we decorate helper classes for conctructs like EqOp that delegate to ICmpOp. The decorator creates a helper class that extends OpView with multiple inheritance. We can avoid the multiple inheritance with minor duplication by having each concrete leaf class inherit OpView, rather than mixing in OpView in the generated helper class. --- lib/Bindings/Python/dialects/comb.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/Bindings/Python/dialects/comb.py b/lib/Bindings/Python/dialects/comb.py index c9ca3d662f6a..28f0100e19e0 100644 --- a/lib/Bindings/Python/dialects/comb.py +++ b/lib/Bindings/Python/dialects/comb.py @@ -29,7 +29,7 @@ def CompareOp(predicate): def decorated(cls): - class _Class(cls, OpView): + class _Class(cls): @staticmethod def create(lhs=None, rhs=None): @@ -50,52 +50,52 @@ def create(lhs=None, rhs=None): @CompareOp(0) -class EqOp: +class EqOp(OpView): pass @CompareOp(1) -class NeOp: +class NeOp(OpView): pass @CompareOp(2) -class LtSOp: +class LtSOp(OpView): pass @CompareOp(3) -class LeSOp: +class LeSOp(OpView): pass @CompareOp(4) -class GtSOp: +class GtSOp(OpView): pass @CompareOp(5) -class GeSOp: +class GeSOp(OpView): pass @CompareOp(6) -class LtUOp: +class LtUOp(OpView): pass @CompareOp(7) -class LeUOp: +class LeUOp(OpView): pass @CompareOp(8) -class GtUOp: +class GtUOp(OpView): pass @CompareOp(9) -class GeUOp: +class GeUOp(OpView): pass From 1e194cfc702f5a8131ca67544753417c762cc143 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Thu, 19 Dec 2024 10:15:46 -0800 Subject: [PATCH 18/27] [Python] Remove multiple inheritance from HW helper class. As we move towards nanobind, the main issue is that nanobind explicitly does not want to participate in multiple inheritance, which means we can't use multiple inheritance in OpViews. In the HW bindings, we mix in the ModuleLike class with multiple inheritance to share some code between HWModuleOp and HWModuleExternOp. To avoid multiple inheritance with a little duplication, this changes the helper methods on ModuleLike to staticmethods, and adds corresponding instance methods to HWModuleOp and HWModuleExternOp that delegate to the implementation defined on ModuleLike. --- lib/Bindings/Python/dialects/hw.py | 130 +++++++++++++++++++---------- 1 file changed, 85 insertions(+), 45 deletions(-) diff --git a/lib/Bindings/Python/dialects/hw.py b/lib/Bindings/Python/dialects/hw.py index 53b57104a157..d9398a26ddb1 100644 --- a/lib/Bindings/Python/dialects/hw.py +++ b/lib/Bindings/Python/dialects/hw.py @@ -111,10 +111,11 @@ def result_names(self): class ModuleLike: - """Custom Python base class for module-like operations.""" + """Custom Python helper class for module-like operations.""" - def __init__( - self, + @staticmethod + def init( + op, name, input_ports=[], output_ports=[], @@ -172,40 +173,39 @@ def __init__( attributes["module_type"] = TypeAttr.get(hw.ModuleType.get(module_ports)) _ods_cext.ir.OpView.__init__( - self, - self.build_generic(attributes=attributes, - results=results, - operands=operands, - loc=loc, - ip=ip)) + op, + op.build_generic(attributes=attributes, + results=results, + operands=operands, + loc=loc, + ip=ip)) if body_builder: - entry_block = self.add_entry_block() + entry_block = op.add_entry_block() with InsertionPoint(entry_block): with support.BackedgeBuilder(str(name)): - outputs = body_builder(self) + outputs = body_builder(op) _create_output_op(name, output_ports, entry_block, outputs) - @property - def type(self): - return hw.ModuleType(TypeAttr(self.attributes["module_type"]).value) + @staticmethod + def type(op): + return hw.ModuleType(TypeAttr(op.attributes["module_type"]).value) - @property - def name(self): - return self.attributes["sym_name"] + @staticmethod + def name(op): + return op.attributes["sym_name"] - @property - def is_external(self): - return len(self.regions[0].blocks) == 0 + @staticmethod + def is_external(op): + return len(op.regions[0].blocks) == 0 - @property - def parameters(self) -> list[ParamDeclAttr]: - return [ - hw.ParamDeclAttr(a) for a in ArrayAttr(self.attributes["parameters"]) - ] + @staticmethod + def parameters(op) -> list[ParamDeclAttr]: + return [hw.ParamDeclAttr(a) for a in ArrayAttr(op.attributes["parameters"])] - def instantiate(self, + @staticmethod + def instantiate(op, name: str, parameters: Dict[str, object] = {}, results=None, @@ -213,7 +213,7 @@ def instantiate(self, loc=None, ip=None, **kwargs): - return InstanceBuilder(self, + return InstanceBuilder(op, name, kwargs, parameters=parameters, @@ -291,7 +291,7 @@ def _create_output_op(cls_name, output_ports, entry_block, bb_ret): @_ods_cext.register_operation(_Dialect, replace=True) -class HWModuleOp(ModuleLike, HWModuleOp): +class HWModuleOp(HWModuleOp): """Specialization for the HW module op class.""" def __init__( @@ -308,14 +308,34 @@ def __init__( ): if "comment" not in attributes: attributes["comment"] = StringAttr.get("") - super().__init__(name, - input_ports, - output_ports, - parameters=parameters, - attributes=attributes, - body_builder=body_builder, - loc=loc, - ip=ip) + ModuleLike.init(self, + name, + input_ports, + output_ports, + parameters=parameters, + attributes=attributes, + body_builder=body_builder, + loc=loc, + ip=ip) + + def instantiate(self, *args, **kwargs): + return ModuleLike.instantiate(self, *args, **kwargs) + + @property + def type(self): + return ModuleLike.type(self) + + @property + def name(self): + return ModuleLike.name(self) + + @property + def is_external(self): + return ModuleLike.is_external(self) + + @property + def parameters(self) -> list[ParamDeclAttr]: + return ModuleLike.parameters(self) @property def body(self): @@ -359,7 +379,7 @@ def add_entry_block(self): @_ods_cext.register_operation(_Dialect, replace=True) -class HWModuleExternOp(ModuleLike, HWModuleExternOp): +class HWModuleExternOp(HWModuleExternOp): """Specialization for the HW module op class.""" def __init__( @@ -376,14 +396,34 @@ def __init__( ): if "comment" not in attributes: attributes["comment"] = StringAttr.get("") - super().__init__(name, - input_ports, - output_ports, - parameters=parameters, - attributes=attributes, - body_builder=body_builder, - loc=loc, - ip=ip) + ModuleLike.init(self, + name, + input_ports, + output_ports, + parameters=parameters, + attributes=attributes, + body_builder=body_builder, + loc=loc, + ip=ip) + + def instantiate(self, *args, **kwargs): + return ModuleLike.instantiate(self, *args, **kwargs) + + @property + def type(self): + return ModuleLike.type(self) + + @property + def name(self): + return ModuleLike.name(self) + + @property + def is_external(self): + return ModuleLike.is_external(self) + + @property + def parameters(self) -> list[ParamDeclAttr]: + return ModuleLike.parameters(self) @_ods_cext.register_operation(_Dialect, replace=True) From 812cbe9939566ccee00460892dce81e0cfec83c5 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Thu, 19 Dec 2024 10:22:02 -0800 Subject: [PATCH 19/27] [Python] Remove multiple inheritance from Seq helper class. As we move towards nanobind, the main issue is that nanobind explicitly does not want to participate in multiple inheritance, which means we can't use multiple inheritance in OpViews. In the Seq bindings, we mix in the CompRegLike class with multiple inheritance to share the constructor code betwee CompRegOp and CompRegClockEnabledOp. To avoid multiple inheritance with a little duplication, this changes the __init__ method on CompRegLike to a staticmethod, and adds corresponding __init__ methods to CompRegOp and CompRegClockEnabledOp that delegate to the implementation defined on CompRegLike. --- lib/Bindings/Python/dialects/seq.py | 36 +++++++++++++++++------------ 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/Bindings/Python/dialects/seq.py b/lib/Bindings/Python/dialects/seq.py index 2f43ccbb3de7..1232b9c866c9 100644 --- a/lib/Bindings/Python/dialects/seq.py +++ b/lib/Bindings/Python/dialects/seq.py @@ -56,19 +56,19 @@ def create_initial_value(self, index, data_type, arg_name): class CompRegLike: - def __init__(self, - data_type, - input, - clk, - clockEnable=None, - *, - reset=None, - reset_value=None, - power_on_value=None, - name=None, - sym_name=None, - loc=None, - ip=None): + def init(self, + data_type, + input, + clk, + clockEnable=None, + *, + reset=None, + reset_value=None, + power_on_value=None, + name=None, + sym_name=None, + loc=None, + ip=None): operands = [input, clk] results = [] attributes = {} @@ -141,7 +141,10 @@ def operand_names(self): @_ods_cext.register_operation(_Dialect, replace=True) -class CompRegOp(CompRegLike, CompRegOp): +class CompRegOp(CompRegOp): + + def __init__(self, *args, **kwargs): + CompRegLike.init(self, *args, **kwargs) @classmethod def create(cls, @@ -168,7 +171,10 @@ def operand_names(self): @_ods_cext.register_operation(_Dialect, replace=True) -class CompRegClockEnabledOp(CompRegLike, CompRegClockEnabledOp): +class CompRegClockEnabledOp(CompRegClockEnabledOp): + + def __init__(self, *args, **kwargs): + CompRegLike.init(self, *args, **kwargs) @classmethod def create(cls, From b56daccff78fabe73dda9bea1aafedb1375a1770 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Thu, 19 Dec 2024 15:08:09 -0800 Subject: [PATCH 20/27] [Python] Manually cast enums while we transition to nanobind. As we transition from pybind11 to nanobind, the enums defined upstream will be defined by nanobind. This no longer works with automatic casting in the pybind11 APIs we define ourselves. To work around this while we smooth the transition, this updates the APIs to work with plain py::objects and manually cast to the appropriate C API enums in a way that's compatible with both the pybind11 and nanobind versions of enums. --- lib/Bindings/Python/SupportModule.cpp | 44 ++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/lib/Bindings/Python/SupportModule.cpp b/lib/Bindings/Python/SupportModule.cpp index e32010373985..ed159c5aa554 100644 --- a/lib/Bindings/Python/SupportModule.cpp +++ b/lib/Bindings/Python/SupportModule.cpp @@ -28,16 +28,32 @@ void circt::python::populateSupportSubmodule(py::module &m) { m.def( "_walk_with_filter", [](MlirOperation operation, const std::vector &opNames, - std::function callback, - MlirWalkOrder walkOrder) { + std::function callback, + py::object walkOrderRaw) { struct UserData { - std::function callback; + std::function callback; bool gotException; std::string exceptionWhat; py::object exceptionType; std::vector opNames; }; + // As we transition from pybind11 to nanobind, the WalkOrder enum and + // automatic casting will be defined as a nanobind enum upstream. Do a + // manual conversion that works with either pybind11 or nanobind for + // now. When we're on nanobind in CIRCT, we can go back to automatic + // casting. + MlirWalkOrder walkOrder; + auto walkOrderRawValue = py::cast(walkOrderRaw.attr("value")); + switch (walkOrderRawValue) { + case 0: + walkOrder = MlirWalkOrder::MlirWalkPreOrder; + break; + case 1: + walkOrder = MlirWalkOrder::MlirWalkPostOrder; + break; + } + std::vector opNamesIdentifiers; opNamesIdentifiers.reserve(opNames.size()); @@ -68,7 +84,27 @@ void circt::python::populateSupportSubmodule(py::module &m) { return MlirWalkResult::MlirWalkResultAdvance; try { - return (calleeUserData->callback)(op); + // As we transition from pybind11 to nanobind, the WalkResult enum + // and automatic casting will be defined as a nanobind enum + // upstream. Do a manual conversion that works with either pybind11 + // or nanobind for now. When we're on nanobind in CIRCT, we can go + // back to automatic casting. + MlirWalkResult walkResult; + auto walkResultRaw = (calleeUserData->callback)(op); + auto walkResultRawValue = + py::cast(walkResultRaw.attr("value")); + switch (walkResultRawValue) { + case 0: + walkResult = MlirWalkResult::MlirWalkResultAdvance; + break; + case 1: + walkResult = MlirWalkResult::MlirWalkResultInterrupt; + break; + case 2: + walkResult = MlirWalkResult::MlirWalkResultSkip; + break; + } + return walkResult; } catch (py::error_already_set &e) { calleeUserData->gotException = true; calleeUserData->exceptionWhat = e.what(); From acd9ae245e720cf20da79bd62e76d6485e43957f Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Thu, 19 Dec 2024 15:29:48 -0800 Subject: [PATCH 21/27] Bump LLVM to 6a01ac7d06df875206f746fc982f58c161249285. This is a clean integrate. Notably, this does pick up the upstream MLIR switch to using nanobind for the core Python library. We are compatible with this change from the four patches immediately preceding this bump. We are free to switch our own dialects and libraries to nanobind at any point. --- llvm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm b/llvm index b0b546d44777..6a01ac7d06df 160000 --- a/llvm +++ b/llvm @@ -1 +1 @@ -Subproject commit b0b546d44777eb1fa25995384876bd14a006a929 +Subproject commit 6a01ac7d06df875206f746fc982f58c161249285 From c5f287513367b13b83df822282c1f47056fcc9f2 Mon Sep 17 00:00:00 2001 From: Mike Urbach Date: Thu, 19 Dec 2024 16:07:11 -0800 Subject: [PATCH 22/27] [Python] Reword CompRegLike helper for clarity, NFC. This was migrated to a helper class with a static method rather than a base class in 812cbe9. But the method naming accepting a `self` parameter is confusing, and a `@staticmethod` annotation would also be appropriate. This adds the annotations and renames `self` to `op` for clarity. --- lib/Bindings/Python/dialects/seq.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/Bindings/Python/dialects/seq.py b/lib/Bindings/Python/dialects/seq.py index 1232b9c866c9..d9630443c3f4 100644 --- a/lib/Bindings/Python/dialects/seq.py +++ b/lib/Bindings/Python/dialects/seq.py @@ -56,7 +56,8 @@ def create_initial_value(self, index, data_type, arg_name): class CompRegLike: - def init(self, + @staticmethod + def init(op, data_type, input, clk, @@ -74,10 +75,10 @@ def init(self, attributes = {} results.append(data_type) operand_segment_sizes = [1, 1] - if isinstance(self, CompRegOp): + if isinstance(op, CompRegOp): if clockEnable is not None: raise Exception("Clock enable not supported on compreg") - elif isinstance(self, CompRegClockEnabledOp): + elif isinstance(op, CompRegClockEnabledOp): if clockEnable is None: raise Exception("Clock enable required on compreg.ce") operands.append(clockEnable) @@ -120,11 +121,11 @@ def init(self, if sym_name is not None: attributes["inner_sym"] = hw.InnerSymAttr.get(StringAttr.get(sym_name)) - self._ODS_OPERAND_SEGMENTS = operand_segment_sizes + op._ODS_OPERAND_SEGMENTS = operand_segment_sizes OpView.__init__( - self, - self.build_generic( + op, + op.build_generic( attributes=attributes, results=results, operands=operands, From cea3812058cc90874175fe2856ae36aa29bf0aff Mon Sep 17 00:00:00 2001 From: John Demme Date: Fri, 20 Dec 2024 14:20:30 -0500 Subject: [PATCH 23/27] [ESI] FIFO: support valid/ready on inputs and outputs (#8009) --- .../Dialect/ESI/widgets/esi_widgets.mlir | 40 +++++- .../Dialect/ESI/widgets/esi_widgets.py | 132 ++++++++++++++---- lib/Dialect/ESI/ESIOps.cpp | 8 +- lib/Dialect/ESI/Passes/ESILowerPhysical.cpp | 56 ++++++-- test/Dialect/ESI/lowering.mlir | 39 ++++++ 5 files changed, 228 insertions(+), 47 deletions(-) diff --git a/integration_test/Dialect/ESI/widgets/esi_widgets.mlir b/integration_test/Dialect/ESI/widgets/esi_widgets.mlir index 2a87add26544..fbdae562e4eb 100644 --- a/integration_test/Dialect/ESI/widgets/esi_widgets.mlir +++ b/integration_test/Dialect/ESI/widgets/esi_widgets.mlir @@ -2,12 +2,46 @@ // Test the original HandshakeToHW flow. -// RUN: circt-opt %s --lower-esi-to-physical --lower-esi-ports --lower-esi-to-hw --lower-seq-fifo --lower-seq-hlmem --lower-seq-to-sv --lower-verif-to-sv --sv-trace-iverilog --prettify-verilog --export-verilog -o %t.hw.mlir > %t.sv -// RUN: circt-cocotb-driver.py --objdir=%T --topLevel=fifo1 --pythonModule=esi_widgets --pythonFolder="%S" %t.sv %esi_prims +// RUN: circt-opt %s --lower-esi-to-physical --lower-esi-ports --lower-esi-to-hw --lower-seq-fifo --lower-seq-hlmem --lower-seq-to-sv --lower-verif-to-sv --canonicalize --prettify-verilog --export-verilog -o %t.hw.mlir > %t.sv +// RUN: circt-cocotb-driver.py --objdir=%T --topLevel=top --pythonModule=esi_widgets --pythonFolder="%S" %t.sv %esi_prims module attributes {circt.loweringOptions = "disallowLocalVariables"} { - hw.module @fifo1(in %clk: !seq.clock, in %rst: i1, in %in: !esi.channel, out out: !esi.channel) { + hw.module @fifo1( + in %clk: !seq.clock, in %rst: i1, + in %in: !esi.channel, + out out: !esi.channel) { %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel -> !esi.channel hw.output %fifo : !esi.channel } + + hw.module @fifoValidReadyInput( + in %clk: !seq.clock, in %rst: i1, + in %in: !esi.channel, + out out: !esi.channel) { + %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel -> !esi.channel + hw.output %fifo : !esi.channel + } + + hw.module @fifoValidReadyOutput( + in %clk: !seq.clock, in %rst: i1, + in %in: !esi.channel, + out out: !esi.channel) { + %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel -> !esi.channel + hw.output %fifo : !esi.channel + } + + + hw.module @top(in %clk: !seq.clock, in %rst: i1, + in %fifo1_in: !esi.channel, out fifo1_out: !esi.channel, + in %fifoValidReadyInput_in: !esi.channel, out fifoValidReadyInput_out: !esi.channel, + in %fifoValidReadyOutput_in: !esi.channel, out fifoValidReadyOutput_out: !esi.channel + ) { + %fifo1 = hw.instance "fifo1" @fifo1( + clk: %clk: !seq.clock, rst: %rst: i1, in: %fifo1_in: !esi.channel) -> (out: !esi.channel) + %fifoValidReadyInput = hw.instance "fifoValidReadyInput" @fifoValidReadyInput( + clk: %clk: !seq.clock, rst: %rst: i1, in: %fifoValidReadyInput_in: !esi.channel) -> (out: !esi.channel) + %fifoValidReadyOutput = hw.instance "fifoValidReadyOutput" @fifoValidReadyOutput( + clk: %clk: !seq.clock, rst: %rst: i1, in: %fifoValidReadyOutput_in: !esi.channel) -> (out: !esi.channel) + hw.output %fifo1, %fifoValidReadyInput, %fifoValidReadyOutput : !esi.channel, !esi.channel, !esi.channel + } } diff --git a/integration_test/Dialect/ESI/widgets/esi_widgets.py b/integration_test/Dialect/ESI/widgets/esi_widgets.py index e84fb76d1d14..45e9b278b0d1 100644 --- a/integration_test/Dialect/ESI/widgets/esi_widgets.py +++ b/integration_test/Dialect/ESI/widgets/esi_widgets.py @@ -6,7 +6,7 @@ async def init(dut, timeout: Optional[int] = None): # Create a 10ns period (100MHz) clock on port clock - clk = Clock(dut.clk, 10, units="ns") + clk = Clock(dut.clk, 1, units="ns") cocotb.start_soon(clk.start()) # Start the clock # Reset @@ -53,62 +53,103 @@ async def write(self, data: int): self.empty.value = 1 +class ESIValidReadyInputPort(ESIInputPort): + + def __init__(self, dut, name): + self.dut = dut + self.name = name + self.data = getattr(dut, name) + self.valid = getattr(dut, f"{name}_valid") + self.ready = getattr(dut, f"{name}_ready") + # Configure initial state + self.valid.value = 0 + + async def write(self, data: int): + self.data.value = data + self.valid.value = 1 + await RisingEdge(self.dut.clk) + while self.ready.value == 0: + await RisingEdge(self.dut.clk) + self.valid.value = 0 + + class ESIOutputPort: + def __init__(self, dut, name: str, latency: int = 0): + self.dut = dut + self.name = name + self.data = getattr(dut, name) + self.latency = latency + self.q: List[int] = [] + async def read(self) -> Optional[int]: raise RuntimeError("Must be implemented by subclass") async def cmd_read(self): raise RuntimeError("Must be implemented by subclass") + async def read_after_latency(self): + for i in range(self.latency): + await RisingEdge(self.dut.clk) + self.q.append(self.data.value) + class ESIFifoOutputPort(ESIOutputPort): def __init__(self, dut, name: str, latency: int = 0): - self.dut = dut - self.name = name - self.data = getattr(dut, name) + super().__init__(dut, name, latency) + self.rden = getattr(dut, f"{name}_rden") self.empty = getattr(dut, f"{name}_empty") - self.latency = latency # Configure initial state self.rden.value = 0 - self.running = 0 - self.q: List[int] = [] async def init_read(self): - - async def read_after_latency(): - for i in range(self.latency): - await RisingEdge(self.dut.clk) - self.q.append(self.data.value) - self.running = 1 self.empty.value = 0 while await RisingEdge(self.dut.clk): if self.rden.value == 1: - cocotb.start_soon(read_after_latency()) + cocotb.start_soon(self.read_after_latency()) async def read(self) -> Optional[int]: - if len(self.q) == 0: + while len(self.q) == 0: await RisingEdge(self.dut.clk) return self.q.pop(0) async def cmd_read(self): - if self.running == 0: - cocotb.start_soon(self.init_read()) - while self.empty.value == 1: await RisingEdge(self.dut.clk) self.rden.value = 1 await RisingEdge(self.dut.clk) self.rden.value = 0 + cocotb.start_soon(self.read_after_latency()) -@cocotb.test() -async def fillAndDrain(dut): - in_port = ESIFifoInputPort(dut, "in") - out_port = ESIFifoOutputPort(dut, "out", 2) +class ESIValidReadyOutputPort(ESIOutputPort): + + def __init__(self, dut, name: str, latency: int = 0): + super().__init__(dut, name, latency) + + self.valid = getattr(dut, f"{name}_valid") + self.ready = getattr(dut, f"{name}_ready") + # Configure initial state + self.ready.value = 0 + + async def read(self) -> Optional[int]: + while len(self.q) == 0: + await RisingEdge(self.dut.clk) + return self.q.pop(0) + + async def cmd_read(self): + self.ready.value = 1 + await RisingEdge(self.dut.clk) + while self.valid.value == 0: + await RisingEdge(self.dut.clk) + self.ready.value = 0 + cocotb.start_soon(self.read_after_latency()) + + +async def runFillAndDrain(dut, in_port, out_port): await init(dut, timeout=10000) for i in range(10): @@ -125,10 +166,7 @@ async def fillAndDrain(dut): await RisingEdge(dut.clk) -@cocotb.test() -async def backToBack(dut): - in_port = ESIFifoInputPort(dut, "in") - out_port = ESIFifoOutputPort(dut, "out", 2) +async def runBackToBack(dut, in_port, out_port): await init(dut) NUM_ITERS = 500 @@ -149,3 +187,45 @@ async def write(): for i in range(4): await RisingEdge(dut.clk) + + +@cocotb.test() +async def fillAndDrainFIFO(dut): + in_port = ESIFifoInputPort(dut, "fifo1_in") + out_port = ESIFifoOutputPort(dut, "fifo1_out", 2) + await runFillAndDrain(dut, in_port, out_port) + + +@cocotb.test() +async def backToBack(dut): + in_port = ESIFifoInputPort(dut, "fifo1_in") + out_port = ESIFifoOutputPort(dut, "fifo1_out", 2) + await runBackToBack(dut, in_port, out_port) + + +@cocotb.test() +async def fillAndDrainValidReadyInputFIFO(dut): + in_port = ESIValidReadyInputPort(dut, "fifoValidReadyInput_in") + out_port = ESIFifoOutputPort(dut, "fifoValidReadyInput_out", 2) + await runFillAndDrain(dut, in_port, out_port) + + +@cocotb.test() +async def backToBackValidReadyInputFIFO(dut): + in_port = ESIValidReadyInputPort(dut, "fifoValidReadyInput_in") + out_port = ESIFifoOutputPort(dut, "fifoValidReadyInput_out", 2) + await runBackToBack(dut, in_port, out_port) + + +@cocotb.test() +async def fillAndDrainValidReadyOutputFIFO(dut): + in_port = ESIFifoInputPort(dut, "fifoValidReadyOutput_in") + out_port = ESIValidReadyOutputPort(dut, "fifoValidReadyOutput_out", 0) + await runFillAndDrain(dut, in_port, out_port) + + +@cocotb.test() +async def backToBackValidReadyOutputFIFO(dut): + in_port = ESIFifoInputPort(dut, "fifoValidReadyOutput_in") + out_port = ESIValidReadyOutputPort(dut, "fifoValidReadyOutput_out", 0) + await runBackToBack(dut, in_port, out_port) diff --git a/lib/Dialect/ESI/ESIOps.cpp b/lib/Dialect/ESI/ESIOps.cpp index 4c94935b14eb..f0f361431546 100644 --- a/lib/Dialect/ESI/ESIOps.cpp +++ b/lib/Dialect/ESI/ESIOps.cpp @@ -77,13 +77,7 @@ LogicalResult ChannelBufferOp::verify() { //===----------------------------------------------------------------------===// LogicalResult FIFOOp::verify() { - ChannelType inputType = getInput().getType(); - if (inputType.getSignaling() != ChannelSignaling::FIFO) - return emitOpError("only supports FIFO signaling on input"); - ChannelType outputType = getOutput().getType(); - if (outputType.getSignaling() != ChannelSignaling::FIFO) - return emitOpError("only supports FIFO signaling on output"); - if (outputType.getInner() != inputType.getInner()) + if (getOutput().getType().getInner() != getInput().getType().getInner()) return emitOpError("input and output types must match"); return success(); } diff --git a/lib/Dialect/ESI/Passes/ESILowerPhysical.cpp b/lib/Dialect/ESI/Passes/ESILowerPhysical.cpp index 172ddb82b277..564c77cd5dec 100644 --- a/lib/Dialect/ESI/Passes/ESILowerPhysical.cpp +++ b/lib/Dialect/ESI/Passes/ESILowerPhysical.cpp @@ -103,21 +103,38 @@ FIFOLowering::matchAndRewrite(FIFOOp op, OpAdaptor adaptor, auto i1 = rewriter.getI1Type(); auto c1 = rewriter.create(loc, rewriter.getI1Type(), rewriter.getBoolAttr(true)); - if (op.getInput().getType().getDataDelay() != 0) + mlir::TypedValue chanInput = op.getInput(); + if (chanInput.getType().getDataDelay() != 0) return op.emitOpError( "currently only supports input channels with zero data delay"); Backedge inputEn = bb.get(i1); - auto unwrapPull = rewriter.create(loc, op.getInput(), inputEn); + Value rawData; + Value dataNotAvailable; + if (chanInput.getType().getSignaling() == ChannelSignaling::ValidReady) { + auto unwrapValidReady = + rewriter.create(loc, chanInput, inputEn); + rawData = unwrapValidReady.getRawOutput(); + dataNotAvailable = comb::createOrFoldNot(loc, unwrapValidReady.getValid(), + rewriter, /*twoState=*/true); + dataNotAvailable.getDefiningOp()->setAttr( + "sv.namehint", rewriter.getStringAttr("dataNotAvailable")); + } else if (chanInput.getType().getSignaling() == ChannelSignaling::FIFO) { + auto unwrapPull = rewriter.create(loc, chanInput, inputEn); + rawData = unwrapPull.getData(); + dataNotAvailable = unwrapPull.getEmpty(); + } else { + return rewriter.notifyMatchFailure( + op, "only supports ValidReady and FIFO signaling"); + } Backedge outputRdEn = bb.get(i1); auto seqFifo = rewriter.create( - loc, outputType.getInner(), i1, i1, Type(), Type(), unwrapPull.getData(), - outputRdEn, inputEn, op.getClk(), op.getRst(), op.getDepthAttr(), + loc, outputType.getInner(), i1, i1, Type(), Type(), rawData, outputRdEn, + inputEn, op.getClk(), op.getRst(), op.getDepthAttr(), rewriter.getI64IntegerAttr(outputType.getDataDelay()), IntegerAttr(), IntegerAttr()); - auto inputNotEmpty = - rewriter.create(loc, unwrapPull.getEmpty(), c1); + auto inputNotEmpty = rewriter.create(loc, dataNotAvailable, c1); inputNotEmpty->setAttr("sv.namehint", rewriter.getStringAttr("inputNotEmpty")); auto seqFifoNotFull = @@ -129,12 +146,29 @@ FIFOLowering::matchAndRewrite(FIFOOp op, OpAdaptor adaptor, static_cast(inputEn).getDefiningOp()->setAttr( "sv.namehint", rewriter.getStringAttr("inputEn")); - auto output = - rewriter.create(loc, mlir::TypeRange{outputType, i1}, - seqFifo.getOutput(), seqFifo.getEmpty()); - outputRdEn.setValue(output.getRden()); + Value output; + if (outputType.getSignaling() == ChannelSignaling::ValidReady) { + auto wrap = rewriter.create( + loc, mlir::TypeRange{outputType, i1}, seqFifo.getOutput(), + comb::createOrFoldNot(loc, seqFifo.getEmpty(), rewriter, + /*twoState=*/true)); + output = wrap.getChanOutput(); + outputRdEn.setValue( + rewriter.create(loc, wrap.getValid(), wrap.getReady())); + static_cast(outputRdEn) + .getDefiningOp() + ->setAttr("sv.namehint", rewriter.getStringAttr("outputRdEn")); + } else if (outputType.getSignaling() == ChannelSignaling::FIFO) { + auto wrap = + rewriter.create(loc, mlir::TypeRange{outputType, i1}, + seqFifo.getOutput(), seqFifo.getEmpty()); + output = wrap.getChanOutput(); + outputRdEn.setValue(wrap.getRden()); + } else { + return rewriter.notifyMatchFailure(op, "only supports ValidReady and FIFO"); + } - rewriter.replaceOp(op, output.getChanOutput()); + rewriter.replaceOp(op, output); return success(); } diff --git a/test/Dialect/ESI/lowering.mlir b/test/Dialect/ESI/lowering.mlir index 50b018603610..93338506ad95 100644 --- a/test/Dialect/ESI/lowering.mlir +++ b/test/Dialect/ESI/lowering.mlir @@ -211,3 +211,42 @@ hw.module @fifo1(in %clk: !seq.clock, in %rst: i1, in %in: !esi.channel -> !esi.channel hw.output %fifo : !esi.channel } + +// CHECK-LABEL: hw.module @fifoValidReadyInput(in %clk : !seq.clock, in %rst : i1, in %in : !esi.channel, out out : !esi.channel) { +// CHECK-NEXT: %true = hw.constant true +// CHECK-NEXT: %rawOutput, %valid = esi.unwrap.vr %in, [[InputEN:%.+]] : i32 +// CHECK-NEXT: %true_0 = hw.constant true +// CHECK-NEXT: [[DataNotAvailable:%.+]] = comb.xor bin %valid, %true_0 {sv.namehint = "dataNotAvailable"} : i1 +// CHECK-NEXT: %out, %full, %empty = seq.fifo depth 12 rd_latency 2 in %rawOutput rdEn %rden wrEn [[InputEN]] clk %clk rst %rst : i32 +// CHECK-NEXT: [[InputNotEmpty:%.+]] = comb.xor [[DataNotAvailable]], %true {sv.namehint = "inputNotEmpty"} : i1 +// CHECK-NEXT: [[SeqFifoNotFull:%.+]] = comb.xor %full, %true {sv.namehint = "seqFifoNotFull"} : i1 +// CHECK-NEXT: [[InputEN]] = comb.and [[InputNotEmpty]], [[SeqFifoNotFull]] {sv.namehint = "inputEn"} : i1 +// CHECK-NEXT: %chanOutput, %rden = esi.wrap.fifo %out, %empty : !esi.channel +// CHECK-NEXT: hw.output %chanOutput : !esi.channel +hw.module @fifoValidReadyInput( + in %clk: !seq.clock, in %rst: i1, + in %in: !esi.channel, + out out: !esi.channel) { + %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel -> !esi.channel + hw.output %fifo : !esi.channel +} + +// CHECK-LABEL: hw.module @fifoValidReadyOutput(in %clk : !seq.clock, in %rst : i1, in %in : !esi.channel, out out : !esi.channel) { +// CHECK-NEXT: %true = hw.constant true +// CHECK-NEXT: %data, %empty = esi.unwrap.fifo %in, [[InputEN:%.+]] : !esi.channel +// CHECK-NEXT: %out, %full, %empty_0 = seq.fifo depth 12 in %data rdEn [[OutputRdEn:%.+]] wrEn [[InputEN]] clk %clk rst %rst : i32 +// CHECK-NEXT: [[InputNotEmpty:%.+]] = comb.xor %empty, %true {sv.namehint = "inputNotEmpty"} : i1 +// CHECK-NEXT: [[SeqFifoNotFull:%.+]] = comb.xor %full, %true {sv.namehint = "seqFifoNotFull"} : i1 +// CHECK-NEXT: [[InputEN]] = comb.and [[InputNotEmpty]], [[SeqFifoNotFull]] {sv.namehint = "inputEn"} : i1 +// CHECK-NEXT: %true_1 = hw.constant true +// CHECK-NEXT: [[R3:%.+]] = comb.xor bin %empty_0, %true_1 : i1 +// CHECK-NEXT: %chanOutput, %ready = esi.wrap.vr %out, [[R3]] : i32 +// CHECK-NEXT: [[OutputRdEn]] = comb.and [[R3]], %ready {sv.namehint = "outputRdEn"} : i1 +// CHECK-NEXT: hw.output %chanOutput : !esi.channel +hw.module @fifoValidReadyOutput( + in %clk: !seq.clock, in %rst: i1, + in %in: !esi.channel, + out out: !esi.channel) { + %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel -> !esi.channel + hw.output %fifo : !esi.channel +} From 40c201414af3ff687ebd585673feca339b3009aa Mon Sep 17 00:00:00 2001 From: John Demme Date: Fri, 20 Dec 2024 18:53:00 -0500 Subject: [PATCH 24/27] [PyCDE] Add fork, join, and merge channel functions (#8011) - .fork creates two new channels, waits until they are both available, then accepts an input. Also buffer the output channels to avoid combinational loops. - Channel.join waits on two channels then creates a message on the one output channel containing a struct with field 'a' equal to input channel A's content and likewise for channel B. - Channel.merge funnels two channels together into a single output stream. This is functionality which really should be handled by the DC dialect but it's not ready for primetime. --- .../PyCDE/integration_test/esi_advanced.py | 101 ++++++++++++++++++ .../test_software/esi_advanced.py | 53 +++++++++ frontends/PyCDE/src/pycde/bsp/__init__.py | 14 +++ frontends/PyCDE/src/pycde/signals.py | 18 ++++ frontends/PyCDE/src/pycde/types.py | 62 ++++++++++- frontends/PyCDE/test/test_esi_advanced.py | 88 +++++++++++++++ 6 files changed, 332 insertions(+), 4 deletions(-) create mode 100644 frontends/PyCDE/integration_test/esi_advanced.py create mode 100644 frontends/PyCDE/integration_test/test_software/esi_advanced.py create mode 100644 frontends/PyCDE/test/test_esi_advanced.py diff --git a/frontends/PyCDE/integration_test/esi_advanced.py b/frontends/PyCDE/integration_test/esi_advanced.py new file mode 100644 index 000000000000..f0967d598157 --- /dev/null +++ b/frontends/PyCDE/integration_test/esi_advanced.py @@ -0,0 +1,101 @@ +# REQUIRES: esi-runtime, esi-cosim, rtl-sim +# RUN: rm -rf %t +# RUN: mkdir %t && cd %t +# RUN: %PYTHON% %s %t 2>&1 +# RUN: esi-cosim.py -- %PYTHON% %S/test_software/esi_advanced.py cosim env + +import sys + +from pycde import generator, Clock, Module, Reset, System +from pycde.bsp import get_bsp +from pycde.common import InputChannel, OutputChannel, Output +from pycde.types import Bits, UInt +from pycde import esi + + +class Merge(Module): + clk = Clock() + rst = Reset() + a = InputChannel(UInt(8)) + b = InputChannel(UInt(8)) + + x = OutputChannel(UInt(8)) + + @generator + def build(ports): + chan = ports.a.type.merge(ports.a, ports.b) + ports.x = chan + + +class Join(Module): + clk = Clock() + rst = Reset() + a = InputChannel(UInt(8)) + b = InputChannel(UInt(8)) + + x = OutputChannel(UInt(9)) + + @generator + def build(ports): + joined = ports.a.type.join(ports.a, ports.b) + ports.x = joined.transform(lambda x: x.a + x.b) + + +class Fork(Module): + clk = Clock() + rst = Reset() + a = InputChannel(UInt(8)) + + x = OutputChannel(UInt(8)) + y = OutputChannel(UInt(8)) + + @generator + def build(ports): + x, y = ports.a.fork(ports.clk, ports.rst) + ports.x = x + ports.y = y + + +class Top(Module): + clk = Clock() + rst = Reset() + + @generator + def build(ports): + clk = ports.clk + rst = ports.rst + merge_a = esi.ChannelService.from_host(esi.AppID("merge_a"), + UInt(8)).buffer(clk, rst, 1) + merge_b = esi.ChannelService.from_host(esi.AppID("merge_b"), + UInt(8)).buffer(clk, rst, 1) + merge = Merge("merge_i8", + clk=ports.clk, + rst=ports.rst, + a=merge_a, + b=merge_b) + esi.ChannelService.to_host(esi.AppID("merge_x"), + merge.x.buffer(clk, rst, 1)) + + join_a = esi.ChannelService.from_host(esi.AppID("join_a"), + UInt(8)).buffer(clk, rst, 1) + join_b = esi.ChannelService.from_host(esi.AppID("join_b"), + UInt(8)).buffer(clk, rst, 1) + join = Join("join_i8", clk=ports.clk, rst=ports.rst, a=join_a, b=join_b) + esi.ChannelService.to_host( + esi.AppID("join_x"), + join.x.buffer(clk, rst, 1).transform(lambda x: x.as_uint(16))) + + fork_a = esi.ChannelService.from_host(esi.AppID("fork_a"), + UInt(8)).buffer(clk, rst, 1) + fork = Fork("fork_i8", clk=ports.clk, rst=ports.rst, a=fork_a) + esi.ChannelService.to_host(esi.AppID("fork_x"), fork.x.buffer(clk, rst, 1)) + esi.ChannelService.to_host(esi.AppID("fork_y"), fork.y.buffer(clk, rst, 1)) + + +if __name__ == "__main__": + bsp = get_bsp(sys.argv[2] if len(sys.argv) > 2 else None) + s = System(bsp(Top), name="ESIAdvanced", output_directory=sys.argv[1]) + s.generate() + s.run_passes() + s.compile() + s.package() diff --git a/frontends/PyCDE/integration_test/test_software/esi_advanced.py b/frontends/PyCDE/integration_test/test_software/esi_advanced.py new file mode 100644 index 000000000000..aa07f66522f8 --- /dev/null +++ b/frontends/PyCDE/integration_test/test_software/esi_advanced.py @@ -0,0 +1,53 @@ +import esiaccel as esi +import sys + +platform = sys.argv[1] +acc = esi.AcceleratorConnection(platform, sys.argv[2]) + +d = acc.build_accelerator() + +merge_a = d.ports[esi.AppID("merge_a")].write_port("data") +merge_a.connect() +merge_b = d.ports[esi.AppID("merge_b")].write_port("data") +merge_b.connect() +merge_x = d.ports[esi.AppID("merge_x")].read_port("data") +merge_x.connect() + +for i in range(10, 15): + merge_a.write(i) + merge_b.write(i + 10) + x1 = merge_x.read() + x2 = merge_x.read() + print(f"merge_a: {i}, merge_b: {i + 10}, " + f"merge_x 1: {x1}, merge_x 2: {x2}") + assert x1 == i + 10 or x1 == i + assert x2 == i + 10 or x2 == i + assert x1 != x2 + +join_a = d.ports[esi.AppID("join_a")].write_port("data") +join_a.connect() +join_b = d.ports[esi.AppID("join_b")].write_port("data") +join_b.connect() +join_x = d.ports[esi.AppID("join_x")].read_port("data") +join_x.connect() + +for i in range(15, 27): + join_a.write(i) + join_b.write(i + 10) + x = join_x.read() + print(f"join_a: {i}, join_b: {i + 10}, join_x: {x}") + assert x == (i + i + 10) & 0xFFFF + +fork_a = d.ports[esi.AppID("fork_a")].write_port("data") +fork_a.connect() +fork_x = d.ports[esi.AppID("fork_x")].read_port("data") +fork_x.connect() +fork_y = d.ports[esi.AppID("fork_y")].read_port("data") +fork_y.connect() + +for i in range(27, 33): + fork_a.write(i) + x = fork_x.read() + y = fork_y.read() + print(f"fork_a: {i}, fork_x: {x}, fork_y: {y}") + assert x == y diff --git a/frontends/PyCDE/src/pycde/bsp/__init__.py b/frontends/PyCDE/src/pycde/bsp/__init__.py index c4c37e67ab9b..9e2f7f23715b 100644 --- a/frontends/PyCDE/src/pycde/bsp/__init__.py +++ b/frontends/PyCDE/src/pycde/bsp/__init__.py @@ -2,5 +2,19 @@ # See https://llvm.org/LICENSE.txt for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +from typing import Optional + from .cosim import CosimBSP from .xrt import XrtBSP + + +def get_bsp(name: Optional[str] = None): + if name is None or name == "cosim": + return CosimBSP + elif name == "xrt": + return XrtBSP + elif name == "xrt_cosim": + from .xrt import XrtCosimBSP + return XrtCosimBSP + else: + raise ValueError(f"Unknown bsp type: {name}") diff --git a/frontends/PyCDE/src/pycde/signals.py b/frontends/PyCDE/src/pycde/signals.py index 689b5c816bc5..8cd0e0459d1f 100644 --- a/frontends/PyCDE/src/pycde/signals.py +++ b/frontends/PyCDE/src/pycde/signals.py @@ -155,6 +155,9 @@ def name(self, new: str): else: self._name = new + def get_name(self, default: str = "") -> str: + return self.name if self.name is not None else default + @property def appid(self) -> Optional[object]: # Optional AppID. from .module import AppID @@ -752,6 +755,21 @@ def transform(self, transform: Callable[[Signal], Signal]) -> ChannelSignal: ready_wire.assign(ready) return ret_chan + def fork(self, clk, rst) -> Tuple[ChannelSignal, ChannelSignal]: + """Fork the channel into two channels, returning the two new channels.""" + from .constructs import Wire + from .types import Bits + both_ready = Wire(Bits(1)) + both_ready.name = self.get_name() + "_fork_both_ready" + data, valid = self.unwrap(both_ready) + valid_gate = both_ready & valid + a, a_rdy = self.type.wrap(data, valid_gate) + b, b_rdy = self.type.wrap(data, valid_gate) + abuf = a.buffer(clk, rst, 1) + bbuf = b.buffer(clk, rst, 1) + both_ready.assign(a_rdy & b_rdy) + return abuf, bbuf + class BundleSignal(Signal): """Signal for types.Bundle.""" diff --git a/frontends/PyCDE/src/pycde/types.py b/frontends/PyCDE/src/pycde/types.py index 62b26fc7c970..008923717539 100644 --- a/frontends/PyCDE/src/pycde/types.py +++ b/frontends/PyCDE/src/pycde/types.py @@ -598,7 +598,7 @@ def inner(self): return self.inner_type def wrap(self, value, - valueOrEmpty) -> typing.Tuple["ChannelSignal", "BitsSignal"]: + valid_or_empty) -> typing.Tuple["ChannelSignal", "BitsSignal"]: """Wrap a data signal and valid signal into a data channel signal and a ready signal.""" @@ -608,21 +608,75 @@ def wrap(self, value, # one. from .dialects import esi + from .signals import Signal signaling = self.signaling if signaling == ChannelSignaling.ValidReady: - value = self.inner_type(value) - valid = types.i1(valueOrEmpty) + if not isinstance(value, Signal): + value = self.inner_type(value) + elif value.type != self.inner_type: + raise TypeError( + f"Expected signal of type {self.inner_type}, got {value.type}") + valid = Bits(1)(valid_or_empty) wrap_op = esi.WrapValidReadyOp(self._type, types.i1, value.value, valid.value) return wrap_op[0], wrap_op[1] elif signaling == ChannelSignaling.FIFO: value = self.inner_type(value) - empty = types.i1(valueOrEmpty) + empty = Bits(1)(valid_or_empty) wrap_op = esi.WrapFIFOOp(self._type, types.i1, value.value, empty.value) return wrap_op[0], wrap_op[1] else: raise TypeError("Unknown signaling standard") + def _join(self, a: "ChannelSignal", b: "ChannelSignal") -> "ChannelSignal": + """Join two channels into a single channel. The resulting type is a struct + with two fields, 'a' and 'b' wherein 'a' is the data from channel a and 'b' + is the data from channel b.""" + + from .constructs import Wire + both_ready = Wire(Bits(1)) + a_data, a_valid = a.unwrap(both_ready) + b_data, b_valid = b.unwrap(both_ready) + both_valid = a_valid & b_valid + result_data = self.inner_type({"a": a_data, "b": b_data}) + result_chan, result_ready = self.wrap(result_data, both_valid) + both_ready.assign(result_ready & both_valid) + return result_chan + + @staticmethod + def join(a: "ChannelSignal", b: "ChannelSignal") -> "ChannelSignal": + """Join two channels into a single channel. The resulting type is a struct + with two fields, 'a' and 'b' wherein 'a' is the data from channel a and 'b' + is the data from channel b.""" + + from .types import Channel, StructType + return Channel( + StructType([("a", a.type.inner_type), + ("b", b.type.inner_type)]))._join(a, b) + + def merge(self, a: "ChannelSignal", b: "ChannelSignal") -> "ChannelSignal": + """Merge two channels into a single channel, selecting a message from either + one. May implement any sort of fairness policy. Both channels must be of the + same type. Returns both the merged channel.""" + + from .constructs import Mux, Wire + a_ready = Wire(Bits(1)) + b_ready = Wire(Bits(1)) + a_data, a_valid = a.unwrap(a_ready) + b_data, b_valid = b.unwrap(b_ready) + + sel_a = a_valid + sel_b = ~sel_a + out_ready = Wire(Bits(1)) + a_ready.assign(sel_a & out_ready) + b_ready.assign(sel_b & out_ready) + + valid = (sel_a & a_valid) | (sel_b & b_valid) + data = Mux(sel_a, b_data, a_data) + chan, ready = self.wrap(data, valid) + out_ready.assign(ready) + return chan + @dataclass class BundledChannel: diff --git a/frontends/PyCDE/test/test_esi_advanced.py b/frontends/PyCDE/test/test_esi_advanced.py new file mode 100644 index 000000000000..8a5b6e74087d --- /dev/null +++ b/frontends/PyCDE/test/test_esi_advanced.py @@ -0,0 +1,88 @@ +# RUN: %PYTHON% %s | FileCheck %s + +from pycde import generator, Clock, Module, Reset +from pycde.common import InputChannel, OutputChannel +from pycde.testing import unittestmodule +from pycde.types import Bits, UInt + +# CHECK-LABEL: hw.module @Merge(in %clk : !seq.clock, in %rst : i1, in %a : !esi.channel, in %b : !esi.channel, out x : !esi.channel) +# CHECK-NEXT: %rawOutput, %valid = esi.unwrap.vr %a, [[R1:%.+]] : i8 +# CHECK-NEXT: %rawOutput_0, %valid_1 = esi.unwrap.vr %b, [[R2:%.+]] : i8 +# CHECK-NEXT: %true = hw.constant true +# CHECK-NEXT: [[R0:%.+]] = comb.xor bin %valid, %true : i1 +# CHECK-NEXT: [[R1]] = comb.and bin %valid, %ready : i1 +# CHECK-NEXT: [[R2]] = comb.and bin [[R0]], %ready : i1 +# CHECK-NEXT: [[R3:%.+]] = comb.and bin %valid, %valid : i1 +# CHECK-NEXT: [[R4:%.+]] = comb.and bin [[R0]], %valid_1 : i1 +# CHECK-NEXT: [[R5:%.+]] = comb.or bin [[R3]], [[R4]] : i1 +# CHECK-NEXT: [[R6:%.+]] = comb.mux bin %valid, %rawOutput, %rawOutput_0 +# CHECK-NEXT: %chanOutput, %ready = esi.wrap.vr [[R6]], [[R5]] : i8 +# CHECK-NEXT: hw.output %chanOutput : !esi.channel + + +@unittestmodule() +class Merge(Module): + clk = Clock() + rst = Reset() + a = InputChannel(Bits(8)) + b = InputChannel(Bits(8)) + + x = OutputChannel(Bits(8)) + + @generator + def build(ports): + chan = ports.a.type.merge(ports.a, ports.b) + ports.x = chan + + +# CHECK-LABEL: hw.module @Join(in %clk : !seq.clock, in %rst : i1, in %a : !esi.channel, in %b : !esi.channel, out x : !esi.channel) +# CHECK-NEXT: %rawOutput, %valid = esi.unwrap.vr %a, [[R2:%.+]] : ui8 +# CHECK-NEXT: %rawOutput_0, %valid_1 = esi.unwrap.vr %b, [[R2]] : ui8 +# CHECK-NEXT: [[R0:%.+]] = comb.and bin %valid, %valid_1 : i1 +# CHECK-NEXT: [[R1:%.+]] = hw.struct_create (%rawOutput, %rawOutput_0) : !hw.struct +# CHECK-NEXT: %chanOutput, %ready = esi.wrap.vr [[R1]], [[R0]] : !hw.struct +# CHECK-NEXT: [[R2]] = comb.and bin %ready, [[R0]] : i1 +# CHECK-NEXT: %rawOutput_2, %valid_3 = esi.unwrap.vr %chanOutput, %ready_7 : !hw.struct +# CHECK-NEXT: %a_4 = hw.struct_extract %rawOutput_2["a"] : !hw.struct +# CHECK-NEXT: %b_5 = hw.struct_extract %rawOutput_2["b"] : !hw.struct +# CHECK-NEXT: [[R3:%.+]] = hwarith.add %a_4, %b_5 : (ui8, ui8) -> ui9 +# CHECK-NEXT: %chanOutput_6, %ready_7 = esi.wrap.vr [[R3]], %valid_3 : ui9 +# CHECK-NEXT: hw.output %chanOutput_6 : !esi.channel +@unittestmodule(run_passes=True, emit_outputs=True) +class Join(Module): + clk = Clock() + rst = Reset() + a = InputChannel(UInt(8)) + b = InputChannel(UInt(8)) + + x = OutputChannel(UInt(9)) + + @generator + def build(ports): + joined = ports.a.type.join(ports.a, ports.b) + ports.x = joined.transform(lambda x: x.a + x.b) + + +# CHECK-LABEL: hw.module @Fork(in %clk : !seq.clock, in %rst : i1, in %a : !esi.channel, out x : !esi.channel, out y : !esi.channel) +# CHECK-NEXT: %rawOutput, %valid = esi.unwrap.vr %a, [[R3:%.+]] : ui8 +# CHECK-NEXT: [[R0:%.+]] = comb.and bin [[R3]], %valid : i1 +# CHECK-NEXT: %chanOutput, %ready = esi.wrap.vr %rawOutput, [[R0]] : ui8 +# CHECK-NEXT: %chanOutput_0, %ready_1 = esi.wrap.vr %rawOutput, [[R0]] : ui8 +# CHECK-NEXT: [[R1:%.+]] = esi.buffer %clk, %rst, %chanOutput {stages = 1 : i64} : ui8 +# CHECK-NEXT: [[R2:%.+]] = esi.buffer %clk, %rst, %chanOutput_0 {stages = 1 : i64} : ui8 +# CHECK-NEXT: [[R3]] = comb.and bin %ready, %ready_1 : i1 +# CHECK-NEXT: hw.output [[R1]], [[R2]] : !esi.channel, !esi.channel +@unittestmodule(run_passes=True, emit_outputs=True) +class Fork(Module): + clk = Clock() + rst = Reset() + a = InputChannel(UInt(8)) + + x = OutputChannel(UInt(8)) + y = OutputChannel(UInt(8)) + + @generator + def build(ports): + x, y = ports.a.fork(ports.clk, ports.rst) + ports.x = x + ports.y = y From 6f7cba6286df676c84e4e8b2c2d5b2c6281c4bb4 Mon Sep 17 00:00:00 2001 From: Andrew Young Date: Sun, 22 Dec 2024 23:22:59 -0800 Subject: [PATCH 25/27] [Comb] delete slow canonicalizer (#8014) This canonicalizer does not have a functional issue, but is causing bad performance issues. This change removes it until it can be fixed properly. --- lib/Dialect/Comb/CombFolds.cpp | 111 ------------------------ test/Dialect/Comb/canonicalization.mlir | 81 ----------------- 2 files changed, 192 deletions(-) diff --git a/lib/Dialect/Comb/CombFolds.cpp b/lib/Dialect/Comb/CombFolds.cpp index 92aa494dc789..691ae579c9cb 100644 --- a/lib/Dialect/Comb/CombFolds.cpp +++ b/lib/Dialect/Comb/CombFolds.cpp @@ -1178,107 +1178,6 @@ OpFoldResult OrOp::fold(FoldAdaptor adaptor) { return constFoldAssociativeOp(inputs, hw::PEO::Or); } -/// Simplify concat ops in an or op when a constant operand is present in either -/// concat. -/// -/// This will invert an or(concat, concat) into concat(or, or, ...), which can -/// often be further simplified due to the smaller or ops being easier to fold. -/// -/// For example: -/// -/// or(..., concat(x, 0), concat(0, y)) -/// ==> or(..., concat(x, 0, y)), when x and y don't overlap. -/// -/// or(..., concat(x: i2, cst1: i4), concat(cst2: i5, y: i1)) -/// ==> or(..., concat(or(x: i2, extract(cst2, 4..3)), -/// or(extract(cst1, 3..1), extract(cst2, 2..0)), -/// or(extract(cst1, 0..0), y: i1)) -static bool canonicalizeOrOfConcatsWithCstOperands(OrOp op, size_t concatIdx1, - size_t concatIdx2, - PatternRewriter &rewriter) { - assert(concatIdx1 < concatIdx2 && "concatIdx1 must be < concatIdx2"); - - auto inputs = op.getInputs(); - auto concat1 = inputs[concatIdx1].getDefiningOp(); - auto concat2 = inputs[concatIdx2].getDefiningOp(); - - assert(concat1 && concat2 && "expected indexes to point to ConcatOps"); - - // We can simplify as long as a constant is present in either concat. - bool hasConstantOp1 = - llvm::any_of(concat1->getOperands(), [&](Value operand) -> bool { - return operand.getDefiningOp(); - }); - if (!hasConstantOp1) { - bool hasConstantOp2 = - llvm::any_of(concat2->getOperands(), [&](Value operand) -> bool { - return operand.getDefiningOp(); - }); - if (!hasConstantOp2) - return false; - } - - SmallVector newConcatOperands; - - // Simultaneously iterate over the operands of both concat ops, from MSB to - // LSB, pushing out or's of overlapping ranges of the operands. When operands - // span different bit ranges, we extract only the maximum overlap. - auto operands1 = concat1->getOperands(); - auto operands2 = concat2->getOperands(); - // Number of bits already consumed from operands 1 and 2, respectively. - unsigned consumedWidth1 = 0; - unsigned consumedWidth2 = 0; - for (auto it1 = operands1.begin(), end1 = operands1.end(), - it2 = operands2.begin(), end2 = operands2.end(); - it1 != end1 && it2 != end2;) { - auto operand1 = *it1; - auto operand2 = *it2; - - unsigned remainingWidth1 = - hw::getBitWidth(operand1.getType()) - consumedWidth1; - unsigned remainingWidth2 = - hw::getBitWidth(operand2.getType()) - consumedWidth2; - unsigned widthToConsume = std::min(remainingWidth1, remainingWidth2); - auto narrowedType = rewriter.getIntegerType(widthToConsume); - - auto extract1 = rewriter.createOrFold( - op.getLoc(), narrowedType, operand1, remainingWidth1 - widthToConsume); - auto extract2 = rewriter.createOrFold( - op.getLoc(), narrowedType, operand2, remainingWidth2 - widthToConsume); - - newConcatOperands.push_back( - rewriter.createOrFold(op.getLoc(), extract1, extract2, false)); - - consumedWidth1 += widthToConsume; - consumedWidth2 += widthToConsume; - - if (widthToConsume == remainingWidth1) { - ++it1; - consumedWidth1 = 0; - } - if (widthToConsume == remainingWidth2) { - ++it2; - consumedWidth2 = 0; - } - } - - ConcatOp newOp = rewriter.create(op.getLoc(), newConcatOperands); - - // Copy the old operands except for concatIdx1 and concatIdx2, and append the - // new ConcatOp to the end. - SmallVector newOrOperands; - newOrOperands.append(inputs.begin(), inputs.begin() + concatIdx1); - newOrOperands.append(inputs.begin() + concatIdx1 + 1, - inputs.begin() + concatIdx2); - newOrOperands.append(inputs.begin() + concatIdx2 + 1, - inputs.begin() + inputs.size()); - newOrOperands.push_back(newOp); - - replaceOpWithNewOpAndCopyName(rewriter, op, op.getType(), - newOrOperands); - return true; -} - LogicalResult OrOp::canonicalize(OrOp op, PatternRewriter &rewriter) { auto inputs = op.getInputs(); auto size = inputs.size(); @@ -1328,16 +1227,6 @@ LogicalResult OrOp::canonicalize(OrOp op, PatternRewriter &rewriter) { } } - // or(..., concat(x, cst1), concat(cst2, y) - // ==> or(..., concat(x, cst3, y)), when x and y don't overlap. - for (size_t i = 0; i < size - 1; ++i) { - if (auto concat = inputs[i].getDefiningOp()) - for (size_t j = i + 1; j < size; ++j) - if (auto concat = inputs[j].getDefiningOp()) - if (canonicalizeOrOfConcatsWithCstOperands(op, i, j, rewriter)) - return success(); - } - // extracts only of or(...) -> or(extract()...) if (narrowOperationWidth(op, true, rewriter)) return success(); diff --git a/test/Dialect/Comb/canonicalization.mlir b/test/Dialect/Comb/canonicalization.mlir index 8c1a3f4345dc..e7cda4bd24c4 100644 --- a/test/Dialect/Comb/canonicalization.mlir +++ b/test/Dialect/Comb/canonicalization.mlir @@ -181,87 +181,6 @@ hw.module @dedupLong(in %arg0 : i7, in %arg1 : i7, in %arg2: i7, out resAnd: i7, hw.output %0, %1 : i7, i7 } -// CHECK-LABEL: hw.module @orExclusiveConcats -hw.module @orExclusiveConcats(in %arg0 : i6, in %arg1 : i2, out o: i9) { - // CHECK-NEXT: %false = hw.constant false - // CHECK-NEXT: %0 = comb.concat %arg1, %false, %arg0 : i2, i1, i6 - // CHECK-NEXT: hw.output %0 : i9 - %c0 = hw.constant 0 : i3 - %0 = comb.concat %c0, %arg0 : i3, i6 - %c1 = hw.constant 0 : i7 - %1 = comb.concat %arg1, %c1 : i2, i7 - %2 = comb.or %0, %1 : i9 - hw.output %2 : i9 -} - -// When two concats are or'd together and have mutually-exclusive fields, they -// can be merged together into a single concat. -// concat0: 0aaa aaa0 0000 0bb0 -// concat1: 0000 0000 ccdd d000 -// merged: 0aaa aaa0 ccdd dbb0 -// CHECK-LABEL: hw.module @orExclusiveConcats2 -hw.module @orExclusiveConcats2(in %arg0 : i6, in %arg1 : i2, in %arg2: i2, in %arg3: i3, out o: i16) { - // CHECK-NEXT: %false = hw.constant false - // CHECK-NEXT: %0 = comb.concat %false, %arg0, %false, %arg2, %arg3, %arg1, %false : i1, i6, i1, i2, i3, i2, i1 - // CHECK-NEXT: hw.output %0 : i16 - %c0 = hw.constant 0 : i1 - %c1 = hw.constant 0 : i6 - %c2 = hw.constant 0 : i1 - %0 = comb.concat %c0, %arg0, %c1, %arg1, %c2: i1, i6, i6, i2, i1 - %c3 = hw.constant 0 : i8 - %c4 = hw.constant 0 : i3 - %1 = comb.concat %c3, %arg2, %arg3, %c4 : i8, i2, i3, i3 - %2 = comb.or %0, %1 : i16 - hw.output %2 : i16 -} - -// When two concats are or'd together and have mutually-exclusive fields, they -// can be merged together into a single concat. -// concat0: aaaa 1111 -// concat1: 1111 10bb -// merged: 1111 1111 -// CHECK-LABEL: hw.module @orExclusiveConcats3 -hw.module @orExclusiveConcats3(in %arg0 : i4, in %arg1 : i2, out o: i8) { - // CHECK-NEXT: [[RES:%[a-z0-9_-]+]] = hw.constant -1 : i8 - // CHECK-NEXT: hw.output [[RES]] : i8 - %c0 = hw.constant -1 : i4 - %0 = comb.concat %arg0, %c0: i4, i4 - %c1 = hw.constant -1 : i5 - %c2 = hw.constant 0 : i1 - %1 = comb.concat %c1, %c2, %arg1 : i5, i1, i2 - %2 = comb.or %0, %1 : i8 - hw.output %2 : i8 -} - -// CHECK-LABEL: hw.module @orMultipleExclusiveConcats -hw.module @orMultipleExclusiveConcats(in %arg0 : i2, in %arg1 : i2, in %arg2: i2, out o: i6) { - // CHECK-NEXT: %0 = comb.concat %arg0, %arg1, %arg2 : i2, i2, i2 - // CHECK-NEXT: hw.output %0 : i6 - %c2 = hw.constant 0 : i2 - %c4 = hw.constant 0 : i4 - %0 = comb.concat %arg0, %c4: i2, i4 - %1 = comb.concat %c2, %arg1, %c2: i2, i2, i2 - %2 = comb.concat %c4, %arg2: i4, i2 - %out = comb.or %0, %1, %2 : i6 - hw.output %out : i6 -} - -// CHECK-LABEL: hw.module @orConcatsWithMux -hw.module @orConcatsWithMux(in %bit: i1, in %cond: i1, out o: i6) { - // CHECK-NEXT: [[RES:%[a-z0-9_-]+]] = hw.constant 0 : i4 - // CHECK-NEXT: %0 = comb.concat [[RES]], %cond, %bit : i4, i1, i1 - // CHECK-NEXT: hw.output %0 : i6 - %c0 = hw.constant 0 : i5 - %0 = comb.concat %c0, %bit: i5, i1 - %c1 = hw.constant 0 : i4 - %c2 = hw.constant 2 : i2 - %c3 = hw.constant 0 : i2 - %1 = comb.mux %cond, %c2, %c3 : i2 - %2 = comb.concat %c1, %1 : i4, i2 - %3 = comb.or %0, %2 : i6 - hw.output %3 : i6 -} - // CHECK-LABEL: @extractNested hw.module @extractNested(in %0: i5, out o1 : i1) { // Multiple layers of nested extract is a weak evidence that the cannonicalization From 550a9a73b01e441b80012c423b3f9846e62f4490 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 26 Dec 2024 12:18:34 -0500 Subject: [PATCH 26/27] [FIRRTL] Drop unused argument, NFC Drop an unused argument in a static function in the Grand Central pass. Signed-off-by: Schuyler Eldridge --- .../FIRRTL/Transforms/GrandCentral.cpp | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp index fdd4853ac5f7..be074cc6fdb9 100644 --- a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp +++ b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp @@ -755,8 +755,8 @@ struct GrandCentralPass /// 2) Scattered annotations for how components bind to interfaces static std::optional parseAugmentedType(ApplyState &state, DictionaryAttr augmentedType, - DictionaryAttr root, StringRef companion, StringAttr name, - StringAttr defName, std::optional id, + DictionaryAttr root, StringAttr name, StringAttr defName, + std::optional id, std::optional description, Twine clazz, StringAttr companionAttr, Twine path = {}) { @@ -932,8 +932,8 @@ parseAugmentedType(ApplyState &state, DictionaryAttr augmentedType, if (auto maybeDescription = field.get("description")) description = cast(maybeDescription); auto eltAttr = parseAugmentedType( - state, tpe, root, companion, name, defName, std::nullopt, description, - clazz, companionAttr, path + "_" + name.getValue()); + state, tpe, root, name, defName, std::nullopt, description, clazz, + companionAttr, path + "_" + name.getValue()); if (!eltAttr) return std::nullopt; @@ -1115,10 +1115,10 @@ parseAugmentedType(ApplyState &state, DictionaryAttr augmentedType, return std::nullopt; SmallVector elements; for (auto [i, elt] : llvm::enumerate(elementsAttr)) { - auto eltAttr = parseAugmentedType( - state, cast(elt), root, companion, name, - StringAttr::get(context, ""), id, std::nullopt, clazz, companionAttr, - path + "_" + Twine(i)); + auto eltAttr = + parseAugmentedType(state, cast(elt), root, name, + StringAttr::get(context, ""), id, std::nullopt, + clazz, companionAttr, path + "_" + Twine(i)); if (!eltAttr) return std::nullopt; elements.push_back(*eltAttr); @@ -1167,8 +1167,8 @@ LogicalResult circt::firrtl::applyGCTView(const AnnoPathValue &target, state.addToWorklistFn(DictionaryAttr::get(context, companionAttrs)); auto prunedAttr = - parseAugmentedType(state, viewAttr, anno, companionAttr.getValue(), name, - {}, id, {}, viewAnnoClass, companionAttr, Twine(name)); + parseAugmentedType(state, viewAttr, anno, name, {}, id, {}, viewAnnoClass, + companionAttr, Twine(name)); if (!prunedAttr) return failure(); From 5b128a1910477bce04367b2ce5e8be54eaf0199a Mon Sep 17 00:00:00 2001 From: Hideto Ueno Date: Fri, 27 Dec 2024 14:02:30 +0900 Subject: [PATCH 27/27] [CombToAIG] Add a pattern for mul (#8015) This commit adds a pattern to lower mul op. The pattern lowers mul op into chains of comb.add + comb.mux. There must be more efficient implementation but for now this naive pattern should work fine. LEC is verified. ``` {a_{n}, a_{n-1}, ..., a_0} * b = sum_{i=0}^{n} a_i * 2^i * b = sum_{i=0}^{n} (a_i ? b : 0) << i ``` --- .../circt-synth/comb-lowering-lec.mlir | 7 +++ lib/Conversion/CombToAIG/CombToAIG.cpp | 51 +++++++++++++++++-- .../CombToAIG/comb-to-aig-arith.mlir | 19 +++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/integration_test/circt-synth/comb-lowering-lec.mlir b/integration_test/circt-synth/comb-lowering-lec.mlir index 87d2d497aaa0..9db9667c7be9 100644 --- a/integration_test/circt-synth/comb-lowering-lec.mlir +++ b/integration_test/circt-synth/comb-lowering-lec.mlir @@ -27,3 +27,10 @@ hw.module @sub(in %lhs: i4, in %rhs: i4, out out: i4) { %0 = comb.sub %lhs, %rhs : i4 hw.output %0 : i4 } + +// RUN: circt-lec %t.mlir %s -c1=mul -c2=mul --shared-libs=%libz3 | FileCheck %s --check-prefix=COMB_MUL +// COMB_MUL: c1 == c2 +hw.module @mul(in %arg0: i3, in %arg1: i3, in %arg2: i3, out add: i3) { + %0 = comb.mul %arg0, %arg1, %arg2 : i3 + hw.output %0 : i3 +} diff --git a/lib/Conversion/CombToAIG/CombToAIG.cpp b/lib/Conversion/CombToAIG/CombToAIG.cpp index d9438ed1082f..f7613baa5aac 100644 --- a/lib/Conversion/CombToAIG/CombToAIG.cpp +++ b/lib/Conversion/CombToAIG/CombToAIG.cpp @@ -283,6 +283,51 @@ struct CombSubOpConversion : OpConversionPattern { } }; +struct CombMulOpConversion : OpConversionPattern { + using OpConversionPattern::OpConversionPattern; + using OpAdaptor = typename OpConversionPattern::OpAdaptor; + LogicalResult + matchAndRewrite(MulOp op, OpAdaptor adaptor, + ConversionPatternRewriter &rewriter) const override { + if (adaptor.getInputs().size() != 2) + return failure(); + + // FIXME: Currently it's lowered to a really naive implementation that + // chains add operations. + + // a_{n}a_{n-1}...a_0 * b + // = sum_{i=0}^{n} a_i * 2^i * b + // = sum_{i=0}^{n} (a_i ? b : 0) << i + int64_t width = op.getType().getIntOrFloatBitWidth(); + auto aBits = extractBits(rewriter, adaptor.getInputs()[0]); + SmallVector results; + auto rhs = op.getInputs()[1]; + auto zero = rewriter.create(op.getLoc(), + llvm::APInt::getZero(width)); + for (int64_t i = 0; i < width; ++i) { + auto aBit = aBits[i]; + auto andBit = + rewriter.createOrFold(op.getLoc(), aBit, rhs, zero); + auto upperBits = rewriter.createOrFold( + op.getLoc(), andBit, 0, width - i); + if (i == 0) { + results.push_back(upperBits); + continue; + } + + auto lowerBits = + rewriter.create(op.getLoc(), APInt::getZero(i)); + + auto shifted = rewriter.createOrFold( + op.getLoc(), op.getType(), ValueRange{upperBits, lowerBits}); + results.push_back(shifted); + } + + rewriter.replaceOpWithNewOp(op, results, true); + return success(); + } +}; + } // namespace //===----------------------------------------------------------------------===// @@ -304,10 +349,10 @@ static void populateCombToAIGConversionPatterns(RewritePatternSet &patterns) { CombAndOpConversion, CombOrOpConversion, CombXorOpConversion, CombMuxOpConversion, // Arithmetic Ops - CombAddOpConversion, CombSubOpConversion, + CombAddOpConversion, CombSubOpConversion, CombMulOpConversion, // Variadic ops that must be lowered to binary operations - CombLowerVariadicOp, CombLowerVariadicOp>( - patterns.getContext()); + CombLowerVariadicOp, CombLowerVariadicOp, + CombLowerVariadicOp>(patterns.getContext()); } void ConvertCombToAIGPass::runOnOperation() { diff --git a/test/Conversion/CombToAIG/comb-to-aig-arith.mlir b/test/Conversion/CombToAIG/comb-to-aig-arith.mlir index 1782dc3f216e..59d956e953a0 100644 --- a/test/Conversion/CombToAIG/comb-to-aig-arith.mlir +++ b/test/Conversion/CombToAIG/comb-to-aig-arith.mlir @@ -27,3 +27,22 @@ hw.module @sub(in %lhs: i4, in %rhs: i4, out out: i4) { %0 = comb.sub %lhs, %rhs : i4 hw.output %0 : i4 } + + +// CHECK-LABEL: @mul +// ALLOW_ADD-LABEL: @mul +// ALLOW_ADD-NEXT: %[[EXT_0:.+]] = comb.extract %lhs from 0 : (i2) -> i1 +// ALLOW_ADD-NEXT: %[[EXT_1:.+]] = comb.extract %lhs from 1 : (i2) -> i1 +// ALLOW_ADD-NEXT: %c0_i2 = hw.constant 0 : i2 +// ALLOW_ADD-NEXT: %[[MUX_0:.+]] = comb.mux %0, %rhs, %c0_i2 : i2 +// ALLOW_ADD-NEXT: %[[MUX_1:.+]] = comb.mux %1, %rhs, %c0_i2 : i2 +// ALLOW_ADD-NEXT: %[[EXT_MUX_1:.+]] = comb.extract %3 from 0 : (i2) -> i1 +// ALLOW_ADD-NEXT: %false = hw.constant false +// ALLOW_ADD-NEXT: %[[SHIFT:.+]] = comb.concat %4, %false : i1, i1 +// ALLOW_ADD-NEXT: %[[ADD:.+]] = comb.add bin %[[MUX_0]], %[[SHIFT]] : i2 +// ALLOW_ADD-NEXT: hw.output %[[ADD]] : i2 +// ALLOW_ADD-NEXT: } +hw.module @mul(in %lhs: i2, in %rhs: i2, out out: i2) { + %0 = comb.mul %lhs, %rhs : i2 + hw.output %0 : i2 +}