Skip to content

Commit 6e74077

Browse files
povikpovik
committed
syn: Fix port order confusion with macros
Signed-off-by: Martin Povišer <povik@cutebit.org>
1 parent 26800e4 commit 6e74077

7 files changed

Lines changed: 235 additions & 13 deletions

File tree

‎src/syn/src/elab/driver.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ std::unique_ptr<Graph> elaborate(utl::Logger* logger,
3232
const std::vector<std::string>& args,
3333
sta::dbSta* sta = nullptr);
3434
std::unique_ptr<Graph> elaborateText(const std::string& source,
35-
const std::vector<std::string>& args = {});
35+
const std::vector<std::string>& args = {},
36+
sta::dbSta* sta = nullptr);
3637

3738
} // namespace syn

‎src/syn/src/elab/error.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ std::unique_ptr<Graph> elaborateImpl(utl::Logger* logger,
2929
std::optional<std::string> buffer);
3030

3131
std::unique_ptr<Graph> elaborateText(const std::string& source,
32-
const std::vector<std::string>& args)
32+
const std::vector<std::string>& args,
33+
sta::dbSta* sta)
3334
{
3435
utl::Logger logger;
35-
return elaborateImpl(&logger, args, nullptr, source);
36+
return elaborateImpl(&logger, args, sta, source);
3637
}
3738

3839
} // namespace syn

‎src/syn/src/flow/import.cc‎

Lines changed: 78 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
#include <cstdint>
77
#include <cstdlib>
8+
#include <map>
9+
#include <string_view>
810
#include <utility>
911

1012
#include "sta/Liberty.hh"
@@ -31,7 +33,8 @@ void importTargets(Graph& g, sta::Network* network, utl::Logger* logger)
3133
return;
3234
}
3335

34-
// Build expected input/output widths from liberty cell ports.
36+
std::map<std::string_view, uint32_t> lib_input_offset;
37+
std::map<std::string_view, uint32_t> lib_output_offset;
3538
uint32_t lib_input_width = 0;
3639
uint32_t lib_output_width = 0;
3740
sta::LibertyCellPortIterator port_iter(cell);
@@ -42,10 +45,14 @@ void importTargets(Graph& g, sta::Network* network, utl::Logger* logger)
4245
}
4346
sta::PortDirection* dir = port->direction();
4447
if (dir->isInput()) {
48+
lib_input_offset[port->name()] = lib_input_width;
4549
lib_input_width += port->size();
4650
} else if (dir->isOutput()) {
51+
lib_output_offset[port->name()] = lib_output_width;
4752
lib_output_width += port->size();
4853
} else if (dir->isBidirect()) {
54+
lib_input_offset[port->name()] = lib_input_width;
55+
lib_output_offset[port->name()] = lib_output_width;
4956
lib_input_width += port->size();
5057
lib_output_width += port->size();
5158
}
@@ -66,8 +73,7 @@ void importTargets(Graph& g, sta::Network* network, utl::Logger* logger)
6673
logger->error(utl::SYN,
6774
10,
6875
"importTargets: cell '{}' input width mismatch:"
69-
" Other has {} bits, Liberty has {} bits. This is an "
70-
"internal tool error.",
76+
" Verilog instantiation has {} bits, Liberty has {} bits.",
7177
other->cellType(),
7278
other_input_width,
7379
lib_input_width);
@@ -77,26 +83,89 @@ void importTargets(Graph& g, sta::Network* network, utl::Logger* logger)
7783
logger->error(utl::SYN,
7884
11,
7985
"importTargets: cell '{}' output width mismatch:"
80-
" Other has {} bits, Liberty has {} bits. This is an "
81-
"internal tool error.",
86+
" Verilog instantiation has {} bits, Liberty has {} bits.",
8287
other->cellType(),
8388
other_output_width,
8489
lib_output_width);
8590
}
8691

