write_rtlil: don't sort

[widlarizer]

Fixes #3576. We need to fix it becuse the obvious, most default attempt at roundtripping through textual representation of the core IR shouldn't fail. But it does, which makes building reproducers and custom workflows insufferable.

I think that the purpose of the sort is to hide reproducibility bugs and cross-platform equivalence bugs in Yosys. I don't know of recent cases of the former and they're bugs, we shouldn't be proactively hiding them. For the latter, see #5205. These are real categories of bugs that probably happen, but hiding them in one place in a way that breaks roundtripping solves nothing as these will still occur inside of flows. See 4395109 for how the sort was added, replacing pre-existing sorting of the output rather than the design.

From #3576:

Reloading the design changes some autoindex numbers and some pointer values

I think this was actually referring to roundtripping through Verilog. Reloading RTLIL doesn't change autoidx. Dependence on pointer values would be a bug and I don't expect we have a lot of these, I would even expect a sanitizer to report them as an error (pointer arithmetic etc) though I haven't checked. Until recently, we haven't been using std::unordered_map and friends, instead opting for custom replacements in hashlib that have well-defined iteration order. Since we have started using the std types, we've been careful not to rely on their iteration order.

For allowing RTLIL designs to be easily diffed, see #5003

Related: #5150

• Fix that the backend writes out things in insertion order (the usual iteration order is reverse insertion order)
• Figure out how to test by comparing against canonical dumps when src attribute values depend on source path

[KrystalDelusion]

I have used dump instead of write_rtlil when running into issues with reproducibility because that also avoids the sort, and I think not needing to remember that they are different would be an improvement.

[widlarizer]

One of the CI failures is a functional backend test failing: ERROR: The design contains an undriven signal $flatten\picorv32_i.$auto$wreduce.cc:514:run$3801. This is not supported by the functional backend. Call setundef with appropriate options to avoid this error. , can be reproduced with pytest -k picorv -m "not smt and not rkt" run in tests/functional

[widlarizer]

The failing functional test is due to #5207

[whitequark]

Is -relativeshare something likely to become a default?

[widlarizer]

@whitequark I added -relativeshare just so that I can use a fully synthesized demo design and compare against a committed reference. I think that with -noabc Yosys actually is fully reproducible across platforms and if not we have to fix it either way. I don't think it would make sense to use in any other context

[widlarizer]

I expect some users may dislike this change, since it will introduce frustrating disorder in even how cell ports are ordered. However, that is just a realistic view of what Yosys sees, so it's necessary for write_rtlil creating "Yosys save files". They can still do write_rtlil -sort to get the old behavior

[widlarizer]

@whitequark Are you okay with the removal of the design.sort() in bugpoint? It's been there from the start, so I assume it was for consistency with write_rtlil or with some similar idea, but it might even hide some bugs, making them unbugpointable

[jix]

Making a pass over the diff, this lgtm