From 06d37d404f2cb34a44cc2ec81af2385b1a0c46bf Mon Sep 17 00:00:00 2001 From: John Demme Date: Wed, 13 May 2026 19:53:10 +0000 Subject: [PATCH 1/5] EsiWrapper: emit callback bundles as outputs Bundle ports for external functions and export-class callbacks are now declared as kanagawa.port.output instead of kanagawa.port.input. The underlying scalar data direction is unchanged: channel directions inside the bundle flip ('from' <-> 'to') so that the wrapper still produces arguments and consumes any results. EmitEsiWrapper takes the new direction via PushPopEsiBundle's isOutputBundle flag and switches between the input-bundle pattern (InputPort + Read + Unpack) and the output-bundle pattern (Pack + OutputPort + Write). Export function bundles continue to be emitted as input bundles. --- compiler/cpp/circt_util.cpp | 90 ++++++++++++++++++++++++++++--------- compiler/cpp/circt_util.h | 18 ++++++-- compiler/cpp/verilog.cpp | 2 +- 3 files changed, 84 insertions(+), 26 deletions(-) diff --git a/compiler/cpp/circt_util.cpp b/compiler/cpp/circt_util.cpp index 70c18d3..54c0f4a 100644 --- a/compiler/cpp/circt_util.cpp +++ b/compiler/cpp/circt_util.cpp @@ -1350,11 +1350,12 @@ ModuleDeclarationHelper::~ModuleDeclarationHelper() assert(!_bundleName); } -void ModuleDeclarationHelper::BeginEsiBundle(const std::string &name) +void ModuleDeclarationHelper::BeginEsiBundle(const std::string &name, bool isOutputBundle) { assert(!_bundleName); _bundleName = name; + _bundleIsOutput = isOutputBundle; _bundleStartPortIndex = _ports.size(); } @@ -1363,9 +1364,11 @@ void ModuleDeclarationHelper::EndEsiBundle() { assert(_bundleName); - SafeInsert(_bundleNameToPortRange, *_bundleName, std::pair(_bundleStartPortIndex, _ports.size())); + SafeInsert(_bundleNameToPortRange, *_bundleName, + EsiBundleInfo{_bundleStartPortIndex, _ports.size(), _bundleIsOutput}); _bundleName.reset(); + _bundleIsOutput = false; } mlir::Block *ModuleDeclarationHelper::GetBodyBlock() @@ -1837,8 +1840,9 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) { const std::string bundleName = bundleInfo.first; - const size_t startPortIndex = bundleInfo.second.first; - const size_t endPortIndex = bundleInfo.second.second; + const size_t startPortIndex = bundleInfo.second.startPortIndex; + const size_t endPortIndex = bundleInfo.second.endPortIndex; + const bool isOutputBundle = bundleInfo.second.isOutputBundle; // Determine channel and bundle types std::array directionToChannelType = {}; @@ -1950,9 +1954,21 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) circt::esi::BundledChannel bc = {}; bc.name = (channelName == EsiChannelName::Arguments) ? StringToStringAttr("arg") : StringToStringAttr("result"); - bc.direction = (channelSemantics == EsiChannelSemantics::FromGeneratedHw) - ? circt::esi::ChannelDirection::from - : circt::esi::ChannelDirection::to; + // For an input bundle, FromGeneratedHw channels are 'from' (output channels of + // the wrapper) and ToGeneratedHw channels are 'to' (input channels). For an + // output bundle, the wrapper produces the bundle itself, so the channel + // direction tags flip to keep the underlying data flow direction unchanged. + const bool wrapperProducesChannel = (channelSemantics == EsiChannelSemantics::FromGeneratedHw); + if (isOutputBundle) + { + bc.direction = wrapperProducesChannel ? circt::esi::ChannelDirection::to + : circt::esi::ChannelDirection::from; + } + else + { + bc.direction = wrapperProducesChannel ? circt::esi::ChannelDirection::from + : circt::esi::ChannelDirection::to; + } bc.type = channelType; bundleChannelDesc.push_back(bc); @@ -1964,16 +1980,18 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) circt::esi::ChannelBundleType bundleType = circt::esi::ChannelBundleType::get(g_compiler->GetMlirContext(), bundleChannelDesc, nullptr); - // Declare a port on the container with bundle type - circt::kanagawa::InputPortOp inputBundlePort = circt::kanagawa::InputPortOp::create(_opb, - _location, getPortSymbol(bundleName), mlir::TypeAttr::get(bundleType), StringToStringAttr(bundleName)); - - // PortReadOp to get the bundle - mlir::Value inputBundle = circt::kanagawa::PortReadOp::create(_opb, _location, inputBundlePort); - - // For each channel in the bundle - // It is important to handle the FromGeneratedHw channel first - // UnpackBundleOp takes that channel as input + // For each channel in the bundle. + // FromGeneratedHw channels are produced by the wrapper (via wrap.fifo) and + // are handled in the first iteration. ToGeneratedHw channels are consumed + // by the wrapper (via unwrap.vr) and are handled in the second iteration. + // + // Between the two iterations we either: + // * Input bundle: declare an input port, read the bundle, and unpack the + // FromGeneratedHw channels into the bundle to obtain the ToGeneratedHw + // channels. + // * Output bundle: pack the FromGeneratedHw channels into a new bundle, + // obtain the ToGeneratedHw channels as pack's `fromChannels` result, + // declare an output port, and write the bundle to it. llvm::SmallVector fromChannels; llvm::SmallVector toChannels; @@ -1984,11 +2002,41 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) if (channelDirection == 1) { - // Unpack the bundle to get the ToGeneratedHw channel - circt::esi::UnpackBundleOp unpackBundleOp = - circt::esi::UnpackBundleOp::create(_opb, _location, inputBundle, fromChannels); + if (isOutputBundle) + { + // Pack the FromGeneratedHw channels (mapped to bundle-relative `to` + // direction for output bundles) into the bundle. The op's + // `fromChannels` results correspond to the ToGeneratedHw channels + // that the wrapper consumes. + circt::esi::PackBundleOp packBundleOp = + circt::esi::PackBundleOp::create(_opb, _location, bundleType, fromChannels); + + circt::kanagawa::OutputPortOp outputBundlePort = + circt::kanagawa::OutputPortOp::create(_opb, _location, getPortSymbol(bundleName), + mlir::TypeAttr::get(bundleType), + StringToStringAttr(bundleName)); - toChannels = unpackBundleOp.getToChannels(); + circt::kanagawa::PortWriteOp::create(_opb, _location, outputBundlePort, packBundleOp.getBundle()); + + toChannels = packBundleOp.getFromChannels(); + } + else + { + // Declare a port on the container with bundle type. + circt::kanagawa::InputPortOp inputBundlePort = circt::kanagawa::InputPortOp::create( + _opb, _location, getPortSymbol(bundleName), mlir::TypeAttr::get(bundleType), + StringToStringAttr(bundleName)); + + // PortReadOp to get the bundle. + mlir::Value inputBundle = + circt::kanagawa::PortReadOp::create(_opb, _location, inputBundlePort); + + // Unpack the bundle to get the ToGeneratedHw channels. + circt::esi::UnpackBundleOp unpackBundleOp = + circt::esi::UnpackBundleOp::create(_opb, _location, inputBundle, fromChannels); + + toChannels = unpackBundleOp.getToChannels(); + } } // Collect information about the channel diff --git a/compiler/cpp/circt_util.h b/compiler/cpp/circt_util.h index faf3aa3..4ea6e65 100644 --- a/compiler/cpp/circt_util.h +++ b/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 _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> _bundleNameToPortRange; + std::map _bundleNameToPortRange; std::map _portNameToIndex; std::map _outputValues; @@ -406,13 +414,15 @@ class ModuleDeclarationHelper class PushPopEsiBundle { public: - PushPopEsiBundle(ModuleDeclarationHelper& helper, const std::optional& name) : _helper(helper) + PushPopEsiBundle(ModuleDeclarationHelper& helper, const std::optional& name, + bool isOutputBundle = false) + : _helper(helper) { if (name) { _pushedName = true; - helper.BeginEsiBundle(*name); + helper.BeginEsiBundle(*name, isOutputBundle); } else { diff --git a/compiler/cpp/verilog.cpp b/compiler/cpp/verilog.cpp index ce449d1..64c8ab1 100644 --- a/compiler/cpp/verilog.cpp +++ b/compiler/cpp/verilog.cpp @@ -6611,7 +6611,7 @@ class VerilogCompiler esiBundleName = prefix; } - PushPopEsiBundle pushPopEsiBundle(coreModule, esiBundleName); + PushPopEsiBundle pushPopEsiBundle(coreModule, esiBundleName, /*isOutputBundle=*/true); if (isNoBackpressure) { From 3bdafaabd11885b87bc8fc87ae2bc7c1d25c4b81 Mon Sep 17 00:00:00 2001 From: John Demme Date: Wed, 13 May 2026 19:57:45 +0000 Subject: [PATCH 2/5] EsiWrapper: emit single-channel async bundles as bare channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [[async]] export functions and [[async]] callbacks (with backpressure) each produce exactly one ESI channel — there is no return-value channel because [[async]] functions and callbacks are always void. Wrapping such a single channel in a one-element bundle is unnecessary noise for PyCDE consumers, so EmitEsiWrapper now exposes them directly as ports of !esi.channel<...> type: - async export functions become input channel ports - async callbacks become output channel ports Multi-channel bundles (regular functions, regular callbacks) still go through the bundle/pack/unpack path. --- compiler/cpp/circt_util.cpp | 44 ++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/compiler/cpp/circt_util.cpp b/compiler/cpp/circt_util.cpp index 54c0f4a..d9a9d23 100644 --- a/compiler/cpp/circt_util.cpp +++ b/compiler/cpp/circt_util.cpp @@ -1977,8 +1977,16 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) assert(!bundleChannelDesc.empty()); - circt::esi::ChannelBundleType bundleType = - circt::esi::ChannelBundleType::get(g_compiler->GetMlirContext(), bundleChannelDesc, nullptr); + // If a bundle has a single channel (the case for [[async]] functions and + // callbacks, whose argument channel is the only channel), expose it as a + // bare ESI channel port instead of wrapping it in a single-element bundle. + const bool bareChannelPort = (bundleChannelDesc.size() == 1); + + circt::esi::ChannelBundleType bundleType; + if (!bareChannelPort) + { + bundleType = circt::esi::ChannelBundleType::get(g_compiler->GetMlirContext(), bundleChannelDesc, nullptr); + } // For each channel in the bundle. // FromGeneratedHw channels are produced by the wrapper (via wrap.fifo) and @@ -1992,6 +2000,8 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) // * Output bundle: pack the FromGeneratedHw channels into a new bundle, // obtain the ToGeneratedHw channels as pack's `fromChannels` result, // declare an output port, and write the bundle to it. + // * Bare channel port (single-channel async case): skip pack/unpack and + // declare a port of channel type directly. llvm::SmallVector fromChannels; llvm::SmallVector toChannels; @@ -2002,7 +2012,35 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) if (channelDirection == 1) { - if (isOutputBundle) + if (bareChannelPort) + { + // The single channel is either FromGeneratedHw (bundle was + // marked as output) or ToGeneratedHw (bundle was input). + const circt::esi::ChannelType channelType = + directionToChannelType[isOutputBundle ? 0 : 1]; + + if (isOutputBundle) + { + // wrap.fifo from iteration 0 produced the channel; write it + // directly to an output port of channel type. + assert(fromChannels.size() == 1); + circt::kanagawa::OutputPortOp outputChannelPort = + circt::kanagawa::OutputPortOp::create(_opb, _location, getPortSymbol(bundleName), + mlir::TypeAttr::get(channelType), + StringToStringAttr(bundleName)); + circt::kanagawa::PortWriteOp::create(_opb, _location, outputChannelPort, fromChannels[0]); + } + else + { + // Read the channel from an input port of channel type. + circt::kanagawa::InputPortOp inputChannelPort = circt::kanagawa::InputPortOp::create( + _opb, _location, getPortSymbol(bundleName), mlir::TypeAttr::get(channelType), + StringToStringAttr(bundleName)); + toChannels.push_back( + circt::kanagawa::PortReadOp::create(_opb, _location, inputChannelPort)); + } + } + else if (isOutputBundle) { // Pack the FromGeneratedHw channels (mapped to bundle-relative `to` // direction for output bundles) into the bundle. The op's From b7519ef70af9be781fdbfa22ef1548d009728e7c Mon Sep 17 00:00:00 2001 From: John Demme Date: Wed, 13 May 2026 20:19:00 +0000 Subject: [PATCH 3/5] EsiWrapper: emit ValidOnly signaling for [[no_backpressure]] ports [[no_backpressure]] functions and callbacks were previously kept out of the ESI bundle/channel path entirely and exposed as raw scalar valid + payload ports. Now that CIRCT supports ChannelSignaling::ValidOnly along with esi.wrap.vo / esi.unwrap.vo, those ports go through the same EmitEsiWrapper path as backpressured ones and are emitted as !esi.channel<..., ValidOnly>. Specifically: - Free [[no_backpressure]] [[async]] callbacks: bare ValidOnly output channel. - Export functions that are no_backpressure but not fixed-latency: input bundle whose arg and result channels are both ValidOnly. - Fixed-latency export results have no Valid signal at all and remain raw scalars (no ESI wrapping possible). The signaling decision in EmitEsiWrapper now infers the protocol from which control signals are present per direction: * Ready -> ValidReady * ReadEnable / Empty -> FIFO * Valid only -> ValidOnly Adds an interface.circt.esi_wrapper_ports test under test/interface/circt/ that compiles a small program exercising every combination of {export, callback} x {regular, async, no_backpressure}, then runs a python script that asserts each port lowers to the expected EsiWrapper shape and forbids the pre-refactor shapes (input callback bundles, single-channel async bundles). --- compiler/cpp/circt_util.cpp | 112 ++++++++++++++- compiler/cpp/verilog.cpp | 13 +- test/interface/CMakeLists.txt | 2 + test/interface/circt/CMakeLists.txt | 73 ++++++++++ .../circt/check_esi_wrapper_ports.py | 130 ++++++++++++++++++ test/interface/circt/esi_wrapper_ports.k | 44 ++++++ 6 files changed, 364 insertions(+), 10 deletions(-) create mode 100644 test/interface/circt/CMakeLists.txt create mode 100644 test/interface/circt/check_esi_wrapper_ports.py create mode 100644 test/interface/circt/esi_wrapper_ports.k diff --git a/compiler/cpp/circt_util.cpp b/compiler/cpp/circt_util.cpp index d9a9d23..ef8cc6a 100644 --- a/compiler/cpp/circt_util.cpp +++ b/compiler/cpp/circt_util.cpp @@ -1859,7 +1859,15 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) bool channelExists = false; - circt::esi::ChannelSignaling signaling = circt::esi::ChannelSignaling::ValidReady; + // Track which control signals are present so we can pick a + // signaling protocol after the per-port loop: + // * Ready -> ValidReady + // * ReadEnable / Empty -> FIFO + // * Valid only (no others) -> ValidOnly + bool hasValid = false; + bool hasReady = false; + bool hasReadEnable = false; + bool hasEmpty = false; EsiChannelName channelName = EsiChannelName::Undefined; @@ -1883,13 +1891,19 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) switch (portInfo._esiPortSemantics) { case EsiPortSemantics::Valid: + hasValid = true; + break; + case EsiPortSemantics::Ready: - signaling = circt::esi::ChannelSignaling::ValidReady; + hasReady = true; break; case EsiPortSemantics::ReadEnable: + hasReadEnable = true; + break; + case EsiPortSemantics::Empty: - signaling = circt::esi::ChannelSignaling::FIFO; + hasEmpty = true; break; case EsiPortSemantics::Payload: @@ -1909,6 +1923,21 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) if (channelExists) { + circt::esi::ChannelSignaling signaling; + if (hasReady) + { + signaling = circt::esi::ChannelSignaling::ValidReady; + } + else if (hasReadEnable || hasEmpty) + { + signaling = circt::esi::ChannelSignaling::FIFO; + } + else + { + assert(hasValid && "ESI channel must have at least a valid signal"); + signaling = circt::esi::ChannelSignaling::ValidOnly; + } + mlir::Type channelPayloadType; if (payloadTypes.empty()) @@ -2086,6 +2115,7 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) std::string rdenName; mlir::Value ready; mlir::Value empty; + mlir::Value valid; std::vector payload; for (size_t portIndex = startPortIndex; portIndex < endPortIndex; portIndex++) @@ -2100,6 +2130,15 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) { case EsiPortSemantics::Valid: validName = portInfo._hwPortInfo.name.str(); + // For FromGeneratedHw + ValidOnly we also need the valid value + // to feed into wrap.vo, so read it from the inner container. + if (channelSemantics == EsiChannelSemantics::FromGeneratedHw) + { + valid = ReadContainerPort( + _opb, _location, pathToContainer, + GetFullyQualifiedStringAttr(ObjectPath(), portInfo._hwPortInfo.name.str()), + portInfo._hwPortInfo.type); + } break; case EsiPortSemantics::Ready: @@ -2183,6 +2222,46 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) GetFullyQualifiedStringAttr(ObjectPath(), validName), GetI1Type(), unwrapOp.getValid()); } + else if (circt::esi::ChannelSignaling::ValidOnly == signaling) + { + circt::esi::UnwrapValidOnlyOp unwrapOp = + circt::esi::UnwrapValidOnlyOp::create(_opb, _location, inputChannel); + + const llvm::SmallVector &payloadTypes = directionToPayloadTypes[channelDirection]; + const llvm::SmallVector &payloadNames = directionToPayloadNames[channelDirection]; + + if (payloadTypes.size() > 0) + { + if (directionToChannelName[channelDirection] == EsiChannelName::Results) + { + assert(payloadTypes.size() == 1); + + WriteContainerPort(_opb, _location, pathToContainer, + GetFullyQualifiedStringAttr(ObjectPath(), payloadNames[0]), + payloadTypes[0], unwrapOp.getRawOutput()); + } + else + { + assert(directionToChannelName[channelDirection] == EsiChannelName::Arguments); + + circt::hw::StructExplodeOp explodeOp = + circt::hw::StructExplodeOp::create(_opb, _location, unwrapOp.getRawOutput()); + + assert(payloadNames.size() == payloadTypes.size()); + + for (size_t i = 0; i < payloadTypes.size(); i++) + { + WriteContainerPort(_opb, _location, pathToContainer, + GetFullyQualifiedStringAttr(ObjectPath(), payloadNames[i]), + payloadTypes[i], explodeOp.getResult()[i]); + } + } + } + + WriteContainerPort(_opb, _location, pathToContainer, + GetFullyQualifiedStringAttr(ObjectPath(), validName), GetI1Type(), + unwrapOp.getValid()); + } else { assert(false); @@ -2230,6 +2309,33 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) GetFullyQualifiedStringAttr(ObjectPath(), rdenName), GetI1Type(), wrapOp.getRden()); } + else if (circt::esi::ChannelSignaling::ValidOnly == signaling) + { + circt::esi::ChannelType channelType = directionToChannelType[channelDirection]; + + mlir::Value wrapPayload; + + if (payload.empty()) + { + wrapPayload = circt::hw::ConstantOp::create(_opb, _location, + _opb.getIntegerAttr(GetIntegerType(0), 0)); + } + else if (directionToChannelName[channelDirection] == EsiChannelName::Results) + { + wrapPayload = payload[0]; + } + else + { + assert(directionToChannelName[channelDirection] == EsiChannelName::Arguments); + + wrapPayload = + circt::hw::StructCreateOp::create(_opb, _location, channelType.getInner(), payload); + } + + circt::esi::WrapValidOnlyOp wrapOp = + circt::esi::WrapValidOnlyOp::create(_opb, _location, channelType, wrapPayload, valid); + outputChannel = wrapOp.getChanOutput(); + } else { assert(false); diff --git a/compiler/cpp/verilog.cpp b/compiler/cpp/verilog.cpp index 64c8ab1..bab9a4d 100644 --- a/compiler/cpp/verilog.cpp +++ b/compiler/cpp/verilog.cpp @@ -6481,7 +6481,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,12 +6608,7 @@ class VerilogCompiler const bool isNoBackpressure = functionNode->GetModifiers() & ParseTreeFunctionModifierNoBackPressure; - std::optional esiBundleName; - - if (!isNoBackpressure) - { - esiBundleName = prefix; - } + std::optional esiBundleName = prefix; PushPopEsiBundle pushPopEsiBundle(coreModule, esiBundleName, /*isOutputBundle=*/true); diff --git a/test/interface/CMakeLists.txt b/test/interface/CMakeLists.txt index 44d1bcf..0ba4d44 100644 --- a/test/interface/CMakeLists.txt +++ b/test/interface/CMakeLists.txt @@ -187,3 +187,5 @@ add_interface_test(ecc_mem ecc_mem.k TESTBENCH ecc_mem.sv ) + +add_subdirectory(circt) diff --git a/test/interface/circt/CMakeLists.txt b/test/interface/circt/CMakeLists.txt new file mode 100644 index 0000000..634d23a --- /dev/null +++ b/test/interface/circt/CMakeLists.txt @@ -0,0 +1,73 @@ +# 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 +# without simulating it. +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 $ + ${_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" +) diff --git a/test/interface/circt/check_esi_wrapper_ports.py b/test/interface/circt/check_esi_wrapper_ports.py new file mode 100644 index 0000000..31bae70 --- /dev/null +++ b/test/interface/circt/check_esi_wrapper_ports.py @@ -0,0 +1,130 @@ +#!/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 + +# (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 = [ + # Pre-Phase-1 callback bundles were `kanagawa.port.input` for any *_cb. + re.compile(r'kanagawa\.port\.input\s+"\w*_cb"'), + # Pre-Phase-2 single-channel async ports were single-element bundles. + 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(('', + 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()) diff --git a/test/interface/circt/esi_wrapper_ports.k b/test/interface/circt/esi_wrapper_ports.k new file mode 100644 index 0000000..773b6f3 --- /dev/null +++ b/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; From 86a39521f6cd1842a8a197ef3db065e36c15b884 Mon Sep 17 00:00:00 2001 From: John Demme Date: Fri, 15 May 2026 20:17:50 +0000 Subject: [PATCH 4/5] Address PR feedback and tidy - circt_util.cpp: replace literal 0/1 indices in the bare-channel branch with named constants (kFromGeneratedHwIdx / kToGeneratedHwIdx) and assert(valid) immediately before WrapValidOnlyOp::create to enforce the invariant at the point of use (Copilot review comments on #129). - verilog.cpp: minor formatting tweak for GetBasicBlockInstanceName. - test/interface/circt: trim duplicated comments and add a TODO noting that the python checker should ideally use the CIRCT Python API. --- compiler/cpp/circt_util.cpp | 12 +++++++++++- compiler/cpp/verilog.cpp | 3 ++- test/interface/circt/CMakeLists.txt | 3 +-- test/interface/circt/check_esi_wrapper_ports.py | 7 +++++-- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/compiler/cpp/circt_util.cpp b/compiler/cpp/circt_util.cpp index ef8cc6a..cc30b16 100644 --- a/compiler/cpp/circt_util.cpp +++ b/compiler/cpp/circt_util.cpp @@ -1844,6 +1844,12 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) const size_t endPortIndex = bundleInfo.second.endPortIndex; const bool isOutputBundle = bundleInfo.second.isOutputBundle; + // Per-direction storage indexed by channelDirection: 0 == FromGeneratedHw, + // 1 == ToGeneratedHw. (Independent of EsiChannelSemantics' underlying enum + // values.) + constexpr size_t kFromGeneratedHwIdx = 0; + constexpr size_t kToGeneratedHwIdx = 1; + // Determine channel and bundle types std::array directionToChannelType = {}; std::array directionToChannelExists = {}; @@ -2046,7 +2052,7 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) // The single channel is either FromGeneratedHw (bundle was // marked as output) or ToGeneratedHw (bundle was input). const circt::esi::ChannelType channelType = - directionToChannelType[isOutputBundle ? 0 : 1]; + directionToChannelType[isOutputBundle ? kFromGeneratedHwIdx : kToGeneratedHwIdx]; if (isOutputBundle) { @@ -2332,6 +2338,10 @@ void ModuleDeclarationHelper::EmitEsiWrapper(const std::string &circtDesignName) circt::hw::StructCreateOp::create(_opb, _location, channelType.getInner(), payload); } + // ValidOnly is only selected when the port group has a Valid + // signal, and the per-port loop above always reads it for + // FromGeneratedHw, so `valid` must be set here. + assert(valid); circt::esi::WrapValidOnlyOp wrapOp = circt::esi::WrapValidOnlyOp::create(_opb, _location, channelType, wrapPayload, valid); outputChannel = wrapOp.getChanOutput(); diff --git a/compiler/cpp/verilog.cpp b/compiler/cpp/verilog.cpp index bab9a4d..8a22ecd 100644 --- a/compiler/cpp/verilog.cpp +++ b/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"); } diff --git a/test/interface/circt/CMakeLists.txt b/test/interface/circt/CMakeLists.txt index 634d23a..6d70536 100644 --- a/test/interface/circt/CMakeLists.txt +++ b/test/interface/circt/CMakeLists.txt @@ -3,8 +3,7 @@ # 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 -# without simulating it. +# 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) diff --git a/test/interface/circt/check_esi_wrapper_ports.py b/test/interface/circt/check_esi_wrapper_ports.py index 31bae70..25c6ccc 100644 --- a/test/interface/circt/check_esi_wrapper_ports.py +++ b/test/interface/circt/check_esi_wrapper_ports.py @@ -14,6 +14,11 @@ 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 = [ @@ -64,9 +69,7 @@ # Forbid the no-longer-emitted shapes so silent regressions don't slip past. FORBIDDEN_LINES = [ - # Pre-Phase-1 callback bundles were `kanagawa.port.input` for any *_cb. re.compile(r'kanagawa\.port\.input\s+"\w*_cb"'), - # Pre-Phase-2 single-channel async ports were single-element bundles. re.compile(r'"AsyncExport"[^\n]*!esi\.bundle<'), re.compile(r'"async_cb"[^\n]*!esi\.bundle<'), re.compile(r'"nbp_cb"[^\n]*!esi\.bundle<'), From 5762179eed8cdc92e97c88c7d3a00a73aaedcfb4 Mon Sep 17 00:00:00 2001 From: John Demme Date: Mon, 18 May 2026 18:36:31 +0000 Subject: [PATCH 5/5] feedback --- compiler/cpp/circt_util.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/cpp/circt_util.cpp b/compiler/cpp/circt_util.cpp index cc30b16..21ce0ab 100644 --- a/compiler/cpp/circt_util.cpp +++ b/compiler/cpp/circt_util.cpp @@ -1350,7 +1350,7 @@ ModuleDeclarationHelper::~ModuleDeclarationHelper() assert(!_bundleName); } -void ModuleDeclarationHelper::BeginEsiBundle(const std::string &name, bool isOutputBundle) +void ModuleDeclarationHelper::BeginEsiBundle(const std::string &name, const bool isOutputBundle) { assert(!_bundleName);