8792
// Concatenate all input port values into a single Bundle.
88-
Bundle inputs;
93+
Bundle inputs = Bundle::sentinel(lib_input_width);
8994
for (auto& port : other->ports()) {
9095
if (port.direction == Other::Port::kInput
9196
|| port.direction == Other::Port::kInOut) {
92-
inputs = inputs.concat(port.value);
97+
sta::LibertyPort* lib_port = cell->findLibertyPort(port.name);
98+
if (!lib_port || !lib_input_offset.contains(port.name)) {
99+
logger->error(
100+
utl::SYN,
101+
35,
102+
"importTargets: cell '{}' Liberty definition is missing "
103+
"port '{}' connected in Verilog instantiation.",
104+
other->cellType(),
105+
port.name);
106+
}
107+
108+
if (((uint32_t) lib_port->size()) != port.width) {
109+
logger->error(
110+
utl::SYN,
111+
36,
112+
"importTargets: cell '{}' instantiated with mismatched width "
113+
"of port '{}': {} in Verilog vs {} in Liberty",
114+
other->cellType(),
115+
port.name,
116+
port.width,
117+
lib_port->size());
118+
}
119+
120+
uint32_t offset = lib_input_offset.at(port.name);
121+
for (uint32_t i = 0; i < port.width; i++) {
122+
inputs.mutableNet(offset + i) = port.value[i];
123+
}
93124
}
94125
}
95126

96127
// Create Target and replace outputs.
97128
BundleView old_output = g.output(other);
98-
Bundle new_output = g.add<Target>(cell, std::move(inputs));
99-
g.forceReplace(old_output, BundleView(new_output));
129+
Bundle target_output = g.add<Target>(cell, std::move(inputs));
130+
Bundle output_replacement = Bundle::sentinel(lib_output_width);
131+
132+
uint32_t offset = 0;
133+
for (auto& port : other->ports()) {
134+
if (port.direction == Other::Port::kOutput
135+
|| port.direction == Other::Port::kInOut) {
136+
sta::LibertyPort* lib_port = cell->findLibertyPort(port.name);
137+
if (!lib_port || !lib_output_offset.contains(port.name)) {
138+
logger->error(
139+
utl::SYN,
140+
37,
141+
"importTargets: cell '{}' Liberty definition is missing "
142+
"port '{}' connected in Verilog instantiation.",
143+
other->cellType(),
144+
port.name);
145+
}
146+
147+
if (((uint32_t) lib_port->size()) != port.width) {
148+
logger->error(
149+
utl::SYN,
150+
38,
151+
"importTargets: cell '{}' instantiated with mismatched width "
152+
"of port '{}': {} in Verilog vs {} in Liberty",
153+
other->cellType(),
154+
port.name,
155+
port.width,
156+
lib_port->size());
157+
}
158+
159+
uint32_t lib_offset = lib_output_offset.at(port.name);
160+
for (uint32_t i = 0; i < port.width; i++) {
161+
output_replacement.mutableNet(offset + i)
162+
= target_output[lib_offset + i];
163+
}
164+
offset += port.width;
165+
}
166+
}
167+
168+
g.forceReplace(old_output, BundleView(output_replacement));
100169
g.removeInstance(
101170
const_cast<Instance*>(static_cast<const Instance*>(other)));
102171
});

‎src/syn/test/BUILD‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,12 @@ cc_test(
115115
"elab_testcases.h",
116116
"equivalence_check.h",
117117
],
118+
data = ["macro_test_cells.lib"],
118119
deps = [
120+
"//src/syn",
119121
"//src/syn/src/elab",
120122
"//src/syn/src/ir",
123+
"//src/tst",
121124
"@googletest//:gtest",
122125
],
123126
)

‎src/syn/test/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,6 @@ foreach(_t abc_test sm_test cm_test)
4040
endforeach()
4141

