Change ESI wrapper ports

compiler/cpp/circt_util.h

@@ -286,7 +286,7 @@ class ModuleDeclarationHelper
 
     mlir::Block* GetBodyBlock();
 
-    void BeginEsiBundle(const std::string& name);
+    void BeginEsiBundle(const std::string& name, bool isOutputBundle = false);
 
     void EndEsiBundle();
 
@@ -380,10 +380,18 @@ class ModuleDeclarationHelper
     StringSourceWriter _verbatimBuffer;
 
     std::optional<std::string> _bundleName;
+    bool _bundleIsOutput = false;
     size_t _bundleStartPortIndex;
 
+    struct EsiBundleInfo
+    {
+        size_t startPortIndex;
+        size_t endPortIndex;
+        bool isOutputBundle;
+    };
+
     // Maps bundle name to range of relevant ports
-    std::map<std::string, std::pair<size_t, size_t>> _bundleNameToPortRange;
+    std::map<std::string, EsiBundleInfo> _bundleNameToPortRange;
 
     std::map<std::string, size_t> _portNameToIndex;
     std::map<std::string, mlir::Value> _outputValues;
@@ -406,13 +414,15 @@ class ModuleDeclarationHelper
 class PushPopEsiBundle
 {
   public:
-    PushPopEsiBundle(ModuleDeclarationHelper& helper, const std::optional<std::string>& name) : _helper(helper)
+    PushPopEsiBundle(ModuleDeclarationHelper& helper, const std::optional<std::string>& name,
+                     bool isOutputBundle = false)
+        : _helper(helper)
     {
         if (name)
         {
             _pushedName = true;
 
-            helper.BeginEsiBundle(*name);
+            helper.BeginEsiBundle(*name, isOutputBundle);
         }
         else
         {

compiler/cpp/verilog.cpp

@@ -495,7 +495,8 @@ std::string GetRegisterBaseName(const Program &program, const size_t registerInd
     return prefix + std::to_string(registerIndex) + "_" + regDesc._name;
 }
 
-std::string GetBasicBlockInstanceName(const BasicBlock &basicBlock) {
+std::string GetBasicBlockInstanceName(const BasicBlock &basicBlock)
+{
     return g_compiler->ClampStringLength(GetBasicBlockName(basicBlock) + "Impl");
 }
 
@@ -6481,7 +6482,11 @@ class VerilogCompiler
 
             const bool fixedLatency = functionNode->IsFixedLatency();
 
-            if (hasBackpressure)
+            // Bundle when ESI signaling can describe both directions:
+            //  * hasBackpressure   -> ValidReady (args) + FIFO (results)
+            //  * !fixedLatency     -> ValidOnly on either direction
+            // Fixed-latency results have no Valid signal at all and stay raw.
+            if (hasBackpressure || !fixedLatency)
             {
                 exportInterface._esiBundleName = combinedFunctionName;
             }
@@ -6604,14 +6609,9 @@ class VerilogCompiler
 
             const bool isNoBackpressure = functionNode->GetModifiers() & ParseTreeFunctionModifierNoBackPressure;
 
-            std::optional<std::string> esiBundleName;
-
-            if (!isNoBackpressure)
-            {
-                esiBundleName = prefix;
-            }
+            std::optional<std::string> esiBundleName = prefix;
 
-            PushPopEsiBundle pushPopEsiBundle(coreModule, esiBundleName);
+            PushPopEsiBundle pushPopEsiBundle(coreModule, esiBundleName, /*isOutputBundle=*/true);
 
             if (isNoBackpressure)
             {

test/interface/CMakeLists.txt

@@ -187,3 +187,5 @@ add_interface_test(ecc_mem ecc_mem.k
   TESTBENCH
     ecc_mem.sv
 )
+
+add_subdirectory(circt)

test/interface/circt/CMakeLists.txt

@@ -0,0 +1,72 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+
+# Tests that compile a Kanagawa source through the front-/middle-end with
+# --skip-circt-lowering and run a python script to inspect the produced
+# CIRCT MLIR. Useful for asserting on the shape of the EsiWrapper interface.
+function(add_circt_mlir_test test_name)
+  set(_one OUTPUT_PREFIX)
+  set(_multi SOURCES OPTIONS TEST)
+  cmake_parse_arguments(_ARG "" "${_one}" "${_multi}" ${ARGN})
+
+  if(NOT _ARG_OUTPUT_PREFIX)
+    set(_ARG_OUTPUT_PREFIX "test")
+  endif()
+
+  set(_outdir "${CMAKE_CURRENT_BINARY_DIR}/${test_name}")
+  set(_fixture "interface_circt_${test_name}")
+
+  add_test(
+    NAME interface.circt.${test_name}.prepare
+    COMMAND ${CMAKE_COMMAND} -E rm -rf "${_outdir}"
+  )
+  set_tests_properties(interface.circt.${test_name}.prepare PROPERTIES
+    FIXTURES_SETUP "${_fixture}_prepared"
+  )
+
+  add_test(
+    NAME interface.circt.${test_name}.mkdir
+    COMMAND ${CMAKE_COMMAND} -E make_directory "${_outdir}"
+  )
+  set_tests_properties(interface.circt.${test_name}.mkdir PROPERTIES
+    FIXTURES_REQUIRED "${_fixture}_prepared"
+    FIXTURES_SETUP    "${_fixture}_dir"
+  )
+
+  add_test(
+    NAME interface.circt.${test_name}.compile
+    COMMAND $<TARGET_FILE:kanagawa::exe>
+              ${_ARG_OPTIONS}
+              --output=${_outdir}/${_ARG_OUTPUT_PREFIX}
+              ${_ARG_SOURCES}
+  )
+  set_tests_properties(interface.circt.${test_name}.compile PROPERTIES
+    FIXTURES_REQUIRED "${_fixture}_dir"
+    FIXTURES_SETUP    "${_fixture}_compiled"
+  )
+
+  set(_idx 0)
+  foreach(_cmd IN LISTS _ARG_TEST)
+    math(EXPR _idx "${_idx} + 1")
+    separate_arguments(_cmd_list UNIX_COMMAND "${_cmd}")
+    add_test(
+      NAME interface.circt.${test_name}.test${_idx}
+      COMMAND ${_cmd_list}
+    )
+    set_tests_properties(interface.circt.${test_name}.test${_idx} PROPERTIES
+      FIXTURES_REQUIRED "${_fixture}_compiled"
+    )
+  endforeach()
+endfunction()
+
+add_circt_mlir_test(esi_wrapper_ports
+  SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/esi_wrapper_ports.k
+  OPTIONS
+    --backend=sv
+    --skip-circt-lowering
+    --base-library=${CMAKE_SOURCE_DIR}/library/base.k
+    --import-dir=${CMAKE_SOURCE_DIR}/library
+    --place-iterations=1
+  TEST
+    "python3 ${CMAKE_CURRENT_SOURCE_DIR}/check_esi_wrapper_ports.py ${CMAKE_CURRENT_BINARY_DIR}/esi_wrapper_ports"
+)

test/interface/circt/check_esi_wrapper_ports.py

@@ -0,0 +1,133 @@
+#!/usr/bin/env python3
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+"""
+Verify the EsiWrapper port shapes for the mixed export/callback class
+defined in `esi_wrapper_ports.k`. Each combination of {export, callback}
+x {regular, async, no_backpressure} must lower to a specific kind of
+port (input/output, bundle/bare channel, signaling protocol).
+
+Exits non-zero on failure.
+"""
+import argparse
+import re
+import sys
+from pathlib import Path
+
+
+# TODO: this really should use the Python CIRCT API to parse the IR and check
+# the port types, but that would require having the Python API available which
+# we don't currently do. Would adding this feature be worth it?
+
+# (port name, list of substrings that must all appear on the port's
+# declaration line in the EsiWrapper container).
+EXPECTED_PORTS = [
+    # Regular export -> input bundle, args FIFO, results ValidReady.
+    ("RegExport", [
+        'kanagawa.port.input "RegExport" sym @RegExport',
+        '!esi.bundle<[',
+        ', FIFO> from "result"',  # results channel uses FIFO (rden/empty pull)
+        ' to "arg"',              # args channel ValidReady (default)
+    ]),
+
+    # [[async]] export -> bare input channel.
+    ("AsyncExport", [
+        'kanagawa.port.input "AsyncExport" sym @AsyncExport',
+        '!esi.channel<',
+    ]),
+
+    # [[no_backpressure]] export -> input bundle with ValidOnly channels.
+    ("NbpExport", [
+        'kanagawa.port.input "NbpExport" sym @NbpExport',
+        '!esi.bundle<[',
+        ', ValidOnly> from "result"',
+        ', ValidOnly> to "arg"',
+    ]),
+
+    # Regular callback -> output bundle, args FIFO, results ValidReady.
+    ("reg_cb", [
+        'kanagawa.port.output "reg_cb" sym @reg_cb',
+        '!esi.bundle<[',
+        ', FIFO> to "arg"',
+        ' from "result"',
+    ]),
+
+    # [[async]] callback -> bare output channel.
+    ("async_cb", [
+        'kanagawa.port.output "async_cb" sym @async_cb',
+        '!esi.channel<',
+        ', FIFO>',
+    ]),
+
+    # [[no_backpressure]] [[async]] callback -> bare ValidOnly output channel.
+    ("nbp_cb", [
+        'kanagawa.port.output "nbp_cb" sym @nbp_cb',
+        '!esi.channel<',
+        ', ValidOnly>',
+    ]),
+]
+
+# Forbid the no-longer-emitted shapes so silent regressions don't slip past.
+FORBIDDEN_LINES = [
+    re.compile(r'kanagawa\.port\.input\s+"\w*_cb"'),
+    re.compile(r'"AsyncExport"[^\n]*!esi\.bundle<'),
+    re.compile(r'"async_cb"[^\n]*!esi\.bundle<'),
+    re.compile(r'"nbp_cb"[^\n]*!esi\.bundle<'),
+]
+
+
+def find_port_line(mlir_text, port_name):
+    pattern = re.compile(
+        rf'^[^\n]*"{re.escape(port_name)}"\s+sym\s+@{re.escape(port_name)}[^\n]*$',
+        re.MULTILINE)
+    match = pattern.search(mlir_text)
+    return match.group(0) if match else None
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument('output_dir', help='Directory containing compiler outputs')
+    args = parser.parse_args()
+
+    output_dir = Path(args.output_dir)
+    if not output_dir.is_dir():
+        print(f"Output directory does not exist: {output_dir}", file=sys.stderr)
+        return 1
+
+    mlir_files = sorted(output_dir.glob('*.mlir'))
+    if not mlir_files:
+        print("No .mlir output found.", file=sys.stderr)
+        return 1
+
+    mlir_text = mlir_files[0].read_text()
+
+    failures = []
+    for name, required in EXPECTED_PORTS:
+        line = find_port_line(mlir_text, name)
+        if line is None:
+            failures.append((name, 'no declaration line found', None))
+            continue
+        for needle in required:
+            if needle not in line:
+                failures.append((name, f'missing substring {needle!r}', line))
+
+    for forbidden in FORBIDDEN_LINES:
+        match = forbidden.search(mlir_text)
+        if match:
+            failures.append(('<forbidden>',
+                             f'pattern /{forbidden.pattern}/ matched',
+                             match.group(0)))
+
+    if failures:
+        print(f"EsiWrapper port-shape mismatch in {mlir_files[0].name}:", file=sys.stderr)
+        for name, reason, line in failures:
+            print(f"  {name}: {reason}", file=sys.stderr)
+            if line is not None:
+                print(f"    {line.strip()}", file=sys.stderr)
+        return 1
+
+    return 0
+
+
+if __name__ == '__main__':
+    sys.exit(main())

test/interface/circt/esi_wrapper_ports.k

@@ -0,0 +1,44 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+// Source for the cli.esi_wrapper_ports test. The class below mixes the six
+// combinations of ESI wrapper port shapes that EsiWrapper now produces:
+//
+//   regular export      -> input  bundle (FIFO + ValidReady channels)
+//   [[async]] export    -> input  bare channel (FIFO)
+//   no_backpressure exp -> input  bundle (ValidOnly channels)
+//   regular callback    -> output bundle (FIFO + ValidReady channels)
+//   [[async]] callback  -> output bare channel (FIFO)
+//   no_backpressure cb  -> output bare channel (ValidOnly)
+//
+// `check_esi_wrapper_ports.py` greps the emitted CIRCT MLIR for each
+// expected port declaration.
+
+class Mixed
+{
+private:
+    (uint32)->uint32 reg_cb;
+
+    [[async]] (uint32)->void async_cb;
+
+    [[no_backpressure]] [[async]] (uint32)->void nbp_cb;
+
+public:
+    uint32 RegExport(uint32 x)
+    {
+        return reg_cb(x);
+    }
+
+    [[async]] void AsyncExport(uint32 x)
+    {
+        async_cb(x);
+    }
+
+    [[no_backpressure]] uint32 NbpExport(uint32 x)
+    {
+        nbp_cb(static_cast(x + 1));
+        return static_cast(x + 4);
+    }
+}
+
+export Mixed;