From c00746316854566d82694a5ab1811ebb9bb7685d Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 7 Apr 2020 13:48:21 -0400 Subject: [PATCH 1/6] Change timing database lookup based on REGMODE value --- ecp5/arch.cc | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/ecp5/arch.cc b/ecp5/arch.cc index 0a44e020..548b8d4f 100644 --- a/ecp5/arch.cc +++ b/ecp5/arch.cc @@ -1071,13 +1071,28 @@ TimingClockingInfo Arch::getPortClockingInfo(const CellInfo *cell, IdString port "INV") ? FALLING_EDGE : RISING_EDGE; + + // REGMODE determines some timing parameters + auto regmode_a = str_or_default(cell->params, id("REGMODE_A"), "NOREG"); + auto regmode_b = str_or_default(cell->params, id("REGMODE_B"), "NOREG"); + nextpnr_ecp5::IdString regmode_timing; + if (regmode_a == "NOREG" && regmode_b == "NOREG") { + regmode_timing = id_DP16KD_REGMODE_A_NOREG_REGMODE_B_NOREG; + } else if (regmode_a == "NOREG" && regmode_b == "OUTREG") { + regmode_timing = id_DP16KD_REGMODE_A_NOREG_REGMODE_B_OUTREG; + } else if (regmode_a == "OUTREG" && regmode_b == "NOREG") { + regmode_timing = id_DP16KD_REGMODE_A_OUTREG_REGMODE_B_NOREG; + } else if (regmode_a == "OUTREG" && regmode_b == "OUTREG") { + regmode_timing = id_DP16KD_REGMODE_A_OUTREG_REGMODE_B_OUTREG; + } else { + NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE configuration: " + regmode_a + ", " + regmode_b); + } + if (cell->ports.at(port).type == PORT_OUT) { - bool is_path = getDelayFromTimingDatabase(id_DP16KD_REGMODE_A_NOREG_REGMODE_B_NOREG, half_clock, port, - info.clockToQ); + bool is_path = getDelayFromTimingDatabase(regmode_timing, half_clock, port, info.clockToQ); NPNR_ASSERT(is_path); } else { - getSetupHoldFromTimingDatabase(id_DP16KD_REGMODE_A_NOREG_REGMODE_B_NOREG, half_clock, port, info.setup, - info.hold); + getSetupHoldFromTimingDatabase(regmode_timing, half_clock, port, info.setup, info.hold); } } else if (cell->type == id_DCUA) { std::string prefix = port.str(this).substr(0, 9); From 0bdf1e05f126e01adcea3f11d1d76f3235ab44e8 Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 7 Apr 2020 14:03:55 -0400 Subject: [PATCH 2/6] Extract regmode configuration to ArchInfo --- ecp5/arch.cc | 12 ++++-------- ecp5/archdefs.h | 2 ++ ecp5/pack.cc | 10 ++++++++++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/ecp5/arch.cc b/ecp5/arch.cc index 548b8d4f..e3b8d65a 100644 --- a/ecp5/arch.cc +++ b/ecp5/arch.cc @@ -1073,19 +1073,15 @@ TimingClockingInfo Arch::getPortClockingInfo(const CellInfo *cell, IdString port : RISING_EDGE; // REGMODE determines some timing parameters - auto regmode_a = str_or_default(cell->params, id("REGMODE_A"), "NOREG"); - auto regmode_b = str_or_default(cell->params, id("REGMODE_B"), "NOREG"); nextpnr_ecp5::IdString regmode_timing; - if (regmode_a == "NOREG" && regmode_b == "NOREG") { + if (!cell->ramInfo.output_a_registered && !cell->ramInfo.output_b_registered) { regmode_timing = id_DP16KD_REGMODE_A_NOREG_REGMODE_B_NOREG; - } else if (regmode_a == "NOREG" && regmode_b == "OUTREG") { + } else if (!cell->ramInfo.output_a_registered && cell->ramInfo.output_b_registered) { regmode_timing = id_DP16KD_REGMODE_A_NOREG_REGMODE_B_OUTREG; - } else if (regmode_a == "OUTREG" && regmode_b == "NOREG") { + } else if (cell->ramInfo.output_a_registered && !cell->ramInfo.output_b_registered) { regmode_timing = id_DP16KD_REGMODE_A_OUTREG_REGMODE_B_NOREG; - } else if (regmode_a == "OUTREG" && regmode_b == "OUTREG") { + } else if (cell->ramInfo.output_a_registered && cell->ramInfo.output_b_registered) { regmode_timing = id_DP16KD_REGMODE_A_OUTREG_REGMODE_B_OUTREG; - } else { - NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE configuration: " + regmode_a + ", " + regmode_b); } if (cell->ports.at(port).type == PORT_OUT) { diff --git a/ecp5/archdefs.h b/ecp5/archdefs.h index b176eec0..5312045c 100644 --- a/ecp5/archdefs.h +++ b/ecp5/archdefs.h @@ -180,6 +180,8 @@ struct ArchCellInfo struct { bool is_pdp; + bool output_a_registered; + bool output_b_registered; } ramInfo; }; diff --git a/ecp5/pack.cc b/ecp5/pack.cc index f5e8a544..16728af4 100644 --- a/ecp5/pack.cc +++ b/ecp5/pack.cc @@ -3004,6 +3004,16 @@ void Arch::assignArchInfo() ci->sliceInfo.has_l6mux = true; } else if (ci->type == id_DP16KD) { ci->ramInfo.is_pdp = (int_or_default(ci->params, id("DATA_WIDTH_A"), 0) == 36); + + // Output register mode (REGMODE_{A,B}). Valid options are 'NOREG' and 'OUTREG'. + std::string regmode_a = str_or_default(ci->params, id("REGMODE_A"), "NOREG"); + if (!(regmode_a == "NOREG" || regmode_a == "OUTREG")) + NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE_A configuration '" + regmode_a + "'"); + std::string regmode_b = str_or_default(ci->params, id("REGMODE_B"), "NOREG"); + if (!(regmode_b == "NOREG" || regmode_b == "OUTREG")) + NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE_B configuration '" + regmode_b + "'"); + ci->ramInfo.output_a_registered = regmode_a == "OUTREG"; + ci->ramInfo.output_b_registered = regmode_b == "OUTREG"; } } for (auto net : sorted(nets)) { From 3257bdc8a15077e8e8ad9285f98e8d9e1e5cae07 Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 7 Apr 2020 14:11:02 -0400 Subject: [PATCH 3/6] Actually just move all the logic to ArchInfo --- ecp5/arch.cc | 17 ++--------------- ecp5/archdefs.h | 10 ++++++++-- ecp5/pack.cc | 15 +++++++++++++-- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/ecp5/arch.cc b/ecp5/arch.cc index e3b8d65a..39d2ba17 100644 --- a/ecp5/arch.cc +++ b/ecp5/arch.cc @@ -1071,24 +1071,11 @@ TimingClockingInfo Arch::getPortClockingInfo(const CellInfo *cell, IdString port "INV") ? FALLING_EDGE : RISING_EDGE; - - // REGMODE determines some timing parameters - nextpnr_ecp5::IdString regmode_timing; - if (!cell->ramInfo.output_a_registered && !cell->ramInfo.output_b_registered) { - regmode_timing = id_DP16KD_REGMODE_A_NOREG_REGMODE_B_NOREG; - } else if (!cell->ramInfo.output_a_registered && cell->ramInfo.output_b_registered) { - regmode_timing = id_DP16KD_REGMODE_A_NOREG_REGMODE_B_OUTREG; - } else if (cell->ramInfo.output_a_registered && !cell->ramInfo.output_b_registered) { - regmode_timing = id_DP16KD_REGMODE_A_OUTREG_REGMODE_B_NOREG; - } else if (cell->ramInfo.output_a_registered && cell->ramInfo.output_b_registered) { - regmode_timing = id_DP16KD_REGMODE_A_OUTREG_REGMODE_B_OUTREG; - } - if (cell->ports.at(port).type == PORT_OUT) { - bool is_path = getDelayFromTimingDatabase(regmode_timing, half_clock, port, info.clockToQ); + bool is_path = getDelayFromTimingDatabase(cell->ramInfo.regmode_timing_id, half_clock, port, info.clockToQ); NPNR_ASSERT(is_path); } else { - getSetupHoldFromTimingDatabase(regmode_timing, half_clock, port, info.setup, info.hold); + getSetupHoldFromTimingDatabase(cell->ramInfo.regmode_timing_id, half_clock, port, info.setup, info.hold); } } else if (cell->type == id_DCUA) { std::string prefix = port.str(this).substr(0, 9); diff --git a/ecp5/archdefs.h b/ecp5/archdefs.h index 5312045c..0f197345 100644 --- a/ecp5/archdefs.h +++ b/ecp5/archdefs.h @@ -180,8 +180,14 @@ struct ArchCellInfo struct { bool is_pdp; - bool output_a_registered; - bool output_b_registered; + // Are the outputs from a DP16KD registered (OUTREG) + // or non-registered (NOREG) + bool is_output_a_registered; + bool is_output_b_registered; + // Which timing information to use for a DP16KD. Depends on registering + // configuration. + nextpnr_ecp5::IdString regmode_timing_id; + } ramInfo; }; diff --git a/ecp5/pack.cc b/ecp5/pack.cc index 16728af4..11aef630 100644 --- a/ecp5/pack.cc +++ b/ecp5/pack.cc @@ -3012,8 +3012,19 @@ void Arch::assignArchInfo() std::string regmode_b = str_or_default(ci->params, id("REGMODE_B"), "NOREG"); if (!(regmode_b == "NOREG" || regmode_b == "OUTREG")) NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE_B configuration '" + regmode_b + "'"); - ci->ramInfo.output_a_registered = regmode_a == "OUTREG"; - ci->ramInfo.output_b_registered = regmode_b == "OUTREG"; + ci->ramInfo.is_output_a_registered = regmode_a == "OUTREG"; + ci->ramInfo.is_output_b_registered = regmode_b == "OUTREG"; + + // Based on the REGMODE, we have different timing lookup tables. + if (!ci->ramInfo.is_output_a_registered && !ci->ramInfo.is_output_b_registered) { + ci->ramInfo.regmode_timing_id = id_DP16KD_REGMODE_A_NOREG_REGMODE_B_NOREG; + } else if (!ci->ramInfo.is_output_a_registered && ci->ramInfo.is_output_b_registered) { + ci->ramInfo.regmode_timing_id = id_DP16KD_REGMODE_A_NOREG_REGMODE_B_OUTREG; + } else if (ci->ramInfo.is_output_a_registered && !ci->ramInfo.is_output_b_registered) { + ci->ramInfo.regmode_timing_id = id_DP16KD_REGMODE_A_OUTREG_REGMODE_B_NOREG; + } else if (ci->ramInfo.is_output_a_registered && ci->ramInfo.is_output_b_registered) { + ci->ramInfo.regmode_timing_id = id_DP16KD_REGMODE_A_OUTREG_REGMODE_B_OUTREG; + } } } for (auto net : sorted(nets)) { From e46b990251a14ade7ede313226a97bd2a3c42191 Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 7 Apr 2020 14:31:17 -0400 Subject: [PATCH 4/6] Rearrange bool algebra --- ecp5/pack.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ecp5/pack.cc b/ecp5/pack.cc index 11aef630..91b8205d 100644 --- a/ecp5/pack.cc +++ b/ecp5/pack.cc @@ -3007,10 +3007,10 @@ void Arch::assignArchInfo() // Output register mode (REGMODE_{A,B}). Valid options are 'NOREG' and 'OUTREG'. std::string regmode_a = str_or_default(ci->params, id("REGMODE_A"), "NOREG"); - if (!(regmode_a == "NOREG" || regmode_a == "OUTREG")) + if (regmode_a != "NOREG" && regmode_a != "OUTREG") NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE_A configuration '" + regmode_a + "'"); std::string regmode_b = str_or_default(ci->params, id("REGMODE_B"), "NOREG"); - if (!(regmode_b == "NOREG" || regmode_b == "OUTREG")) + if (regmode_b != "NOREG" && regmode_b != "OUTREG") NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE_B configuration '" + regmode_b + "'"); ci->ramInfo.is_output_a_registered = regmode_a == "OUTREG"; ci->ramInfo.is_output_b_registered = regmode_b == "OUTREG"; From fc591421f970b8bb43c911adbfefccdd00d81b9c Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 7 Apr 2020 14:42:27 -0400 Subject: [PATCH 5/6] Change assert to error --- ecp5/pack.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ecp5/pack.cc b/ecp5/pack.cc index 91b8205d..eea05c83 100644 --- a/ecp5/pack.cc +++ b/ecp5/pack.cc @@ -2972,6 +2972,7 @@ bool Arch::pack() void Arch::assignArchInfo() { + Context *ctx = getCtx(); for (auto cell : sorted(cells)) { CellInfo *ci = cell.second; if (ci->type == id_TRELLIS_SLICE) { @@ -3008,10 +3009,12 @@ void Arch::assignArchInfo() // Output register mode (REGMODE_{A,B}). Valid options are 'NOREG' and 'OUTREG'. std::string regmode_a = str_or_default(ci->params, id("REGMODE_A"), "NOREG"); if (regmode_a != "NOREG" && regmode_a != "OUTREG") - NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE_A configuration '" + regmode_a + "'"); + log_error("DP16KD %s has invalid REGMODE_A configuration '%s'\n", ci->name.c_str(ctx), + regmode_a.c_str()); std::string regmode_b = str_or_default(ci->params, id("REGMODE_B"), "NOREG"); if (regmode_b != "NOREG" && regmode_b != "OUTREG") - NPNR_ASSERT_FALSE_STR("bad DP16KD REGMODE_B configuration '" + regmode_b + "'"); + log_error("DP16KD %s has invalid REGMODE_B configuration '%s'\n", ci->name.c_str(ctx), + regmode_b.c_str()); ci->ramInfo.is_output_a_registered = regmode_a == "OUTREG"; ci->ramInfo.is_output_b_registered = regmode_b == "OUTREG"; From 3aecb3b08c20c3dc5055bce035bd2667705ea5b2 Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 7 Apr 2020 14:44:19 -0400 Subject: [PATCH 6/6] No need to fetch context --- ecp5/pack.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ecp5/pack.cc b/ecp5/pack.cc index eea05c83..dac889ce 100644 --- a/ecp5/pack.cc +++ b/ecp5/pack.cc @@ -2972,7 +2972,6 @@ bool Arch::pack() void Arch::assignArchInfo() { - Context *ctx = getCtx(); for (auto cell : sorted(cells)) { CellInfo *ci = cell.second; if (ci->type == id_TRELLIS_SLICE) { @@ -3009,11 +3008,11 @@ void Arch::assignArchInfo() // Output register mode (REGMODE_{A,B}). Valid options are 'NOREG' and 'OUTREG'. std::string regmode_a = str_or_default(ci->params, id("REGMODE_A"), "NOREG"); if (regmode_a != "NOREG" && regmode_a != "OUTREG") - log_error("DP16KD %s has invalid REGMODE_A configuration '%s'\n", ci->name.c_str(ctx), + log_error("DP16KD %s has invalid REGMODE_A configuration '%s'\n", ci->name.c_str(this), regmode_a.c_str()); std::string regmode_b = str_or_default(ci->params, id("REGMODE_B"), "NOREG"); if (regmode_b != "NOREG" && regmode_b != "OUTREG") - log_error("DP16KD %s has invalid REGMODE_B configuration '%s'\n", ci->name.c_str(ctx), + log_error("DP16KD %s has invalid REGMODE_B configuration '%s'\n", ci->name.c_str(this), regmode_b.c_str()); ci->ramInfo.is_output_a_registered = regmode_a == "OUTREG"; ci->ramInfo.is_output_b_registered = regmode_b == "OUTREG";