Skip to content

Commit

Permalink
[ESI] Improve the way to/from server requests are handled (#5586)
Browse files Browse the repository at this point in the history 
Fixes #5490.

Don't break apart in/out requests. Create an op interface to avoid having to reason about three different ops.
  • Loading branch information
@teqdruid
teqdruid authored Jul 17, 2023
1 parent 54839d3 commit 95c011b
Show file tree
Hide file tree
Showing 6 changed files with 279 additions and 265 deletions.
65 changes: 65 additions & 0 deletions include/circt/Dialect/ESI/ESIInterfaces.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,68 @@ def ServiceDeclOpInterface : OpInterface<"ServiceDeclOpInterface"> {
}]>,
];
}

def ServiceReqOpInterface : OpInterface<"ServiceReqOpInterface"> {
let cppNamespace = "circt::esi";
let description = [{
Any op which is requesting connection to a service's port or has information
pertaining to one of said requests.
}];

let methods = [
InterfaceMethod<
"Returns the service port symbol.",
"hw::InnerRefAttr", "getServicePort", (ins)
>,
InterfaceMethod<
"Returns the client name path.",
"ArrayAttr", "getClientNamePath", (ins)
>,
InterfaceMethod<
"Set the client name path.",
"void", "setClientNamePath", (ins "ArrayAttr":$newClientNamePath),
/*methodBody=*/"",
/*defaultImplementation=*/[{
$_op.setClientNamePathAttr(newClientNamePath);
}]>,
InterfaceMethod<
"Returns the type headed to the client. Can be null.",
"Type", "getToClientType", (ins),
/*methodBody=*/[{
Value toClient = $_op.getToClient();
if (!toClient)
return {};
return toClient.getType();
}]>,
InterfaceMethod<
"Returns the value headed to the client. Can be null.",
"Value", "getToClient", (ins),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return {};
}]>,
InterfaceMethod<
"Returns the type headed to the server. Can be null.",
"Type", "getToServerType", (ins),
/*methodBody=*/[{
Value toServer = $_op.getToServer();
if (!toServer)
return {};
return toServer.getType();
}]>,
InterfaceMethod<
"Returns the value headed to the server. Can be null.",
"Value", "getToServer", (ins),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return {};
}]>,
InterfaceMethod<
"Set the value headed to the server. Can assert().",
"void", "setToServer", (ins "Value":$newValue),
/*methodBody=*/"",
/*defaultImplementation=*/[{
assert(false && "This op doesn't support the to server direction");
}]>
];
}
34 changes: 22 additions & 12 deletions include/circt/Dialect/ESI/ESIServices.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,6 @@ def ServiceImplementReqOp : ESI_Op<"service.impl_req", [NoTerminator]> {
let results = (outs Variadic<AnyType>:$outputs);
let regions = (region SizedRegion<1>:$portReqs);

let extraClassDeclaration = [{
/// Find pairs of toServer and toClient requests with the same client path.
/// In cases where there doesn't exist a pair, one of the two entries will
/// be null. Should never contain a pair with both entries null.
void gatherPairedReqs(
llvm::SmallVectorImpl<std::pair<RequestToServerConnectionOp,
RequestToClientConnectionOp>>&);
}];

let assemblyFormat = [{
(`svc` $service_symbol^)? `impl` `as` $impl_type (`opts` $impl_opts^)?
`(` $inputs `)` attr-dict `:` functional-type($inputs, results)
Expand All @@ -137,7 +128,8 @@ def ServiceImplementReqOp : ESI_Op<"service.impl_req", [NoTerminator]> {
}

def RequestToServerConnectionOp : ESI_Op<"service.req.to_server", [
DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
DeclareOpInterfaceMethods<SymbolUserOpInterface>,
ServiceReqOpInterface]> {
let summary = "Request a connection to send data";

let arguments = (ins HWInnerRefAttr:$servicePort,
Expand All @@ -146,10 +138,19 @@ def RequestToServerConnectionOp : ESI_Op<"service.req.to_server", [
$toServer `->` $servicePort `(` $clientNamePath `)`
attr-dict `:` qualified(type($toServer))
}];

let extraClassDeclaration = [{
// Set the value headed to the server. Overrides method in
// ServiceReqOpInterface.
void setToServer(Value v) {
getToServerMutable().assign(v);
}
}];
}

def RequestToClientConnectionOp : ESI_Op<"service.req.to_client", [
DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
DeclareOpInterfaceMethods<SymbolUserOpInterface>,
ServiceReqOpInterface]> {
let summary = "Request a connection to receive data";

let arguments = (ins HWInnerRefAttr:$servicePort,
Expand All @@ -162,7 +163,8 @@ def RequestToClientConnectionOp : ESI_Op<"service.req.to_client", [
}

def RequestInOutChannelOp : ESI_Op<"service.req.inout", [
DeclareOpInterfaceMethods<SymbolUserOpInterface>]> {
DeclareOpInterfaceMethods<SymbolUserOpInterface>,
ServiceReqOpInterface]> {
let summary = "Request a bidirectional channel";

let arguments = (ins HWInnerRefAttr:$servicePort,
Expand All @@ -174,6 +176,14 @@ def RequestInOutChannelOp : ESI_Op<"service.req.inout", [
$toServer `->` $servicePort `(` $clientNamePath `)` attr-dict `:`
qualified(type($toServer)) `->` qualified(type($toClient))
}];

let extraClassDeclaration = [{
// Set the value headed to the server. Overrides method in
// ServiceReqOpInterface.
void setToServer(Value v) {
getToServerMutable().assign(v);
}
}];
}

def ServiceHierarchyMetadataOp : ESI_Op<"service.hierarchy.metadata", [
Expand Down
55 changes: 0 additions & 55 deletions lib/Dialect/ESI/ESIOps.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,61 +464,6 @@ LogicalResult ServiceHierarchyMetadataOp::verifySymbolUses(
return success();
}

void ServiceImplementReqOp::gatherPairedReqs(
llvm::SmallVectorImpl<std::pair<RequestToServerConnectionOp,
RequestToClientConnectionOp>> &reqPairs) {

// Build a mapping of client path names to requests.
DenseMap<std::pair<hw::InnerRefAttr, ArrayAttr>,
SmallVector<RequestToServerConnectionOp, 0>>
clientNameToServer;
DenseMap<std::pair<hw::InnerRefAttr, ArrayAttr>,
SmallVector<RequestToClientConnectionOp, 0>>
clientNameToClient;
for (auto &op : getOps())
if (auto req = dyn_cast<RequestToClientConnectionOp>(op))
clientNameToClient[std::make_pair(req.getServicePort(),
req.getClientNamePathAttr())]
.push_back(req);
else if (auto req = dyn_cast<RequestToServerConnectionOp>(op))
clientNameToServer[std::make_pair(req.getServicePort(),
req.getClientNamePathAttr())]
.push_back(req);

// Find all of the pairs and emit them.
DenseSet<Operation *> emittedOps;
for (auto op : getOps<RequestToServerConnectionOp>()) {
std::pair<hw::InnerRefAttr, ArrayAttr> clientName =
std::make_pair(op.getServicePort(), op.getClientNamePathAttr());
const SmallVector<RequestToServerConnectionOp, 0> &ops =
clientNameToServer[clientName];

// Only emit a pair if there's one toServer and one toClient request for a
// given client name path.
if (ops.size() == 1) {
auto toClientF = clientNameToClient.find(clientName);
if (toClientF != clientNameToClient.end() &&
toClientF->second.size() == 1) {
reqPairs.push_back(
std::make_pair(ops.front(), toClientF->second.front()));
emittedOps.insert(ops.front());
emittedOps.insert(toClientF->second.front());
continue;
}
}
}

// Emit partial pairs for all the remaining requests.
for (auto &op : getOps()) {
if (emittedOps.contains(&op))
continue;
if (auto req = dyn_cast<RequestToClientConnectionOp>(op))
reqPairs.push_back(std::make_pair(nullptr, req));
else if (auto req = dyn_cast<RequestToServerConnectionOp>(op))
reqPairs.push_back(std::make_pair(req, nullptr));
}
}

//===----------------------------------------------------------------------===//
// Structural ops.
//===----------------------------------------------------------------------===//
Expand Down
Loading

0 comments on commit 95c011b

Please sign in to comment.