Skip to content

Commit

Permalink
[ESI] Inline ChannelBufferOptions into ChannelBuffer op.
Browse files Browse the repository at this point in the history
StructAttr was finally removed from upstream MLIR in
d883a02a7c2bb89000d0685749f062c9206ac40c. ESI had a peculiar usage of
StructAttr, ChannelBufferOptions, that did not migrate cleanly to the
new way of defining Attrs with multiple parameters. The issue was that
ChannelBufferOptions acted as a dictionary of attributes for
the ChannelBuffer op, but this adds an extra layer of key-value pair
nesting under the new scheme.

Since the two properties of ChannelBufferOptions were orthogonal, it was
cleaner to just define these as regular, scalar attributes directly on
ChannelBuffer.

This commit inlines these two properties of ChannelBufferOptions
directly onto ChannelBuffer and updates all the ESI code to directly
access the attributes on the ChannelBuffer op. It slightly changes the
assembly format in the case where no attributes are specified, where
instead of writing a pair of empty braces (`{ }`), the braces are
omitted altogether.

This ultimately simplified the custom assembly format parser, however,
since we can just directly use the `parseOptionalAttrDict()` method now.
  • Loading branch information
richardxia committed Jun 23, 2022
1 parent bd185d6 commit 10a4aeb
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 34 deletions.
6 changes: 1 addition & 5 deletions include/circt/Dialect/ESI/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@ add_circt_dialect(ESI esi)
add_circt_dialect_doc(ESI esi)

set(LLVM_TARGET_DEFINITIONS ESI.td)
mlir_tablegen(ESIAttrs.h.inc -gen-struct-attr-decls)
mlir_tablegen(ESIAttrs.cpp.inc -gen-struct-attr-defs)
mlir_tablegen(ESIPasses.h.inc -gen-pass-decls)

add_public_tablegen_target(MLIRESITransformsIncGen)
add_circt_doc(ESI ESIPasses -gen-pass-doc)

add_public_tablegen_target(MLIRESIEnumsIncGen)

add_subdirectory(cosim)
1 change: 0 additions & 1 deletion include/circt/Dialect/ESI/ESIDialect.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ Operation *buildESIWrapper(OpBuilder &b, Operation *mod,
} // namespace esi
} // namespace circt

#include "circt/Dialect/ESI/ESIAttrs.h.inc"
#include "circt/Dialect/ESI/ESIDialect.h.inc"

#endif
21 changes: 9 additions & 12 deletions include/circt/Dialect/ESI/ESIOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,6 @@
//
//===----------------------------------------------------------------------===//

/// Define a struct with all the possible ChannelBuffer options
def ChannelBufferOptions : StructAttr<"ChannelBufferOptions", ESI_Dialect, [
// 'Stages' is used to specify a specific number of cycles (pipeline stages)
// to use on this channel. Must be greater than 0.
StructFieldAttr<"stages", OptionalAttr< Confined<I64Attr, [IntMinValue<1>]> >>,

// Name assigned to a buffered connection.
StructFieldAttr<"name", OptionalAttr< StrAttr >>
]>;

