Rewrite the RTLIL parser for efficiency

[rocallahan]

What are the reasons/motivation for this change?

See https://yosyshq.discourse.group/t/faster-rtlil-parser for context. The current parser is not very C++ friendly, is hard to debug because of the Flex/Bison dependency, and does a lot of unnecessary copying.

Explain how this is achieved.

By rewriting it into a handwritten recursive-descent parser it becomes more maintainable and 2.5x faster.

I ran the Amaranth tests and they pass.

[whitequark]

You should also run the Glasgow tests since they exercise a bigger class of netlists.

[KrystalDelusion]

FYI currently failing with memory leak in make abcopt-tests/tests/alumacc (I think tests/alumacc/macc_b_port_compat.ys is probably just the first read_rtlil with a heredoc that gets run)

=================================================================
==5081==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 384 byte(s) in 1 object(s) allocated from:
    #0 0x555d229a24a1 in operator new(unsigned long) (/home/runner/work/yosys/yosys/yosys+0xc3e4a1) (BuildId: 50d17c5ea35ae557)
    #1 0x555d22a6b830 in Yosys::Frontend::extra_args(std::istream*&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>, unsigned long, bool) /home/runner/work/yosys/yosys/build/../kernel/register.cc:475:8
    #2 0x555d23295e8b in Yosys::RTLILFrontend::execute(std::istream*&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>, Yosys::RTLIL::Design*) /home/runner/work/yosys/yosys/build/../frontends/rtlil/rtlil_frontend.cc:823:3
    #3 0x555d22a6a7a3 in Yosys::Frontend::execute(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>, Yosys::RTLIL::Design*) /home/runner/work/yosys/yosys/build/../kernel/register.cc:424:3
    #4 0x555d22a6429c in Yosys::Pass::call(Yosys::RTLIL::Design*, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>) /home/runner/work/yosys/yosys/build/../kernel/register.cc:272:26
    #5 0x555d22a62974 in Yosys::Pass::call(Yosys::RTLIL::Design*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>) /home/runner/work/yosys/yosys/build/../kernel/register.cc:249:2
    #6 0x555d22c9f649 in Yosys::run_frontend(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, Yosys::RTLIL::Design*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/runner/work/yosys/yosys/build/../kernel/yosys.cc:761:6
    #7 0x555d229be3c6 in main /home/runner/work/yosys/yosys/build/../kernel/driver.cc:544:7
    #8 0x7f6e7a62a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 282c2c16e7b6600b0b22ea0c99010d2795752b5f)
    #9 0x7f6e7a62a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 282c2c16e7b6600b0b22ea0c99010d2795752b5f)
    #10 0x555d228c8fb4 in _start (/home/runner/work/yosys/yosys/yosys+0xb64fb4) (BuildId: 50d17c5ea35ae557)

rtlil_frontend.cc:823 extra_args(worker.f, filename, args, argidx);
register.cc:475 f = new std::istringstream(last_here_document);

Something to do with modifying the input file istream within the worker?

Compare cxxrtl_backend.cc:

		CxxrtlWorker worker;
...

			if (args[argidx] == "-header") {
				worker.split_intf = true;
				continue;
			}
...
		extra_args(f, filename, args, argidx);
...
		worker.impl_f = f;
		worker.prepare_design(design);

[georgerennie]

I don't know how much we care about behaviour on malformed input, but with a fuzzer I found the following:

module \top
  wire width 12 input 0 \A
  wire width 2 input 1 \S
  wire width 6 output 2 \Y

  cell $bmux $0
    parameter \WIDTH 6
    parameter \S_WIDTH 2
    connect \A $0
  end
end

With previous read_rtlil this results in ERROR: Parser error in line 10: RTLIL error: wire $0 not found, with this patch I instead get SIGSEGV or SIGILL depending on the build config (but ive struggled to debug it under gdb to see more).

[rocallahan]

You should also run the Glasgow tests since they exercise a bigger class of netlists.

I ran the Glasgow tests with GLASGOW_TOOLCHAIN=system,builtin and all tests passed:

Ran 345 tests in 43.617s

I hope I did that right...

[rocallahan]

With previous read_rtlil this results in ERROR: Parser error in line 10: RTLIL error: wire $0 not found, with this patch I instead get SIGSEGV or SIGILL depending on the build config

Fixed.

[rocallahan]

FYI currently failing with memory leak

Fixed.

[rocallahan]

I ran the AFL++ fuzzer for 300 CPU-hours and found one issue: it's trivially easy to crash the parser on with OOM on a tiny input by writing a constant like 999999999999'0. (That crashes the old parser too, a bit more slowly.) Maybe we don't care about that, but it may be blocking fuzzers from finding more interesting crashes, and it seems pretty reasonable to me to limit constants to some maximum size like 1Gb, so I've added a commit to do that. (Of course Yosys internally limits a lot of values to int, i.e. 2G, already.)

[georgerennie]

Yeah I saw the same in my fuzzing runs - didn't mention it because it's not a change in behaviour and I doubt it would actually affect fuzzer effectiveness because from a coverage perspective it looks roughly the same whether it crashes or raises an error for afl++, it just means the testcase gets binned as a crash.

I'm happy to see a limit on the max size, there was discussion of this in the past as #3460 but it was never merged (the author stopped working on Yosys as much around then).

[rocallahan]

I doubt it would actually affect fuzzer effectiveness because from a coverage perspective it looks roughly the same whether it crashes or raises an error for afl++, it just means the testcase gets binned as a crash.

Good point.

[whitequark]

I ran the Glasgow tests with GLASGOW_TOOLCHAIN=system,builtin and all tests passed:

Sounds good to me, thank you!