diff --git a/xilinx/fasm.cc b/xilinx/fasm.cc index fc7d7a61..e40cbd08 100644 --- a/xilinx/fasm.cc +++ b/xilinx/fasm.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "log.h" #include "nextpnr.h" @@ -5176,8 +5177,20 @@ void write_gtx_channel(CellInfo *ci) write_bit("ZADREG[0]", !bool_or_default(ci->params, ctx->id("ADREG"), true)); write_bit("ZALUMODEREG[0]", !bool_or_default(ci->params, ctx->id("ALUMODEREG"))); - write_bit("ZAREG_2_ACASCREG_1", !bool_or_default(ci->params, ctx->id("ACASCREG"))); - write_bit("ZBREG_2_BCASCREG_1", !bool_or_default(ci->params, ctx->id("BCASCREG"))); + // AREG_2_ACASCREG_1 is prjxray's name for a CONJUNCTION, not for + // ACASCREG: fuzzers/100-dsp-mskpat/generate.py sets it to + // (AREG == 2 && ACASCREG == 1). Its Z-form bit therefore belongs in + // the bitstream whenever that conjunction is FALSE. Deciding it from + // ACASCREG alone mis-encodes the case yosys emits whenever it absorbs + // one level of input register into the DSP -- AREG=1 with + // ACASCREG=1 -- which wrote no bit at all and reads back on silicon + // as AREG=2, one pipeline stage deeper than the netlist asked for. + // The multiply then samples its operands a cycle late and the design + // returns wrong products with a correct netlist. Same for B. + auto acascreg = int_or_default(ci->params, ctx->id("ACASCREG"), 1); + auto bcascreg = int_or_default(ci->params, ctx->id("BCASCREG"), 1); + write_bit("ZAREG_2_ACASCREG_1", !(areg == 2 && acascreg == 1)); + write_bit("ZBREG_2_BCASCREG_1", !(breg == 2 && bcascreg == 1)); write_bit("ZCARRYINREG[0]", !bool_or_default(ci->params, ctx->id("CARRYINREG"))); write_bit("ZCARRYINSELREG[0]", !bool_or_default(ci->params, ctx->id("CARRYINSELREG"))); write_bit("ZCREG[0]", !bool_or_default(ci->params, ctx->id("CREG"), true)); @@ -5201,9 +5214,39 @@ void write_gtx_channel(CellInfo *ci) boost::split(pins, attr_value, boost::is_any_of(" ")); for (auto pin : pins) { if (boost::empty(pin)) continue; + // These pins have no interconnect path into the site, so the + // tile bit IS the pin: it bypasses the site's optional input + // inverter, and the ZIS_*_INVERTED bits fasm.cc writes apply + // only to the routed pins. The bit therefore has to carry the + // LOGICAL value -- flip the constant when the netlist asked + // for an inversion. + // + // Splitting the trailing index off matters: erase_all() of + // "0123456789" turned "INMODE1" into "INMODE" and then looked + // up IS_INMODE_INVERTED, which yosys never emits (it writes + // the per-bit IS_INMODE[1]_INVERTED), so `inv` was always + // false and every bussed pin got the un-flipped constant. + // That is the "seems to be inverted for unknown reasons" that + // kept INMODE/ALUMODE2/ALUMODE3 commented out of the packer's + // const-pin list in pack_dsp_xc7.cc. auto pin_basename = pin; - boost::erase_all(pin_basename, "0123456789"); - auto inv = bool_or_default(ci->params, ctx->id("IS_" + pin_basename + "_INVERTED"), 0); + std::string idx; + while (!pin_basename.empty() && std::isdigit((unsigned char)pin_basename.back())) { + idx.insert(idx.begin(), pin_basename.back()); + pin_basename.pop_back(); + } + bool inv = bool_or_default(ci->params, ctx->id("IS_" + pin + "_INVERTED"), false); + if (!idx.empty()) { + inv |= bool_or_default( + ci->params, + ctx->id("IS_" + pin_basename + "[" + idx + "]_INVERTED"), false); + inv |= ((int_or_default(ci->params, + ctx->id("IS_" + pin_basename + "_INVERTED"), 0) + >> std::stoi(idx)) & 0x1) != 0; + } else { + inv |= bool_or_default(ci->params, ctx->id("IS_" + pin_basename + "_INVERTED"), + false); + } auto net_name = inv ? (const_net_name == "GND" ? "VCC" : "GND") : const_net_name; write_bit(dsp + "_" + pin + ".DSP_" + net_name + "_" + tile_side); } diff --git a/xilinx/pack_dsp_xc7.cc b/xilinx/pack_dsp_xc7.cc index c78250a1..4e896126 100644 --- a/xilinx/pack_dsp_xc7.cc +++ b/xilinx/pack_dsp_xc7.cc @@ -121,12 +121,33 @@ void XC7Packer::pack_dsps() // prjxray has extra bits for these ports to hardwire them to VCC/GND // as these seem to be internal to the tile, // this saves us from having to route those externally + // The pins below have NO routing path into the site at all: + // they appear in prjxray's segbits_dsp_{l,r}.db only as + // .DSP_GND_* / .DSP_VCC_* and in no ppips/pips + // list, so a tile-local constant bit is the ONLY way to give + // them a value. Leaving one out does not fail routing -- it + // silently emits no bit, the pin sits at the tile default 0, + // and the site's ZIS_*_INVERTED bit (written independently by + // fasm.cc's write_bus_zinv) flips it to 1. + // + // INMODE, ALUMODE2/3 and OPMODE6 were missing. For a plain + // inferred multiplier that turns INMODE 00000 into 11111 -- + // and INMODE[1]=1 gates the multiplier's A input to zero + // (UG479 Table 1-11) -- OPMODE[6:4] 000 into 100 (P feedback + // instead of zero) and ALUMODE[3:2] 00 into 11. The DSP then + // ignores its A operand entirely and returns near-constant + // junk, with a correct netlist and clean timing. + // + // The earlier "these seem to be inverted for unknown reasons" + // is that ZIS_*_INVERTED: the pin must be driven with the + // UNINVERTED value (VCC for a wanted 0 when the site inverts), + // which is what connecting it to its actual constant net does. if (boost::starts_with(n, "D") || boost::starts_with(n, "RSTD") || - // TODO: these seem to be inverted for unknown reasons - // boost::starts_with(n, "INMODE") || - // boost::starts_with(n, "ALUMODE2") || - // boost::starts_with(n, "ALUMODE3") || + boost::starts_with(n, "INMODE") || + boost::starts_with(n, "ALUMODE2") || + boost::starts_with(n, "ALUMODE3") || + boost::starts_with(n, "OPMODE6") || boost::starts_with(n, "CARRYINSEL2") || boost::starts_with(n, "CED") || boost::starts_with(n, "CEAD") ||