Skip to content

Commit d89f7d8

Browse files
teqdruidteqdruid
authored
[ESI] Service requests now track AppIDs rather than instance hierarchy (#6328)
This makes for far more sensible hierarchies. Also makes the index field of AppIDs optional.
1 parent b9b6592 commit d89f7d8

File tree

12 files changed

+111
-74
lines changed

12 files changed

+111
-74
lines changed

‎include/circt-c/Dialect/ESI.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ MLIR_CAPI_EXPORTED
8181
MlirAttribute circtESIAppIDAttrGet(MlirContext, MlirStringRef name,
8282
uint64_t index);
8383
MLIR_CAPI_EXPORTED MlirStringRef circtESIAppIDAttrGetName(MlirAttribute attr);
84-
MLIR_CAPI_EXPORTED uint64_t circtESIAppIDAttrGetIndex(MlirAttribute attr);
84+
MLIR_CAPI_EXPORTED bool circtESIAppIDAttrGetIndex(MlirAttribute attr,
85+
uint64_t *index);
8586

8687
MLIR_CAPI_EXPORTED bool circtESIAttributeIsAnAppIDPathAttr(MlirAttribute);
8788
MLIR_CAPI_EXPORTED

‎include/circt/Dialect/ESI/ESIAppID.td

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#ifndef CIRCT_DIALECT_ESI_APPID_TD
1414
#define CIRCT_DIALECT_ESI_APPID_TD
1515

16+
include "circt/Dialect/ESI/ESIDialect.td"
17+
1618
include "mlir/IR/AttrTypeBase.td"
1719

1820
def AppIDAttr : ESI_Attr<"AppID"> {
@@ -22,17 +24,21 @@ def AppIDAttr : ESI_Attr<"AppID"> {
2224
Indended to make locating an instance easier in the instance hierarchy.
2325
}];
2426

25-
let parameters = (ins "StringAttr":$name, "uint64_t":$index);
26-
let mnemonic = "appid";
27-
let assemblyFormat = [{
28-
`<` $name `[` $index `]` `>`
29-
}];
27+
let parameters = (ins "StringAttr":$name,
28+
OptionalParameter<"std::optional<uint64_t>">:$index);
29+
let mnemonic = "appid";
30+
let assemblyFormat = [{
31+
`<` $name (`[` $index^ `]`)? `>`
32+
}];
3033

3134
let extraClassDeclaration = [{
3235
static constexpr StringRef AppIDAttrName = "esi.appid";
3336
}];
3437
}
3538

