Represent string constants as strings

[widlarizer]

Currently, a RTLIL::Const representing a string will use std::vector<RTLIL::State>, encoding the string into a vector of individual bits. This increases the memory consumption of string attributes, such as src and hdlname, by a factor of 8.

As of this PR, the structure now holds data represented with an std::variant<bitvectype, std::string> named backing a union. The bits vector is no longer accessible and instead std::vector<RTLIL::State>& bits() has to be used. When a Const is constructed from a string, it is represented with the string alternative. When its bits() method is used, the string is encoded into the bit vector alternative as prior to this PR, see Const::bitvectorize(). This is similar to how RTLIL::SigSpec "unpacks" into bits.

In the initial implementation, when bits() were being accessed, I asserted that the Const isn't represented with a string. This found some places in Yosys that access string attributes bit-by-bit. As a result, this PR comes with bonus content:

• kernel/cellaigs.cc
• frontends/ast/ast.cc
• Const::pretty_fmt
• cell checker is_param that doesn't try return bits

It's possible to split these now necessary changes out to a separate PR, but it will be heavy on git conflicts. Please advise.

This PR does not affect constant SigChunks which also have an std::vector<RTLIL::State> each.

• clean my desk (TODO, log_debug, did_something_hook, unwrap std::get from assert_get_bits etc)
• add some way of debugging how much unpacking we're doing requires unrelated changes in followup PR

Please comment what memory usage changes you see with this PR. I've observed memory savings of 0-20% depending on the design with negligible runtime improvements. The open source Verilog frontend often dominates memory usage, so flows that use Verific may more readily show these improvements.

[povik]

Please comment what memory usage changes you see with this PR.

On the sky130hd/jpeg design from OpenROAD-flow-scripts, dumping the netlist just after the techmap command, I see the following difference in memory usage when loading in the RTLIL dump in a fresh Yosys process:

	with src attributes	src stripped
baseline	291 MB	180 MB
9aa6a6b	221 MB	172 MB
9aa6a6b + hashlib patch	202 MB	175 MB

[povik]

I have tried the following patch to hashlib on top of the PR, and updated the table:

diff --git a/kernel/hashlib.h b/kernel/hashlib.h
index 6b880f3a6..0e839e73e 100644
--- a/kernel/hashlib.h
+++ b/kernel/hashlib.h
@@ -190,7 +190,7 @@ inline int hashtable_size(int min_size)
 {
        // Primes as generated by https://oeis.org/A175953
        static std::vector<int> zero_and_some_primes = {
-               0, 23, 29, 37, 47, 59, 79, 101, 127, 163, 211, 269, 337, 431, 541, 677,
+               0, 3, 23, 29, 37, 47, 59, 79, 101, 127, 163, 211, 269, 337, 431, 541, 677,
                853, 1069, 1361, 1709, 2137, 2677, 3347, 4201, 5261, 6577, 8231, 10289,
                12889, 16127, 20161, 25219, 31531, 39419, 49277, 61603, 77017, 96281,
                120371, 150473, 188107, 235159, 293957, 367453, 459317, 574157, 717697,

[widlarizer]

Switching from std::variant to a union yielded further 1% runtime and 0-1% memory usage reduction. std::variant always uses a size_t for its tag. This inflated the data structure beyond 32B. Effects of such sizes may become more severe when we play around to custom allocation strategies. I added private accessor methods to give it a safer interface for methods that don't strictly need direct access. I still use direct union and tag access in constructors, the assignment operator, and bitvectorize.

[widlarizer]

@povik the prime list change adds runtime improvements up to 3% on top of this with a 0.5% regression in one case

[povik]

Comparison and indexing isn't consistent across backings

[povik]

Looks good apart from the extra cxxrtl change and the extra assert

[povik]

I haven't looked at the new unit testing but the Const change itself looks fine with the points from the last review resolved

[whitequark]

Great work!

[widlarizer]

just some fixes from the functional merges

[widlarizer]

I validated the memory savings to be 0-30%.

ORFS peak:

design	5c9b2df main [MB]	2dd0ae3 PR [MB]
asap7_ibex	224.52	194.96
sky130hd_gcd	57.75	57.75
sky130hd_ibex	223.20	159.32
sky130hd_jpeg	726.52	613.00

In the flow I dumped out the RTLIL before synthesis and outside of ORFS loaded the RTLIL and ran just the basic synth command:

design	5c9b2df main [MB]	2dd0ae3 PR [MB]
asap7_ibex	175.88	122.42
sky130hd_gcd	45.56	45.56
sky130hd_ibex	175.76	123.02
sky130hd_jpeg	85.14	75.63

As for runtime, there was negligible improvement (0-1%)

[KrystalDelusion]

This PR introduced a lot of warnings along the lines of
comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Wsign-compare], I'm guessing because the previous method of checking string length used the .size() method of a std::vector which returns a size_t, but the RTLIL::const .size() method returns it as an int (and seems to implicitly convert the underlying .size() size_t value)

[widlarizer]

True. I brought up size types on a dev jf and we decided to stick with int for Yosys data structures so I guess I'll make Const::size() consistent with that then