Updating test_cell

[KrystalDelusion]

What are the reasons/motivation for this change?

The test_cell pass hasn't been kept fully up to date, and there are a number of cells marked as evaluable but not supported by it (or in some cases, by ConstEval::eval() which it uses). $pmux is also only supported when using the -edges flag, though it's not specified as to why. This is spun out of #4910. $macc support is also out of date since the change to $macc_v2.

Explain how this is achieved.

• Update test_cell with support for (most) of the evaluable cells, gated by nosat or techmap_cmd.compare as needed.
• Fix ConstEval::eval() base case to work with $bwmux.
• Add wide mux testing spun out to Wide mux cells ($_MUX4_ et al) are incorrectly marked as evaluable #5035
• Update $macc cell to use $macc_v2.
• Add comments for partially supported cell types.
• Add comment to CellTypes::eval() about using ConstEval.
• Add test_cell to CI

If applicable, please suggest to reviewers how they can test the change.

Call test_cell all.
$pow and $pmux require -nosat and -aigmap flags.
$eqx, $nex, and $bweqx require -nosat and either -aigmap or -simlib flags.
$buf requires either -aigmap or -simlib flags.

OR

Check test-cells CI job ("Build and run tests/Run test_cell" in Checks tab)

[KrystalDelusion]

@povik Can you update the handling for $macc_v2 in the create_gold_module function in test_cell.cc? Just changing it from $macc to $macc_v2 fails the internal cell checker. And leaving it as $macc doesn't work because it uses Yosys::Macc::to_cell() which converts it to a v2 and then fails the internal cell checker.

[povik]

@KrystalDelusion please see this patch and #5025

diff --git a/passes/tests/test_cell.cc b/passes/tests/test_cell.cc
index 39b7ccd3a..f42255803 100644
--- a/passes/tests/test_cell.cc
+++ b/passes/tests/test_cell.cc
@@ -167,7 +167,7 @@ static RTLIL::Cell* create_gold_module(RTLIL::Design *design, RTLIL::IdString ce
                cell->setPort(ID::CO, wire);
        }
 
-       if (cell_type == ID($macc))
+       if (cell_type == ID($macc_v2))
        {
                Macc macc;
                int width = 1 + xorshift32(8 * bloat_factor);
@@ -201,6 +201,7 @@ static RTLIL::Cell* create_gold_module(RTLIL::Design *design, RTLIL::IdString ce
                        this_port.do_subtract = xorshift32(2) == 1;
                        macc.ports.push_back(this_port);
                }
+
                // Macc::to_cell sets the input ports
                macc.to_cell(cell);
 
@@ -208,12 +209,6 @@ static RTLIL::Cell* create_gold_module(RTLIL::Design *design, RTLIL::IdString ce
                wire->width = width;
                wire->port_output = true;
                cell->setPort(ID::Y, wire);
-
-               // override the B input (macc helpers always sets an empty vector)
-               wire = module->addWire(ID::B);
-               wire->width = xorshift32(mulbits_a ? xorshift32(4)+1 : xorshift32(16)+1);
-               wire->port_input = true;
-               cell->setPort(ID::B, wire);
        }
 
        if (cell_type == ID($lut))
@@ -937,7 +932,7 @@ struct TestCellPass : public Pass {
                cell_types[ID($sop)] = "*";
                cell_types[ID($alu)] = "ABSY";
                cell_types[ID($lcu)] = "*";
-               cell_types[ID($macc)] = "*";
+               cell_types[ID($macc_v2)] = "*";
                cell_types[ID($fa)] = "*";
 
                for (; argidx < GetSize(args); argidx++)

In case you're wondering this shouldn't be removing any coverage on the B port as all A, B, C ports get populated by the helper for macc_v2.

[KrystalDelusion]

Rebased on latest main to include #5025

[mmicko]

We can see later if it is too costly to spawn new CI machine just for this, but guess additional useful tests could be added here, or we can merge it with running yosys-tests.

[KrystalDelusion]

We can see later if it is too costly to spawn new CI machine just for this, but guess additional useful tests could be added here, or we can merge it with running yosys-tests.

Looking at the time taken for the checks on this PR, the test_cell tests took 16m, while regular Ubuntu tests took just 15m. It is running 100 tests on each cell type though, so I think if we did move it into the main yosys tests we should reduce that that.

It might also be a good idea to use a fixed seed for better repeatability, I think I was going to do that but forgot while waiting for the CI to run and then got distracted doing something else 😅

[ShinyKate]

@KrystalDelusion this good to merge?