39+
def AppIDArrayAttr :
40+
TypedArrayAttrBase<AppIDAttr, "Array of AppIDs">;
41+
3642
def AppIDPathAttr : ESI_Attr<"AppIDPath"> {
3743
let summary = "An application-specific hierarchical path through a design";
3844
let description = [{

‎include/circt/Dialect/ESI/ESIOps.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef CIRCT_DIALECT_ESI_ESIOPS_H
1414
#define CIRCT_DIALECT_ESI_ESIOPS_H
1515

16+
#include "circt/Dialect/ESI/ESIAttributes.h"
1617
#include "circt/Dialect/ESI/ESIDialect.h"
1718
#include "circt/Dialect/ESI/ESITypes.h"
1819

‎include/circt/Dialect/ESI/ESIServices.td

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
include "mlir/IR/RegionKindInterface.td"
1313
include "mlir/IR/SymbolInterfaces.td"
1414

15+
include "circt/Dialect/ESI/ESIAppID.td"
1516
include "circt/Dialect/ESI/ESIChannels.td"
1617
include "circt/Dialect/ESI/ESIInterfaces.td"
1718
include "circt/Dialect/ESI/ESITypes.td"
@@ -147,10 +148,10 @@ def ServiceImplementConnReqOp : ESI_Op<"service.impl_req.req", [
147148
let summary = "The canonical form of a connection request";
148149

149150
let arguments = (ins InnerRefAttr:$servicePort,
150-
StrArrayAttr:$clientNamePath);
151+
AppIDArrayAttr:$relativeAppIDPath);
151152
let results = (outs ChannelBundleType:$toClient);
152153
let assemblyFormat = [{
153-
$servicePort `(` $clientNamePath `)`
154+
$servicePort `(` $relativeAppIDPath `)`
154155
attr-dict `:` qualified(type($toClient))
155156
}];
156157

@@ -165,9 +166,9 @@ def RequestToServerConnectionOp : ESI_Op<"service.req.to_server", [
165166

166167
let arguments = (ins InnerRefAttr:$servicePort,
167168
ChannelBundleType:$toServer,
168-
StrArrayAttr:$clientNamePath);
169+
AppIDAttr:$appID);
169170
let assemblyFormat = [{
170-
$toServer `->` $servicePort `(` $clientNamePath `)`
171+
$toServer `->` $servicePort `(` qualified($appID) `)`
171172
attr-dict `:` qualified(type($toServer))
172173
}];
173174
}
@@ -177,10 +178,10 @@ def RequestToClientConnectionOp : ESI_Op<"service.req.to_client", [
177178
let summary = "Request a connection to receive data";
178179

179180
let arguments = (ins InnerRefAttr:$servicePort,
180-
StrArrayAttr:$clientNamePath);
181+
AppIDAttr:$appID);
181182
let results = (outs ChannelBundleType:$toClient);
182183
let assemblyFormat = [{
183-
$servicePort `(` $clientNamePath `)`
184+
$servicePort `(` qualified($appID) `)`
184185
attr-dict `:` qualified(type($toClient))
185186
}];
186187
}

‎integration_test/Bindings/Python/dialects/esi.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545

4646
# CHECK-LABEL: === testGen called with op:
4747
# CHECK: %0:2 = esi.service.impl_req svc @HostComms impl as "test"(%clk) : (i1) -> (i8, !esi.bundle<[!esi.channel<i8> to "recv"]>) {
48-
# CHECK: %2 = esi.service.impl_req.req <@HostComms::@Recv>(["m1", "loopback_tohw"]) : !esi.bundle<[!esi.channel<i8> to "recv"]>
48+
# CHECK: %2 = esi.service.impl_req.req <@HostComms::@Recv>([#esi.appid<"loopback_tohw">]) : !esi.bundle<[!esi.channel<i8> to "recv"]>
4949
def testGen(reqOp: esi.ServiceImplementReqOp) -> bool:
5050
print("=== testGen called with op:")
5151
reqOp.print()
@@ -71,7 +71,7 @@ def testGen(reqOp: esi.ServiceImplementReqOp) -> bool:
7171
}
7272
7373
hw.module @MsLoopback (in %clk : i1) {
74-
%dataIn = esi.service.req.to_client <@HostComms::@Recv> (["loopback_tohw"]) : !recvI8
74+
%dataIn = esi.service.req.to_client <@HostComms::@Recv> (#esi.appid<"loopback_tohw">) : !recvI8
7575
}
7676
""")
7777
pm = passmanager.PassManager.parse("builtin.module(esi-connect-services)")

‎lib/Bindings/Python/ESIModule.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,11 @@ void circt::python::populateDialectESISubmodule(py::module &m) {
170170
unwrap(circtESIAppIDAttrGetName(self));
171171
return std::string(name.data(), name.size());
172172
})
173-
.def_property_readonly("index", [](MlirAttribute self) {
174-
return circtESIAppIDAttrGetIndex(self);
173+
.def_property_readonly("index", [](MlirAttribute self) -> py::object {
174+
uint64_t index;
175+
if (circtESIAppIDAttrGetIndex(self, &index))
176+
return py::cast(index);
177+
return py::none();
175178
});
176179

177180
mlir_attribute_subclass(m, "AppIDPathAttr",

‎lib/CAPI/Dialect/ESI.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,12 @@ MlirAttribute circtESIAppIDAttrGet(MlirContext ctxt, MlirStringRef name,
145145
MlirStringRef circtESIAppIDAttrGetName(MlirAttribute attr) {
146146
return wrap(unwrap(attr).cast<AppIDAttr>().getName().getValue());
147147
}
148-
uint64_t circtESIAppIDAttrGetIndex(MlirAttribute attr) {
149-
return unwrap(attr).cast<AppIDAttr>().getIndex();
148+
bool circtESIAppIDAttrGetIndex(MlirAttribute attr, uint64_t *indexOut) {
149+
std::optional<uint64_t> index = unwrap(attr).cast<AppIDAttr>().getIndex();
150+
if (!index)
151+
return false;
152+
*indexOut = index.value();
153+
return true;
150154
}
151155

152156
bool circtESIAttributeIsAnAppIDPathAttr(MlirAttribute attr) {

‎lib/Dialect/ESI/ESIServices.cpp

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,18 @@ static LogicalResult instantiateCosimEndpointOps(ServiceImplementReqOp implReq,
6262
}
6363

6464
// Assemble the name to use for an endpoint.
65-
auto toStringAttr = [&](ArrayAttr strArr) {
65+
auto toStringAttr = [&](ArrayAttr strArr, StringAttr channelName) {
6666
std::string buff;
6767
llvm::raw_string_ostream os(buff);
68-
llvm::interleave(strArr.getAsValueRange<StringAttr>(), os, ".");
68+
llvm::interleave(
69+
strArr.getAsRange<AppIDAttr>(), os,
70+
[&](AppIDAttr appid) {
71+
os << appid.getName().getValue();
72+
if (appid.getIndex())
73+
os << "[" << appid.getIndex() << "]";
74+
},
75+
".");
76+
os << "." << channelName.getValue();
6977
return StringAttr::get(ctxt, os.str());
7078
};
7179

@@ -81,15 +89,14 @@ static LogicalResult instantiateCosimEndpointOps(ServiceImplementReqOp implReq,
8189
for (auto req : implReq.getOps<ServiceImplementConnReqOp>()) {
8290
Location loc = req->getLoc();
8391
ChannelBundleType bundleType = req.getToClient().getType();
84-
ArrayAttr clientNamePathAttr = req.getClientNamePath();
8592

8693
SmallVector<Value, 8> toServerValues;
8794
for (BundledChannel ch : bundleType.getChannels()) {
8895
if (ch.direction == ChannelDirection::to) {
8996
auto nullSource = b.create<NullSourceOp>(loc, i1ch);
9097
auto cosim = b.create<CosimEndpointOp>(
9198
loc, ch.type, clk, rst, nullSource.getOut(),
92-
toStringAttr(clientNamePathAttr));
99+
toStringAttr(req.getRelativeAppIDPathAttr(), ch.name));
93100
toServerValues.push_back(cosim.getRecv());
94101
}
95102
}
@@ -102,9 +109,9 @@ static LogicalResult instantiateCosimEndpointOps(ServiceImplementReqOp implReq,
102109
size_t chanIdx = 0;
103110
for (BundledChannel ch : bundleType.getChannels()) {
104111
if (ch.direction == ChannelDirection::from)
105-
b.create<CosimEndpointOp>(loc, i1ch, clk, rst,
106-
pack.getFromChannels()[chanIdx++],
107-
toStringAttr(clientNamePathAttr));
112+
b.create<CosimEndpointOp>(
113+
loc, i1ch, clk, rst, pack.getFromChannels()[chanIdx++],
114+
toStringAttr(req.getRelativeAppIDPathAttr(), ch.name));
108115
}
109116
}
110117

@@ -352,7 +359,8 @@ void ESIConnectServicesPass::convertReq(
352359
// to_client requests are already in the canonical form, just the wrong op.
353360
auto newReq = b.create<ServiceImplementConnReqOp>(
354361
toClientReq.getLoc(), toClientReq.getToClient().getType(),
355-
toClientReq.getServicePortAttr(), toClientReq.getClientNamePath());
362+
toClientReq.getServicePortAttr(),
363+
ArrayAttr::get(&getContext(), {toClientReq.getAppIDAttr()}));
356364
newReq->setDialectAttrs(toClientReq->getDialectAttrs());
357365
toClientReq.getToClient().replaceAllUsesWith(newReq.getToClient());
358366

@@ -372,7 +380,7 @@ void ESIConnectServicesPass::convertReq(
372380
// Create the new canonical request. It's modeled in the to_client form.
373381
auto toClientReq = b.create<ServiceImplementConnReqOp>(
374382
toServerReq.getLoc(), toClientType, toServerReq.getServicePortAttr(),
375-
toServerReq.getClientNamePath());
383+
ArrayAttr::get(&getContext(), {toServerReq.getAppIDAttr()}));
376384
toClientReq->setDialectAttrs(toServerReq->getDialectAttrs());
377385

378386
// Unpack the bundle coming from the new request.
@@ -512,8 +520,12 @@ ESIConnectServicesPass::surfaceReqs(hw::HWMutableModuleLike mod,
512520
std::string portName;
513521
llvm::raw_string_ostream nameOS(portName);
514522
llvm::interleave(
515-
namePath.getValue(), nameOS,
516-
[&](Attribute attr) { nameOS << attr.cast<StringAttr>().getValue(); },
523+
namePath.getAsRange<AppIDAttr>(), nameOS,
524+
[&](AppIDAttr appid) {
525+
nameOS << appid.getName().getValue();
526+
if (appid.getIndex())
527+
nameOS << "_" << appid.getIndex();
528+
},
517529
".");
518530
return StringAttr::get(ctxt, nameOS.str());
519531
};
@@ -527,8 +539,8 @@ ESIConnectServicesPass::surfaceReqs(hw::HWMutableModuleLike mod,
527539
for (auto req : reqs) {
528540
newInputs.push_back(std::make_pair(
529541
origNumInputs,
530-
hw::PortInfo{{getPortName(req.getClientNamePath()), req.getBundleType(),
531-
hw::ModulePort::Direction::Input},
542+
hw::PortInfo{{getPortName(req.getRelativeAppIDPathAttr()),
543+
req.getBundleType(), hw::ModulePort::Direction::Input},
532544
origNumInputs,
533545
{},
534546
req->getLoc()}));
@@ -540,11 +552,11 @@ ESIConnectServicesPass::surfaceReqs(hw::HWMutableModuleLike mod,
540552
mod.insertPorts(newInputs, {});
541553

542554
// Prepend a name to the instance tracking array.
543-
auto prependNamePart = [&](ArrayAttr namePath, StringRef part) {
544-
SmallVector<Attribute, 8> newNamePath;
545-
newNamePath.push_back(StringAttr::get(namePath.getContext(), part));
546-
newNamePath.append(namePath.begin(), namePath.end());
547-
return ArrayAttr::get(namePath.getContext(), newNamePath);
555+
auto prependNamePart = [&](ArrayAttr appIDPath, AppIDAttr appID) {
556+
SmallVector<Attribute, 8> newAppIDPath;
557+
newAppIDPath.push_back(appID);
558+
newAppIDPath.append(appIDPath.begin(), appIDPath.end());
559+
return ArrayAttr::get(appIDPath.getContext(), newAppIDPath);
548560
};
549561

550562
// Update the module instantiations.
@@ -555,9 +567,16 @@ ESIConnectServicesPass::surfaceReqs(hw::HWMutableModuleLike mod,
555567
// Add new inputs for the new bundles being requested.
556568
SmallVector<Value, 16> newOperands;
557569
for (auto req : reqs) {
570+
// If the instance has an AppID, prepend it.
571+
ArrayAttr appIDPath = req.getRelativeAppIDPathAttr();
572+
if (auto instAppID = dyn_cast_or_null<AppIDAttr>(
573+
inst->getDiscardableAttr(AppIDAttr::AppIDAttrName)))
574+
appIDPath = prependNamePart(appIDPath, instAppID);
575+
576+
// Clone the request.
558577
auto clone = b.create<ServiceImplementConnReqOp>(
559578
req.getLoc(), req.getToClient().getType(), req.getServicePortAttr(),
560-
prependNamePart(req.getClientNamePath(), inst.getInstanceName()));
579+
appIDPath);
561580
clone->setDialectAttrs(req->getDialectAttrs());
562581
newOperands.push_back(clone.getToClient());
563582
}

‎lib/Dialect/Handshake/Transforms/LowerExtmemToHW.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,8 @@ LogicalResult HandshakeLowerExtmemToHWPass::wrapESI(
211211
auto reqPack = b.create<esi::PackBundleOp>(loc, readPortInfo.type,
212212
(Value)backedges[resIdx]);
213213
b.create<esi::RequestToServerConnectionOp>(
214-
loc, readPortInfo.port, reqPack.getBundle(), b.getArrayAttr({}));
214+
loc, readPortInfo.port, reqPack.getBundle(),
215+
esi::AppIDAttr::get(ctx, b.getStringAttr("load"), {}));
215216
instanceArgsFromThisMem.push_back(
216217
reqPack.getFromChannels()
217218
[esi::RandomAccessMemoryDeclOp::RespDirChannelIdx]);
@@ -223,7 +224,8 @@ LogicalResult HandshakeLowerExtmemToHWPass::wrapESI(
223224
auto reqPack = b.create<esi::PackBundleOp>(loc, writePortInfo.type,
224225
(Value)backedges[resIdx]);
225226
b.create<esi::RequestToServerConnectionOp>(
226-
loc, writePortInfo.port, reqPack.getBundle(), b.getArrayAttr({}));
227+
loc, writePortInfo.port, reqPack.getBundle(),
228+
esi::AppIDAttr::get(ctx, b.getStringAttr("store"), {}));
227229
instanceArgsFromThisMem.push_back(
228230
reqPack.getFromChannels()
229231
[esi::RandomAccessMemoryDeclOp::RespDirChannelIdx]);

‎test/Dialect/ESI/errors.mlir

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ esi.service.decl @HostComms {
5252

5353
hw.module @Loopback (in %clk: i1, in %dataIn: !esi.bundle<[!esi.channel<i8> to "send"]>) {
5454
// expected-error @+1 {{Service port is not a to-server port}}
55-
esi.service.req.to_server %dataIn -> <@HostComms::@Send> (["loopback_fromhw"]) : !esi.bundle<[!esi.channel<i8> to "send"]>
55+
esi.service.req.to_server %dataIn -> <@HostComms::@Send> (#esi.appid<"loopback_fromhw">) : !esi.bundle<[!esi.channel<i8> to "send"]>
5656
}
5757

5858
// -----
@@ -63,7 +63,7 @@ esi.service.decl @HostComms {
6363

6464
hw.module @Loopback (in %clk: i1) {
6565
// expected-error @+1 {{Service port is not a to-client port}}
66-
esi.service.req.to_client <@HostComms::@Send> (["loopback_fromhw"]) : !esi.bundle<[!esi.channel<i8> to "send"]>
66+
esi.service.req.to_client <@HostComms::@Send> (#esi.appid<"loopback_fromhw">) : !esi.bundle<[!esi.channel<i8> to "send"]>
6767
}
6868

6969
// -----
@@ -74,7 +74,7 @@ esi.service.decl @HostComms {
7474

7575
hw.module @Loopback (in %clk: i1, in %dataIn: !esi.bundle<[!esi.channel<i8> to "send"]>) {
7676
// expected-error @+1 {{Request channel type does not match service port bundle channel type}}
77-
esi.service.req.to_server %dataIn -> <@HostComms::@Send> (["loopback_fromhw"]) : !esi.bundle<[!esi.channel<i8> to "send"]>
77+
esi.service.req.to_server %dataIn -> <@HostComms::@Send> (#esi.appid<"loopback_fromhw">) : !esi.bundle<[!esi.channel<i8> to "send"]>
7878
}
7979

8080
// -----
@@ -85,7 +85,7 @@ esi.service.decl @HostComms {
8585

8686
hw.module @Loopback (in %clk: i1, in %dataIn: !esi.bundle<[!esi.channel<i8> to "send", !esi.channel<i3> to "foo"]>) {
8787
// expected-error @+1 {{Request port bundle channel count does not match service port bundle channel count}}
88-
esi.service.req.to_server %dataIn -> <@HostComms::@Send> (["loopback_fromhw"]) : !esi.bundle<[!esi.channel<i8> to "send", !esi.channel<i3> to "foo"]>
88+
esi.service.req.to_server %dataIn -> <@HostComms::@Send> (#esi.appid<"loopback_fromhw">) : !esi.bundle<[!esi.channel<i8> to "send", !esi.channel<i3> to "foo"]>
8989
}
9090

9191
// -----
@@ -95,14 +95,14 @@ esi.service.decl @HostComms {
9595

9696
hw.module @Loopback (in %clk: i1) {
9797
// expected-error @+1 {{'esi.service.req.to_client' op Could not locate port "Recv"}}
98-
%dataIn = esi.service.req.to_client <@HostComms::@Recv> (["loopback_tohw"]) : !esi.bundle<[!esi.channel<i1> from "foo"]>
98+
%dataIn = esi.service.req.to_client <@HostComms::@Recv> (#esi.appid<"loopback_tohw">) : !esi.bundle<[!esi.channel<i1> from "foo"]>
9999
}
100100

101101
// -----
102102

103103
hw.module @Loopback (in %clk: i1) {
104104
// expected-error @+1 {{'esi.service.req.to_client' op Could not find service declaration @HostComms}}
105-
%dataIn = esi.service.req.to_client <@HostComms::@Recv> (["loopback_tohw"]) : !esi.bundle<[!esi.channel<i1> from "foo"]>
105+
%dataIn = esi.service.req.to_client <@HostComms::@Recv> (#esi.appid<"loopback_tohw">) : !esi.bundle<[!esi.channel<i1> from "foo"]>
106106
}
107107

108108
// -----

0 commit comments

Comments
 (0)