-
Notifications
You must be signed in to change notification settings - Fork 48
[Operations] Replace top-level join and blocker with synchronizer #557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 14 commits
4f8b591
f80bd64
e112e15
f63cdeb
73be3ad
2a9aa91
224f467
510fb71
362120a
7dabd97
3d7fe36
0a54dcb
f1a6256
ff5d26c
4a5eb6e
3c5949d
07a46f0
c702ecf
1b8607b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -506,27 +506,27 @@ LogicalResult ftd::createPhiNetworkDeps( | |
| continue; | ||
| } | ||
|
|
||
| // If the operand has one dependency only, there is no need for a join. | ||
| // If the operand has one dependency only, there is no need for a synchronizer. | ||
| if (dependencies.size() == 1) { | ||
| if (failed(connect(operand, dependencies[0]))) | ||
| return failure(); | ||
| continue; | ||
| } | ||
|
|
||
| // If the operand has many dependencies, then each of them is singularly | ||
| // connected with an SSA network, and then everything is joined. | ||
| // connected with an SSA network, and then everything is synchronized. | ||
| ValueRange operands = dependencies; | ||
| rewriter.setInsertionPointToStart(operand->getOwner()->getBlock()); | ||
| auto joinOp = rewriter.create<handshake::JoinOp>( | ||
| auto synchronizerOp = rewriter.create<handshake::SynchronizerOp>( | ||
| operand->getOwner()->getLoc(), operands); | ||
| joinOp->moveBefore(operandOwner); | ||
| synchronizerOp->moveBefore(operandOwner); | ||
|
|
||
| for (unsigned i = 0; i < dependencies.size(); i++) { | ||
| if (failed(connect(&joinOp->getOpOperand(i), dependencies[i]))) | ||
| if (failed(connect(&synchronizerOp->getOpOperand(i), dependencies[i]))) | ||
| return failure(); | ||
| } | ||
|
|
||
| operand->set(joinOp.getResult()); | ||
| operand->set(synchronizerOp.getOuts()[0]); | ||
|
Comment on lines
516
to
+529
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. synchronizer can easily replace join, you just take the first output, second output is sinked |
||
| } | ||
|
|
||
| return success(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -489,12 +489,12 @@ createElasticMiter(MLIRContext &context, ModuleOp lhsModule, ModuleOp rhsModule, | |
| setHandshakeAttributes(builder, rhsEndBufferOp, BB_OUT, rhsBufName); | ||
|
|
||
| if (lhsResult.getType().isa<handshake::ControlType>()) { | ||
| ValueRange joinInputs = {lhsEndBufferOp.getResult(), | ||
| ValueRange synchronizerInputs = {lhsEndBufferOp.getResult(), | ||
| rhsEndBufferOp.getResult()}; | ||
| JoinOp joinOp = | ||
| builder.create<JoinOp>(builder.getUnknownLoc(), joinInputs); | ||
| setHandshakeAttributes(builder, joinOp, BB_OUT, eqName); | ||
| miterResultValues.push_back(joinOp.getResult()); | ||
| SynchronizerOp synchronizerOp = | ||
| builder.create<SynchronizerOp>(builder.getUnknownLoc(), synchronizerInputs); | ||
| setHandshakeAttributes(builder, synchronizerOp, BB_OUT, eqName); | ||
| miterResultValues.push_back(synchronizerOp.getOuts()[0]); | ||
|
Comment on lines
491
to
+497
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. join replacement here |
||
| } else { | ||
| CmpIOp compOp = builder.create<CmpIOp>( | ||
| builder.getUnknownLoc(), CmpIPredicate::eq, | ||
|
|
||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will add comment of example output |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| from generators.handshake.join import generate_join | ||
| from generators.handshake.fork import generate_fork | ||
| from generators.support.utils import data | ||
|
|
||
|
|
||
| def generate_synchronizer(name, params): | ||
| size = params["size"] | ||
|
|
||
| bitwidths = [] | ||
| for i in range(size): | ||
| bitwidths.append(params[f"bitwidth_{i}"]) | ||
|
|
||
| input_ports = "" | ||
| for i, bitwidth in enumerate(bitwidths): | ||
| input_ports += input_port(i, bitwidth) | ||
| input_ports = input_ports.lstrip() | ||
|
|
||
| output_ports = "" | ||
| for i, bitwidth in enumerate(bitwidths): | ||
| output_ports += output_port(i, bitwidth) | ||
| output_ports = output_ports.lstrip().rpartition(";")[0].rstrip() | ||
|
|
||
| join_name = f"{name}_join" | ||
| fork_name = f"{name}_fork" | ||
|
|
||
| dependencies = generate_join(join_name, {"size": size}) + \ | ||
| generate_fork(fork_name, {"size": size, "bitwidth": 0}) | ||
|
|
||
|
|
||
| entity = f""" | ||
| library ieee; | ||
| use ieee.std_logic_1164.all; | ||
|
|
||
| -- Entity of synchronizer | ||
| entity {name} is | ||
| port ( | ||
| {input_ports} | ||
| {output_ports} | ||
| ); | ||
| end entity; | ||
| """ | ||
|
|
||
| valid_instance_assignments = "" | ||
| for i in range(size): | ||
| valid_instance_assignments += f" ins_valid({i}) => ins{i}_valid,\n" | ||
|
|
||
| for i in range(size): | ||
| valid_instance_assignments += (f" ins_ready({i}) => ins{i}_ready,\n") | ||
|
|
||
| valid_instance_assignments = valid_instance_assignments.lstrip() | ||
|
|
||
|
|
||
| fork_instance_assignments = "" | ||
| for i in range(size): | ||
| fork_instance_assignments += f" outs_valid({i}) => outs{i}_valid,\n" | ||
|
|
||
| for i in range(size): | ||
| fork_instance_assignments += (f" outs_ready({i}) => outs{i}_ready,\n") | ||
|
|
||
| fork_instance_assignments = fork_instance_assignments.lstrip() | ||
|
|
||
| assignments = "" | ||
| for i, bitwidth in enumerate(bitwidths): | ||
| potential_assignment = f" outs{i} <= ins{i};\n" | ||
| assignments += data(potential_assignment, bitwidth) | ||
| assignments = assignments.lstrip() | ||
|
|
||
|
|
||
| architecture = f""" | ||
| -- Architecture of synchronizer | ||
| architecture arch of {name} is | ||
| signal join_valid : std_logic; | ||
| signal fork_ready : std_logic; | ||
| begin | ||
| join : entity work.{join_name} | ||
| port map( | ||
| -- input channels | ||
| {valid_instance_assignments} | ||
| -- output channel to eager fork | ||
| outs_valid => join_valid, | ||
| outs_ready => fork_ready | ||
| ); | ||
|
|
||
| fork : entity work.{fork_name} | ||
| port map( | ||
| -- output channels | ||
| {fork_instance_assignments} | ||
| -- input channel from fork | ||
| ins_valid => join_valid, | ||
| ins_ready => fork_ready | ||
| ); | ||
|
|
||
| {assignments} | ||
|
|
||
| end architecture; | ||
| """ | ||
|
|
||
|
|
||
| return dependencies + entity + architecture | ||
|
|
||
|
|
||
| def input_port(i, bitwidth): | ||
| potential_port_declaration = f"ins{i} : in std_logic_vector({bitwidth} - 1 downto 0);" | ||
| return f""" -- input port {i} | ||
| {data(potential_port_declaration, bitwidth)} | ||
| ins{i}_valid : in std_logic; | ||
| ins{i}_ready : out std_logic; | ||
|
|
||
| """ | ||
|
|
||
| def output_port(i, bitwidth): | ||
| potential_port_declaration = f"outs{i} : out std_logic_vector({bitwidth} - 1 downto 0);" | ||
| return f""" -- output port {i} | ||
| {data(potential_port_declaration, bitwidth)} | ||
| outs{i}_valid : out std_logic; | ||
| outs{i}_ready : in std_logic; | ||
|
|
||
| """ |
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to add custom naming to this so that it has operands of ins0, ins1, ins2 instead of ins_0, ins_1, ins_2. sharing wrapper also uses this trick to have a variable number of operands but 1d port assignments. ultimately we should make this be set in a sensible matter, but we are stuck with what we have for now
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to add custom type verification to this to make sure each output has the bitwidth of its corresponding input |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -591,47 +591,32 @@ def SourceOp : Handshake_Op<"source", [Pure]> { | |
| let assemblyFormat = "attr-dict `:` type($result)"; | ||
| } | ||
|
|
||
| def JoinOp : Handshake_Op<"join", [ | ||
| SameOperandsAndResultType, | ||
| DeclareOpInterfaceMethods<ControlInterface, ["isControl"]> | ||
| ]> { | ||
| let summary = "join operation"; | ||
| def SynchronizerOp : Handshake_Op<"synchronizer"> { | ||
| let summary = "Synchronization operation"; | ||
| let description = [{ | ||
| A control-only synchronizer. Produces a valid output when all | ||
| inputs become available. | ||
| Token synchronizer. | ||
| Outputs only become valid when all inputs are valid. | ||
|
|
||
| Example: | ||
|
|
||
| ```mlir | ||
| %0 = join %a, %b, %c : !handshake.control | ||
| %0, %1, %2 = synchronizer %a, %b, %c : !handshake.channel<i32>, !handshake.channel<i8>, !handshake.control | ||
| ``` | ||
| }]; | ||
|
|
||
| let arguments = (ins Variadic<ControlType>:$data); | ||
| let results = (outs ControlType:$result); | ||
| let assemblyFormat = "$data attr-dict `:` type($result)"; | ||
| } | ||
|
|
||
| // TODO: Split the input into two parts and explicitly mark the forwarded input, | ||
| // once the backend supports variadic channels with zero or one item correctly. | ||
| def BlockerOp : Handshake_Op<"blocker", [ | ||
| SameOperandsAndResultType | ||
| ]> { | ||
| let summary = "blocker operation"; | ||
| let description = [{ | ||
| A data synchronizer. Blocks handshakes until all inputs are valid, | ||
| then forwards the first input. | ||
|
|
||
| Example: | ||
| let arguments = (ins Variadic<ChannelType>:$ins); | ||
| let results = (outs Variadic<ChannelType>:$outs); | ||
|
|
||
| ```mlir | ||
| %0 = blocker %a, %b, %c : !handshake.channel<i32> | ||
| ``` | ||
| }]; | ||
| let builders = [ | ||
| OpBuilder<(ins "ValueRange":$ins), [{ | ||
| SmallVector<Type> outTypes; | ||
| for (Value v : ins) | ||
| outTypes.push_back(v.getType()); | ||
| build($_builder, $_state, outTypes, ins); | ||
| }]> | ||
| ]; | ||
|
Comment on lines
+615
to
+622
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shundroid custom builder to infer out types from in types |
||
|
|
||
| let arguments = (ins Variadic<ChannelType>:$data); | ||
| let results = (outs ChannelType:$result); | ||
| let assemblyFormat = "$data attr-dict `:` type($result)"; | ||
| let assemblyFormat = "$ins `:` type($ins) attr-dict `->` type($outs)"; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure if this is the best format, we don't have any other super varying input types like this, but apparently this is a common format? |
||
| } | ||
|
|
||
| def NotOp : Handshake_Op<"not", [Pure, SameOperandsAndResultType]> { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -537,7 +537,7 @@ ModuleDiscriminator::ModuleDiscriminator(Operation *op) { | |
| addUnsigned("SIZE", op->getNumOperands()); | ||
| addType("DATA_TYPE", op->getResult(0)); | ||
| }) | ||
| .Case<handshake::JoinOp, handshake::BlockerOp>([&](auto) { | ||
| .Case<handshake::SynchronizerOp>([&](auto) { | ||
| // Number of input channels | ||
| addUnsigned("SIZE", op->getNumOperands()); | ||
| }) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blocker op has a bug here since it doesnt register datatype which will break phase 2 matching @AyaElAkhras , will add this later also |
||
|
|
@@ -1821,8 +1821,7 @@ class HandshakeToHWPass | |
| ConvertToHWInstance<handshake::MergeOp>, | ||
| ConvertToHWInstance<handshake::ControlMergeOp>, | ||
| ConvertToHWInstance<handshake::MuxOp>, | ||
| ConvertToHWInstance<handshake::JoinOp>, | ||
| ConvertToHWInstance<handshake::BlockerOp>, | ||
| ConvertToHWInstance<handshake::SynchronizerOp>, | ||
| ConvertToHWInstance<handshake::SourceOp>, | ||
| ConvertToHWInstance<handshake::ConstantOp>, | ||
| ConvertToHWInstance<handshake::SinkOp>, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is needed for synchronizer is easy to add, but this is a separate thing- we never added blocker to this