[ESI] FIFO: support valid/ready on inputs and outputs (#8009)

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<i32, FIFO>, out out: !esi.channel<i32, FIFO(2)>) {
+  hw.module @fifo1(
+      in %clk: !seq.clock, in %rst: i1,
+      in %in: !esi.channel<i32, FIFO>,
+      out out: !esi.channel<i32, FIFO(2)>) {
     %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel<i32, FIFO> -> !esi.channel<i32, FIFO(2)>
     hw.output %fifo : !esi.channel<i32, FIFO(2)>
   }
+
+  hw.module @fifoValidReadyInput(
+      in %clk: !seq.clock, in %rst: i1,
+      in %in: !esi.channel<i32, ValidReady>,
+      out out: !esi.channel<i32, FIFO(2)>) {
+    %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel<i32, ValidReady> -> !esi.channel<i32, FIFO(2)>
+    hw.output %fifo : !esi.channel<i32, FIFO(2)>
+  }
+
+  hw.module @fifoValidReadyOutput(
+      in %clk: !seq.clock, in %rst: i1,
+      in %in: !esi.channel<i32, FIFO>,
+      out out: !esi.channel<i32, ValidReady>) {
+    %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel<i32, FIFO> -> !esi.channel<i32, ValidReady>
+    hw.output %fifo : !esi.channel<i32, ValidReady>
+  }
+
+
+  hw.module @top(in %clk: !seq.clock, in %rst: i1,
+      in %fifo1_in: !esi.channel<i32, FIFO>, out fifo1_out: !esi.channel<i32, FIFO(2)>,
+      in %fifoValidReadyInput_in: !esi.channel<i32, ValidReady>, out fifoValidReadyInput_out: !esi.channel<i32, FIFO(2)>,
+      in %fifoValidReadyOutput_in: !esi.channel<i32, FIFO>, out fifoValidReadyOutput_out: !esi.channel<i32, ValidReady>
+  ) {
+    %fifo1 = hw.instance "fifo1" @fifo1(
+      clk: %clk: !seq.clock, rst: %rst: i1, in: %fifo1_in: !esi.channel<i32, FIFO>) -> (out: !esi.channel<i32, FIFO(2)>)
+    %fifoValidReadyInput = hw.instance "fifoValidReadyInput" @fifoValidReadyInput(
+      clk: %clk: !seq.clock, rst: %rst: i1, in: %fifoValidReadyInput_in: !esi.channel<i32, ValidReady>) -> (out: !esi.channel<i32, FIFO(2)>)
+    %fifoValidReadyOutput = hw.instance "fifoValidReadyOutput" @fifoValidReadyOutput(
+      clk: %clk: !seq.clock, rst: %rst: i1, in: %fifoValidReadyOutput_in: !esi.channel<i32, FIFO>) -> (out: !esi.channel<i32, ValidReady>)
+    hw.output %fifo1, %fifoValidReadyInput, %fifoValidReadyOutput : !esi.channel<i32, FIFO(2)>, !esi.channel<i32, FIFO(2)>, !esi.channel<i32, ValidReady>
+  }
 }

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)

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();
 }

lib/Dialect/ESI/Passes/ESILowerPhysical.cpp

