abc9: replace cell type/parameters if derived type already processed (#2991)
authorEddie Hung <eddie@fpgeh.com>
Thu, 9 Sep 2021 17:05:55 +0000 (10:05 -0700)
committerGitHub <noreply@github.com>
Thu, 9 Sep 2021 17:05:55 +0000 (10:05 -0700)
* Add close bracket

* Add testcase

* Replace cell type/param if in unmap_design

* Improve abc9_box error message too

* Update comment as per review

passes/techmap/abc9_ops.cc
techlibs/ecp5/synth_ecp5.cc
tests/arch/ecp5/bug2731.ys [new file with mode: 0644]

index 4b5def5eb92b41a3aeba476bb34d5259217e355a..c3eaa70d1c93e69b1d7550294b775943f928e76b 100644 (file)
@@ -180,8 +180,16 @@ void prep_hier(RTLIL::Design *design, bool dff_mode)
                                        continue;
                        }
                        else {
-                               if (!derived_module->get_bool_attribute(ID::abc9_box) && !derived_module->get_bool_attribute(ID::abc9_bypass))
+                               if (!derived_module->get_bool_attribute(ID::abc9_box) && !derived_module->get_bool_attribute(ID::abc9_bypass)) {
+                                       if (unmap_design->module(derived_type)) {
+                                               // If derived_type is present in unmap_design, it means that it was processed previously, but found to be incompatible -- e.g. if
+                                               // it contained a non-zero initial state. In this case, continue to replace the cell type/parameters so that it has the same properties
+                                               // as a compatible type, yet will be safely unmapped later
+                                               cell->type = derived_type;
+                                               cell->parameters.clear();
+                                       }
                                        continue;
+                               }
                        }
 
                        if (!unmap_design->module(derived_type)) {
@@ -442,7 +450,14 @@ void prep_dff(RTLIL::Design *design)
                        if (!inst_module->get_bool_attribute(ID::abc9_flop))
                                continue;
                        log_assert(!inst_module->get_blackbox_attribute(true /* ignore_wb */));
-                       log_assert(cell->parameters.empty());
+                       if (!cell->parameters.empty())
+                       {
+                               // At this stage of the ABC9 flow, cells instantiating (* abc9_flop *) modules must not contain any parameters -- instead it should
+                               // be instantiating the derived module which will have had any parameters constant-propagated.
+                               // This task is expected to be performed by `abc9_ops -prep_hier`, but it looks like it failed to do so for this design.
+                               // Please file a bug report!
+                               log_error("Not expecting parameters on cell '%s' instantiating module '%s' marked (* abc9_flop *)\n", log_id(cell->name), log_id(cell->type));
+                       }
                        modules_sel.select(inst_module);
                }
 }
@@ -783,10 +798,11 @@ void prep_xaiger(RTLIL::Module *module, bool dff)
                        continue;
                if (!cell->parameters.empty())
                {
-                   // At this stage of the ABC9 flow, all modules must be nonparametric, because ABC itself requires concrete netlists, and the presence of
-                   // parameters implies a non-concrete netlist. This means an (* abc9_box *) parametric module but due to a bug somewhere this hasn't been
-                   // uniquified into a concrete parameter-free module. This is a bug, and a bug report would be welcomed.
-                   log_error("Not expecting parameters on module '%s'  marked (* abc9_box *)\n", log_id(cell_name));
+                       // At this stage of the ABC9 flow, cells instantiating (* abc9_box *) modules must not contain any parameters -- instead it should
+                       // be instantiating the derived module which will have had any parameters constant-propagated.
+                       // This task is expected to be performed by `abc9_ops -prep_hier`, but it looks like it failed to do so for this design.
+                       // Please file a bug report!
+                       log_error("Not expecting parameters on cell '%s' instantiating module '%s' marked (* abc9_box *)\n", log_id(cell_name), log_id(cell->type));
                }
                log_assert(box_module->get_blackbox_attribute());
 
index 2df9a1f876589955624518fe209f2fc658c83ebd..dc67fc71b52eabca36a8261610234eeec4542327 100644 (file)
@@ -326,7 +326,7 @@ struct SynthEcp5Pass : public ScriptPass
                        }
                        run("dfflegalize" + dfflegalize_args, "($_DFFSR_*_ only if -asyncprld, $_*DFFE_* only if not -nodffe)");
                        if ((abc9 && dff) || help_mode)
-                               run("zinit -all w:* t:$_DFF_?_ t:$_DFFE_??_ t:$_SDFF*", "(only if -abc9 and -dff");
+                               run("zinit -all w:* t:$_DFF_?_ t:$_DFFE_??_ t:$_SDFF*", "(only if -abc9 and -dff)");
                        run(stringf("techmap -D NO_LUT %s -map +/ecp5/cells_map.v", help_mode ? "[-D ASYNC_PRLD]" : (asyncprld ? "-D ASYNC_PRLD" : "")));
                        run("opt_expr -undriven -mux_undef");
                        run("simplemap");
diff --git a/tests/arch/ecp5/bug2731.ys b/tests/arch/ecp5/bug2731.ys
new file mode 100644 (file)
index 0000000..c609cea
--- /dev/null
@@ -0,0 +1,7 @@
+read_verilog -icells <<EOF
+module top(input c, r, input [1:0] d, output reg [1:0] q);
+TRELLIS_FF #(.REGSET("SET")) ff1(.CLK(c), .LSR(r), .DI(d[0]), .Q(q[0]));
+TRELLIS_FF #(.REGSET("SET")) ff2(.CLK(c), .LSR(r), .DI(d[1]), .Q(q[1]));
+endmodule
+EOF
+synth_ecp5 -abc9 -dff