vtr-verilog-to-routing
vtr-verilog-to-routing copied to clipboard
verilog-to-routing
Fix bug in which all non-chain luts are considered length-1 chains.
Current find_new_root_atom_for_chain assumes that an atom without a driver is the root of a chain, and so all non-chain luts are considered trivial length-1 chains. This fix permits a driverless atom to be a chain root only if it drives another chain atom.
Description
Added a new boolean input, is_non_trivial_chain, to find_new_root_atom_for_chain, initially set to false. If the current block has no drivers, the function can only return it as the new chain root if the boolean is true. The boolean is set to true when the function is called recursively from further down the chain.
Related Issue
Motivation and Context
I noticed this phenomenon when examining lists of pack molecules while debugging a legalizer tool to read in an external flat placement and recreate a legal clustering. My tool packs chains first, and it works better without trivial chains.
How Has This Been Tested?
This has been tested on all Titan23s as part of normal vpr packing and as part of my legalizer tool.
Types of changes
- [ ] Bug fix (change which fixes an issue)
- [ ] New feature (change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist:
- [ ] My change requires a change to the documentation
- [ ] I have updated the documentation accordingly
- [ ] I have added tests to cover my changes
- [ ] All new and existing tests passed
Some of the CI test failures may be random changes. E.g.: vtr_nightly_test1_odin: [Fail] 22:07:36 | k6_frac_2ripple_N8_22nm.xml/fir_pipe_15.v/common min_chan_width_route_time relative value 15.494845360824742 outside of range [0.1,15.0], above absolute threshold 2.0 and not equal to golden value: 2.91 --> this just looks like a minor change on a small design; it's OK to just update the golden results (minimum channel width routing time went up more than 15x, but the overall runtime on this small circuit is still small and minimum channel width route times can have high noise)
This vtr_reg_strong test looks like it actually failed to complete though: slicem.xml/carry_chain.blif/common vpr_status Task value 'exited with return code 1' does not match golden 'success' 18:55:05 | [Fail]
So that is likely a corner case.
Some other runs seem to have run out of time; relaunching.
After CI passes (or the errors are found to be spurious as in the small QoR change above), you should also get a QoR comparison on the vtr and titan circuits anyway to ensure nothing bad has happened. I don't expect it would, but it is best to be safe. See https://docs.verilogtorouting.org/en/latest/README.developers/#example-vtr-benchmarks-qor-measurement for a how-to guide.
Yes, the carry chain test failure is real. The corner case is when the head is the first node encountered in a chain. I have an idea for a fix, stay tuned.
It appears that vtr_reg_nightly_test2 and vtr_reg_nightly_test2_odin failed with the message: Error: The operation was canceled.
All others passed. Re-running just those two.
These two tests keep stopping with the note "the operation was cancelled" and I am not sure why. Vaughn can you take a look?
Update: @KA7E is deciding if this is a bug or just a somewhat unexpected way to consider all LUTs part of (length 1 at least) carry chains.
@KA7E tells me her preference is to not merge (at least for now). It seems like an odd behaviour, but it no longer impacts the legalizer. Since there are CI failures (hopefully spurious) and QoR would have to be checked this would need some work to merge. Putting on hold for now.