@@ -103,21 +103,38 @@ FIFOLowering::matchAndRewrite(FIFOOp op, OpAdaptor adaptor,
   auto i1 = rewriter.getI1Type();
   auto c1 = rewriter.create<hw::ConstantOp>(loc, rewriter.getI1Type(),
                                             rewriter.getBoolAttr(true));
-  if (op.getInput().getType().getDataDelay() != 0)
+  mlir::TypedValue<ChannelType> 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<UnwrapFIFOOp>(loc, op.getInput(), inputEn);
+  Value rawData;
+  Value dataNotAvailable;
+  if (chanInput.getType().getSignaling() == ChannelSignaling::ValidReady) {
+    auto unwrapValidReady =
+        rewriter.create<UnwrapValidReadyOp>(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<UnwrapFIFOOp>(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<seq::FIFOOp>(
-      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<comb::XorOp>(loc, unwrapPull.getEmpty(), c1);
+  auto inputNotEmpty = rewriter.create<comb::XorOp>(loc, dataNotAvailable, c1);
   inputNotEmpty->setAttr("sv.namehint",
                          rewriter.getStringAttr("inputNotEmpty"));
   auto seqFifoNotFull =
@@ -129,12 +146,29 @@ FIFOLowering::matchAndRewrite(FIFOOp op, OpAdaptor adaptor,
   static_cast<Value>(inputEn).getDefiningOp()->setAttr(
       "sv.namehint", rewriter.getStringAttr("inputEn"));
 
-  auto output =
-      rewriter.create<WrapFIFOOp>(loc, mlir::TypeRange{outputType, i1},
-                                  seqFifo.getOutput(), seqFifo.getEmpty());
-  outputRdEn.setValue(output.getRden());
+  Value output;
+  if (outputType.getSignaling() == ChannelSignaling::ValidReady) {
+    auto wrap = rewriter.create<WrapValidReadyOp>(
+        loc, mlir::TypeRange{outputType, i1}, seqFifo.getOutput(),
+        comb::createOrFoldNot(loc, seqFifo.getEmpty(), rewriter,
+                              /*twoState=*/true));
+    output = wrap.getChanOutput();
+    outputRdEn.setValue(
+        rewriter.create<comb::AndOp>(loc, wrap.getValid(), wrap.getReady()));
+    static_cast<Value>(outputRdEn)
+        .getDefiningOp()
+        ->setAttr("sv.namehint", rewriter.getStringAttr("outputRdEn"));
+  } else if (outputType.getSignaling() == ChannelSignaling::FIFO) {
+    auto wrap =
+        rewriter.create<WrapFIFOOp>(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();
 }
 

test/Dialect/ESI/lowering.mlir

@@ -211,3 +211,42 @@ hw.module @fifo1(in %clk: !seq.clock, in %rst: i1, in %in: !esi.channel<i32, FIF
   %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel<i32, FIFO> -> !esi.channel<i32, FIFO(2)>
   hw.output %fifo : !esi.channel<i32, FIFO(2)>
 }
+
+// CHECK-LABEL:  hw.module @fifoValidReadyInput(in %clk : !seq.clock, in %rst : i1, in %in : !esi.channel<i32>, out out : !esi.channel<i32, FIFO(2)>) {
+// 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<i32, FIFO(2)>
+// CHECK-NEXT:     hw.output %chanOutput : !esi.channel<i32, FIFO(2)>
+hw.module @fifoValidReadyInput(
+    in %clk: !seq.clock, in %rst: i1,
+    in %in: !esi.channel<i32, ValidReady>,
+    out out: !esi.channel<i32, FIFO(2)>) {
+  %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel<i32, ValidReady> -> !esi.channel<i32, FIFO(2)>
+  hw.output %fifo : !esi.channel<i32, FIFO(2)>
+}
+
+// CHECK-LABEL:  hw.module @fifoValidReadyOutput(in %clk : !seq.clock, in %rst : i1, in %in : !esi.channel<i32, FIFO>, out out : !esi.channel<i32>) {
+// CHECK-NEXT:     %true = hw.constant true
+// CHECK-NEXT:     %data, %empty = esi.unwrap.fifo %in, [[InputEN:%.+]] : !esi.channel<i32, FIFO>
+// 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<i32>
+hw.module @fifoValidReadyOutput(
+    in %clk: !seq.clock, in %rst: i1,
+    in %in: !esi.channel<i32, FIFO>,
+    out out: !esi.channel<i32, ValidReady>) {
+  %fifo = esi.fifo in %in clk %clk rst %rst depth 12 : !esi.channel<i32, FIFO> -> !esi.channel<i32, ValidReady>
+  hw.output %fifo : !esi.channel<i32, ValidReady>
+}