Skip to content

Commit

Permalink
Merge pull request #839 from rdaly525/order-ports
Browse files Browse the repository at this point in the history 
Preserve port order in module instantiation
  • Loading branch information
@leonardt
leonardt authored Feb 21, 2020
2 parents 47cf165 + 4e3e06c commit 077db4a
Show file tree
Hide file tree
Showing 13 changed files with 228 additions and 102 deletions.
1 change: 1 addition & 0 deletions .travis.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ matrix:
homebrew:
packages:
- verilator
update: true
before_install:
# install conda for py 3.7
- |
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ project(verilogAST-download NONE)
include(ExternalProject)
ExternalProject_Add(verilogAST
GIT_REPOSITORY https://github.com/leonardt/verilogAST-cpp.git
GIT_TAG master
GIT_TAG order-ports
SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}/verilogAST-src"
BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/verilogAST-build"
CONFIGURE_COMMAND ""
Expand Down
120 changes: 58 additions & 62 deletions src/passes/analysis/verilog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,12 @@ std::unique_ptr<vAST::Expression> get_primitive_expr(CoreIR::Instance *instance)

std::unique_ptr<vAST::StructuralStatement> inline_binary_op(
std::pair<std::string, CoreIR::Instance *> instance,
std::map<std::string, std::unique_ptr<vAST::Expression>> verilog_connections
) {
BinaryOpReplacer transformer(
std::move(verilog_connections["in0"]),
std::move(verilog_connections["in1"]));
return std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(instance.first + "_out"),
transformer.visit(get_primitive_expr(instance.second)));
std::unique_ptr<vAST::Connections> verilog_connections) {
BinaryOpReplacer transformer(verilog_connections->at("in0"),
verilog_connections->at("in1"));
return std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(instance.first + "_out"),
transformer.visit(get_primitive_expr(instance.second)));
}

bool can_inline_unary_op(CoreIR::Module *module, bool _inline) {
Expand All @@ -151,12 +149,11 @@ bool can_inline_unary_op(CoreIR::Module *module, bool _inline) {

std::unique_ptr<vAST::StructuralStatement> inline_unary_op(
std::pair<std::string, CoreIR::Instance *> instance,
std::map<std::string, std::unique_ptr<vAST::Expression>> verilog_connections
) {
UnaryOpReplacer transformer(std::move(verilog_connections["in"]));
return std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(instance.first + "_out"),
transformer.visit(get_primitive_expr(instance.second)));
std::unique_ptr<vAST::Connections> verilog_connections) {
UnaryOpReplacer transformer(verilog_connections->at("in"));
return std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(instance.first + "_out"),
transformer.visit(get_primitive_expr(instance.second)));
}

bool can_inline_const_op(CoreIR::Module *module, bool _inline) {
Expand Down Expand Up @@ -198,14 +195,13 @@ bool can_inline_mux_op(CoreIR::Module *module, bool _inline) {

std::unique_ptr<vAST::StructuralStatement> inline_mux_op(
std::pair<std::string, CoreIR::Instance *> instance,
std::map<std::string, std::unique_ptr<vAST::Expression>> verilog_connections
) {
MuxReplacer transformer(std::move(verilog_connections["in0"]),
std::move(verilog_connections["in1"]),
std::move(verilog_connections["sel"]));
return std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(instance.first + "_out"),
transformer.visit(get_primitive_expr(instance.second)));
std::unique_ptr<vAST::Connections> verilog_connections) {
MuxReplacer transformer(verilog_connections->at("in0"),
verilog_connections->at("in1"),
verilog_connections->at("sel"));
return std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(instance.first + "_out"),
transformer.visit(get_primitive_expr(instance.second)));
}