4242
add_executable(elab_test elab_test.cc)
43-
target_link_libraries(elab_test ${_syn_test_libs} syn_elab syn_ir)
43+
target_link_libraries(elab_test ${_syn_test_libs} syn_elab syn_flow syn_ir tst)
4444
gtest_discover_tests(elab_test WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
4545
add_dependencies(build_and_test elab_test)

‎src/syn/test/elab_test.cc‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@
1111
#include "driver.h"
1212
#include "elab_testcases.h"
1313
#include "equivalence_check.h"
14+
#include "flow/import.h"
1415
#include "gtest/gtest.h"
1516
#include "syn/ir/Bundle.h"
1617
#include "syn/ir/Graph.h"
1718
#include "syn/ir/Instance.h"
1819
#include "syn/ir/TritModel.h"
20+
#include "syn/synthesis.h"
21+
#include "tst/fixture.h"
1922

2023
int main(int argc, char** argv)
2124
{
@@ -289,4 +292,82 @@ INSTANTIATE_TEST_SUITE_P(Elab,
289292
ElabSlangTest,
290293
::testing::ValuesIn(elabTestcases));
291294

295+
class ElabWithMacrosTest : public tst::Fixture
296+
{
297+
protected:
298+
void SetUp() override
299+
{
300+
Fixture::SetUp();
301+
readLiberty(getFilePath("_main/src/syn/test/macro_test_cells.lib"));
302+
}
303+
};
304+
305+
TEST_F(ElabWithMacrosTest, BlackboxImport)
306+
{
307+
// Check macro connections are correct even when Verilog blackbox
308+
// module does not match Liberty port order
309+
auto result = syn::elaborateText(R"(
310+
module top(
311+
input logic clk,
312+
input logic ce,
313+
input logic we,
314+
input logic [8:0] addr,
315+
input logic [7:0] wd,
316+
output logic [7:0] rd
317+
);
318+
fakeram_512x8 u_ram (
319+
.rd_out(rd),
320+
.addr_in(addr),
321+
.we_in(we),
322+
.wd_in(wd),
323+
.clk(clk),
324+
.ce_in(ce)
325+
);
326+
endmodule
327+
328+
(* blackbox *)
329+
module fakeram_512x8 (
330+
output logic [7:0] rd_out,
331+
input logic [8:0] addr_in,
332+
input logic we_in,
333+
input logic [7:0] wd_in,
334+
input logic clk,
335+
input logic ce_in
336+
);
337+
endmodule
338+
)",
339+
{"--top", "top"},
340+
getSta());
341+
ASSERT_TRUE(result) << "Elaboration failed";
342+
343+
syn::Graph& g = *result;
344+
g.normalize();
345+
importTargets(g, getSta()->network(), getLogger());
346+
std::ostringstream os;
347+
g.dump(os);
348+
EXPECT_EQ(os.str(),
349+
R"(%3:0 = name "clk" 0 1 %9
350+
%4:0 = name "ce" 0 1 %10
351+
%5:0 = name "we" 0 1 %11
352+
%6:0 = name "addr" 0 9 vector %12:9
353+
%7:0 = name "wd" 0 8 vector %21:8
354+
%8:0 = name "rd" 0 8 vector %30:8
355+
%9:1 = input "clk"
356+
%10:1 = input "ce"
357+
%11:1 = input "we"
358+
%12:9 = input "addr"
359+
%21:8 = input "wd"
360+
%29:0 = output "rd" %30:8
361+
%30:8 = buf %38:8
362+
%38:8 = target "fakeram_512x8" {
363+
input "clk" = %9
364+
%38:8 = output "rd_out"
365+
input "we_in" = %11
366+
input "ce_in" = %10
367+
input "addr_in" = %12:9
368+
input "wd_in" = %21:8
369+
}
370+
)");
371+
}
372+
292373
} // namespace syn

‎src/syn/test/macro_test_cells.lib‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
library(macro_test) {
2+
delay_model : table_lookup;
3+
time_unit : "1ns";
4+
voltage_unit : "1V";
5+
current_unit : "1mA";
6+
capacitive_load_unit(1, pf);
7+
type (fakeram_512x8_DATA) {
8+
base_type : array ;
9+
data_type : bit ;
10+
bit_width : 8;
11+
bit_from : 7;
12+
bit_to : 0 ;
13+
downto : true ;
14+
}
15+
type (fakeram_512x8_ADDRESS) {
16+
base_type : array ;
17+
data_type : bit ;
18+
bit_width : 9;
19+
bit_from : 8;
20+
bit_to : 0 ;
21+
downto : true ;
22+
}
23+
cell(fakeram_512x8) {
24+
area : 171.993;
25+
interface_timing : true;
26+
memory() {
27+
type : ram;
28+
address_width : 9;
29+
word_width : 8;
30+
}
31+
pin(clk) {
32+
direction : input;
33+
capacitance : 0.025;
34+
clock : true;
35+
}
36+
bus(rd_out) {
37+
bus_type : fakeram_512x8_DATA;
38+
direction : output;
39+
max_capacitance : 0.500;
40+
memory_read() {
41+
address : addr_in;
42+
}
43+
}
44+
pin(we_in) {
45+
direction : input;
46+
capacitance : 0.005;
47+
}
48+
pin(ce_in) {
49+
direction : input;
50+
capacitance : 0.005;
51+
}
52+
bus(addr_in) {
53+
bus_type : fakeram_512x8_ADDRESS;
54+
direction : input;
55+
capacitance : 0.005;
56+
}
57+
bus(wd_in) {
58+
bus_type : fakeram_512x8_DATA;
59+
memory_write() {
60+
address : addr_in;
61+
clocked_on : "clk";
62+
}
63+
direction : input;
64+
capacitance : 0.005;
65+
}
66+
}
67+
}

0 commit comments

Comments
 (0)