def ChannelBuffer : ESI_Abstract_Op<"buffer", [NoSideEffect]> {
let summary = "Control options for an ESI channel.";
let description = [{
Expand All @@ -30,12 +20,18 @@ def ChannelBuffer : ESI_Abstract_Op<"buffer", [NoSideEffect]> {
This operation is inserted on an ESI dataflow edge. It must exist
previous to SystemVerilog emission but can be added in a lowering pass.

A `stages` attribute may be provided to specify a specific number of cycles
(pipeline stages) to use on this channel. Must be greater than 0.

A `name` attribute may be provided to assigned a name to a buffered
connection.

Example:

```mlir
%esiChan = hw.instance "sender" @Sender () : () -> (!esi.channel<i1>)
// Allow automatic selection of options.
%bufferedChan = esi.buffer %esiChan { } : i1
%bufferedChan = esi.buffer %esiChan : i1
hw.instance "recv" @Reciever (%bufferedChan) : (!esi.channel<i1>) -> ()

// Alternatively, specify the number of stages.
Expand All @@ -44,7 +40,8 @@ def ChannelBuffer : ESI_Abstract_Op<"buffer", [NoSideEffect]> {
}];

let arguments = (ins I1:$clk, I1:$rstn, ChannelType:$input,
ChannelBufferOptions:$options);
OptionalAttr<Confined<I64Attr, [IntMinValue<1>]>>:$stages,
OptionalAttr<StrAttr>:$name);
let results = (outs ChannelType:$output);
let hasCustomAssemblyFormat = 1;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/Dialect/ESI/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ set(ESI_LinkLibs

set(ESI_Deps
${ESI_LinkLibs}
MLIRESIEnumsIncGen
MLIRESITransformsIncGen
)

if(CapnProto_FOUND)
Expand All @@ -48,7 +48,7 @@ add_circt_dialect_library(CIRCTESI
${srcs}

DEPENDS
MLIRESIEnumsIncGen
MLIRESITransformsIncGen
${ESI_Deps}

LINK_COMPONENTS
Expand Down
1 change: 0 additions & 1 deletion lib/Dialect/ESI/ESIDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,5 +330,4 @@ circt::esi::buildESIWrapper(OpBuilder &b, Operation *pearl,
return shell;
}

#include "circt/Dialect/ESI/ESIAttrs.cpp.inc"
#include "circt/Dialect/ESI/ESIDialect.cpp.inc"
9 changes: 1 addition & 8 deletions lib/Dialect/ESI/ESIOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ ParseResult ChannelBuffer::parse(OpAsmParser &parser, OperationState &result) {
/*delimiter=*/OpAsmParser::Delimiter::None))
return failure();

ChannelBufferOptions optionsAttr;
if (parser.parseAttribute(optionsAttr,
parser.getBuilder().getType<NoneType>(), "options",
result.attributes))
return failure();

Type innerOutputType;
if (parser.parseOptionalAttrDict(result.attributes) || parser.parseColon() ||
parser.parseType(innerOutputType))
Expand All @@ -57,8 +51,7 @@ ParseResult ChannelBuffer::parse(OpAsmParser &parser, OperationState &result) {

void ChannelBuffer::print(OpAsmPrinter &p) {
p << " " << clk() << ", " << rstn() << ", " << input() << " ";
p.printAttributeWithoutType(options());
p.printOptionalAttrDict((*this)->getAttrs(), /*elidedAttrs=*/{"options"});
p.printOptionalAttrDict((*this)->getAttrs());
p << " : " << output().getType().cast<ChannelPort>().getInner();
}

Expand Down
5 changes: 2 additions & 3 deletions lib/Dialect/ESI/ESIPasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,18 +295,17 @@ LogicalResult ChannelBufferLowering::matchAndRewrite(
ConversionPatternRewriter &rewriter) const {
auto loc = buffer.getLoc();

ChannelBufferOptions opts = buffer.options();
auto type = buffer.getType();

// Expand 'abstract' buffer into 'physical' stages.
auto stages = opts.stages();
auto stages = buffer.stagesAttr();
uint64_t numStages = 1;
if (stages) {
// Guaranteed positive by the parser.
numStages = stages.getValue().getLimitedValue();
}
Value input = buffer.input();
StringAttr bufferName = buffer.options().name();
StringAttr bufferName = buffer.nameAttr();
for (uint64_t i = 0; i < numStages; ++i) {
// Create the stages, connecting them up as we build.
auto stage = rewriter.create<PipelineStage>(loc, type, buffer.clk(),
Expand Down
4 changes: 2 additions & 2 deletions test/Dialect/ESI/connectivity.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ hw.module @StructRcvr(%a: !esi.channel<!FooStruct>) {

hw.module @test(%clk: i1, %rstn: i1) {
%esiChan = hw.instance "sender" @Sender() -> (x: !esi.channel<i1>)
%bufferedChan = esi.buffer %clk, %rstn, %esiChan { } : i1
%bufferedChan = esi.buffer %clk, %rstn, %esiChan : i1
hw.instance "recv" @Reciever (a: %bufferedChan: !esi.channel<i1>) -> ()

// CHECK: %sender.x = hw.instance "sender" @Sender() -> (x: !esi.channel<i1>)
// CHECK-NEXT: %0 = esi.buffer %clk, %rstn, %sender.x {} : i1
// CHECK-NEXT: %0 = esi.buffer %clk, %rstn, %sender.x : i1
// CHECK-NEXT: hw.instance "recv" @Reciever(a: %0: !esi.channel<i1>) -> ()

%esiChan2 = hw.instance "sender" @Sender() -> (x: !esi.channel<i1>)
Expand Down

0 comments on commit 10a4aeb

Please sign in to comment.