bool can_inline_slice_op(CoreIR::Module *module, bool _inline) {
Expand Down Expand Up @@ -332,13 +328,13 @@ declare_connections(std::map<std::string, Instance *> instances) {
std::unique_ptr<vAST::Declaration>>>
wire_declarations;
for (auto instance : instances) {
for (auto port :
cast<RecordType>(instance.second->getModuleRef()->getType())
->getRecord()) {
if (!port.second->isInput()) {
RecordType *record_type =
cast<RecordType>(instance.second->getModuleRef()->getType());
for (auto field : record_type->getFields()) {
Type *field_type = record_type->getRecord().at(field);
if (!field_type->isInput()) {
std::unique_ptr<vAST::Identifier> id =
std::make_unique<vAST::Identifier>(instance.first + "_" +
port.first);
std::make_unique<vAST::Identifier>(instance.first + "_" + field);
// Can't find a simple way to "convert" a variant type to a
// superset, so we just manually unpack it to call the Wire
// constructor
Expand All @@ -347,7 +343,7 @@ declare_connections(std::map<std::string, Instance *> instances) {
wire_declarations.push_back(
std::make_unique<vAST::Wire>(std::move(arg)));
},
process_decl(std::move(id), port.second));
process_decl(std::move(id), field_type));
}
}
}
Expand Down Expand Up @@ -444,26 +440,24 @@ Passes::Verilog::compileStringBodyModule(json verilog_json, std::string name,
std::vector<std::unique_ptr<vAST::AbstractPort>>
Passes::Verilog::compilePorts(RecordType *record_type) {
std::vector<std::unique_ptr<vAST::AbstractPort>> ports;
for (auto entry : record_type->getRecord()) {
std::string name_str = entry.first;
for (auto field : record_type->getFields()) {
Type *field_type = record_type->getRecord().at(field);
std::unique_ptr<vAST::Identifier> name =
std::make_unique<vAST::Identifier>(name_str);

Type *type = entry.second;
std::make_unique<vAST::Identifier>(field);

vAST::Direction verilog_direction;
if (type->isInput()) {
if (field_type->isInput()) {
verilog_direction = vAST::INPUT;
} else if (type->isOutput()) {
} else if (field_type->isOutput()) {
verilog_direction = vAST::OUTPUT;
} else if (type->isInOut()) {
} else if (field_type->isInOut()) {
verilog_direction = vAST::INOUT;
} else {
ASSERT(false, "Not implemented for type = " + toString(type));
ASSERT(false, "Not implemented for type = " + toString(field_type));
}
std::unique_ptr<vAST::Port> port = std::make_unique<vAST::Port>(
process_decl(std::move(name), type), verilog_direction,
vAST::WIRE);
std::unique_ptr<vAST::Port> port =
std::make_unique<vAST::Port>(process_decl(std::move(name), field_type),
verilog_direction, vAST::WIRE);
if (this->verilator_debug) {
port = vAST::AddComment(std::move(port), "verilator public");
}
Expand Down Expand Up @@ -639,26 +633,27 @@ void assign_module_outputs(
std::vector<std::variant<std::unique_ptr<vAST::StructuralStatement>,
std::unique_ptr<vAST::Declaration>>> &body,
std::map<ConnMapKey, std::vector<ConnMapEntry>> connection_map) {
for (auto port : record_type->getRecord()) {
if (port.second->isInput()) {
for (auto field : record_type->getFields()) {
Type *field_type = record_type->getRecord().at(field);
if (field_type->isInput()) {
continue;
}
auto entries = connection_map[ConnMapKey("self", port.first)];
auto entries = connection_map[ConnMapKey("self", field)];
if (entries.size() == 0) {
continue;
} else if (entries.size() > 1) {
std::unique_ptr<vAST::Concat> concat =
convert_non_bulk_connection_to_concat(entries, body, port.first);
convert_non_bulk_connection_to_concat(entries, body, field);
body.push_back(std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(port.first), std::move(concat)));
std::make_unique<vAST::Identifier>(field), std::move(concat)));
} else {
std::unique_ptr<vAST::Expression> verilog_conn =
convert_to_expression(convert_to_verilog_connection(entries[0].source));
process_connection_debug_metadata(entries[0], verilog_conn->toString(), body,
port.first);
field);
// Regular (possibly bulk) connection
body.push_back(std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(port.first),
std::make_unique<vAST::Identifier>(field),
std::move(verilog_conn)
));
}
Expand Down Expand Up @@ -725,32 +720,33 @@ compile_module_body(RecordType *module_type,
body.push_back(std::make_unique<vAST::SingleLineComment>(debug_str));
}

std::map<std::string, std::unique_ptr<vAST::Expression>> verilog_connections;
for (auto port :
cast<RecordType>(instance_module->getType())->getRecord()) {
if (!port.second->isInput()) {
std::unique_ptr<vAST::Connections> verilog_connections =
std::make_unique<vAST::Connections>();
RecordType *record_type = cast<RecordType>(instance_module->getType());
for (auto field : record_type->getFields()) {
Type *field_type = record_type->getRecord().at(field);
if (!field_type->isInput()) {
// output or inout, emit wire name
verilog_connections.insert(
std::make_pair(port.first, std::make_unique<vAST::Identifier>(
instance.first + "_" + port.first)));
verilog_connections->insert(
field,
std::make_unique<vAST::Identifier>(instance.first + "_" + field));
continue;
}
auto entries = connection_map[ConnMapKey(instance.first, port.first)];
auto entries = connection_map[ConnMapKey(instance.first, field)];
if (entries.size() == 0) {
continue;
} else if (entries.size() > 1) {
std::unique_ptr<vAST::Concat> concat =
convert_non_bulk_connection_to_concat(entries, body,
instance_name + "." + port.first);
verilog_connections.insert(
std::make_pair(port.first, std::move(concat)));
instance_name + "." + field);
verilog_connections->insert(field, std::move(concat));
// Otherwise we just use the entry in the connection map
} else {
std::unique_ptr<vAST::Expression> verilog_conn =
convert_to_expression(convert_to_verilog_connection(entries[0].source));
process_connection_debug_metadata(entries[0], verilog_conn->toString(), body,
instance_name + "." + port.first);
verilog_connections.insert(std::make_pair(port.first, std::move(verilog_conn)));
instance_name + "." + field);
verilog_connections->insert(field, std::move(verilog_conn));
}
}
// Handle module arguments
Expand Down Expand Up @@ -793,7 +789,7 @@ compile_module_body(RecordType *module_type,
statement = std::make_unique<vAST::ContinuousAssign>(
std::make_unique<vAST::Identifier>(instance.first + "_out"),
std::make_unique<vAST::Slice>(
std::move(verilog_connections["in"]),
verilog_connections->at("in"),
vAST::make_binop(
std::move(instance_parameters[0].second),
vAST::BinOp::SUB,
Expand Down
2 changes: 1 addition & 1 deletion tests/gtest/array_select_golden.v
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
module foo (input [3:0] I, output [3:0] O);
assign O = I;
endmodule
module top (output [3:0] O, input [3:0] self_I);
module top (input [3:0] self_I, output [3:0] O);
wire [3:0] inst0_O;
wire [3:0] inst1_O;
foo inst0(.I({self_I[2],self_I[1],self_I[0],self_I[0]}), .O(inst0_O));
Expand Down
4 changes: 2 additions & 2 deletions tests/gtest/inline_verilog_golden.v
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ always @(posedge CLK) begin
O <= I;
end
endmodule
module Main (input CLK, input I, output O);
FF FF_inst0(.CLK(CLK), .I(I), .O(O));
module Main (input I, output O, input CLK);
FF FF_inst0(.I(I), .O(O), .CLK(CLK));

assert property { @(posedge CLK) I |-> ##1 O };

Expand Down
2 changes: 1 addition & 1 deletion tests/gtest/mux_golden.v
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ commonlib_muxn__N4__width8 muxN_1(.in_data_0(in_data_4), .in_data_1(in_data_5),
assign out = in_sel[2] ? muxN_1_out : muxN_0_out;
endmodule

module Mux8x8 (input [7:0] I0, input [7:0] I1, input [7:0] I2, input [7:0] I3, input [7:0] I4, input [7:0] I5, input [7:0] I6, input [7:0] I7, output [7:0] O, input [2:0] S);
module Mux8x8 (input [7:0] I0, input [7:0] I1, input [7:0] I2, input [7:0] I3, input [7:0] I4, input [7:0] I5, input [7:0] I6, input [7:0] I7, input [2:0] S, output [7:0] O);
commonlib_muxn__N8__width8 coreir_commonlib_mux8x8_inst0(.in_data_0(I0), .in_data_1(I1), .in_data_2(I2), .in_data_3(I3), .in_data_4(I4), .in_data_5(I5), .in_data_6(I6), .in_data_7(I7), .in_sel(S), .out(O));
endmodule

94 changes: 94 additions & 0 deletions tests/gtest/port_order.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
{"top":"global.test_two_ops",
"namespaces":{
"global":{
"modules":{
"Add8_cin":{
"type":["Record",[
["z",["Array",8,"BitIn"]],
["x",["Array",8,"BitIn"]],
["a",["Array",8,"Bit"]],
["CIN","BitIn"]
]],
"instances":{
"bit_const_0_None":{
"modref":"corebit.const",
"modargs":{"value":["Bool",false]}
},
"inst0":{
"genref":"coreir.add",
"genargs":{"width":["Int",8]}
},
"inst1":{
"genref":"coreir.add",
"genargs":{"width":["Int",8]}
}
},
"connections":[
["inst1.in0.1","bit_const_0_None.out"],
["inst1.in0.2","bit_const_0_None.out"],
["inst1.in0.3","bit_const_0_None.out"],
["inst1.in0.4","bit_const_0_None.out"],
["inst1.in0.5","bit_const_0_None.out"],
["inst1.in0.6","bit_const_0_None.out"],
["inst1.in0.7","bit_const_0_None.out"],
["inst1.out","inst0.in0"],
["self.x","inst0.in1"],
["self.a","inst0.out"],
["self.CIN","inst1.in0.0"],
["self.z","inst1.in1"]
]
},
"Sub8":{
"type":["Record",[
["z",["Array",8,"BitIn"]],
["x",["Array",8,"BitIn"]],
["a",["Array",8,"Bit"]]
]],
"instances":{
"bit_const_1_None":{
"modref":"corebit.const",
"modargs":{"value":["Bool",true]}
},
"inst0":{
"genref":"coreir.not",
"genargs":{"width":["Int",8]}
},
"inst1":{
"modref":"global.Add8_cin"
}
},
"connections":[
["inst1.CIN","bit_const_1_None.out"],
["self.x","inst0.in"],
["inst1.x","inst0.out"],
["self.z","inst1.z"],
["self.a","inst1.a"]
]
},
"test_two_ops":{
"type":["Record",[
["z",["Array",8,"BitIn"]],
["x",["Array",8,"BitIn"]],
["a",["Array",8,"Bit"]]
]],
"instances":{
"inst0":{
"genref":"coreir.add",
"genargs":{"width":["Int",8]}
},
"inst1":{
"modref":"global.Sub8"
}
},
"connections":[
["self.z","inst0.in0"],
["self.x","inst0.in1"],
["inst1.z","inst0.out"],
["self.z","inst1.x"],
["self.a","inst1.a"]
]
}
}
}
}
}
12 changes: 12 additions & 0 deletions tests/gtest/port_order_golden.v
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module Add8_cin (input [7:0] z, input [7:0] x, output [7:0] a, input CIN);
assign a = (({1'b0,1'b0,1'b0,1'b0,1'b0,1'b0,1'b0,CIN}) + z) + x;
endmodule

module Sub8 (input [7:0] z, input [7:0] x, output [7:0] a);
Add8_cin inst1(.z(z), .x(~ x), .a(a), .CIN(1'b1));
endmodule

module test_two_ops (input [7:0] z, input [7:0] x, output [7:0] a);
Sub8 inst1(.z(z + x), .x(z), .a(a));
endmodule

Loading

0 comments on commit 077db4a

Please sign in to comment.