Skip to content

synth: Add sort before ABC to avoid nondeterminism (workaround)#4058

Open
openroad-ci wants to merge 8 commits intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-workaround-yosys-nd
Open

synth: Add sort before ABC to avoid nondeterminism (workaround)#4058
openroad-ci wants to merge 8 commits intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-workaround-yosys-nd

Conversation

@openroad-ci
Copy link
Copy Markdown
Collaborator

Summary

  • Insert a sort command before abc execution in synth.tcl to stabilize module iteration order
  • Without this, abc_new visits modules in dict insertion order which can vary between runs, producing different netlists from the same RTL input
  • Sorting runtime is trivial (125ms in nangate45/swerv_wrapper)
  • Enabled by default; can be disabled with SYNTH_SORT_BEFORE_ABC=-1
  • This is a work-around for the following issue. This work-around should be removed after Yosys non-determinism issue is resolved.

Closes #4056

Insert a `sort` command before abc execution in synth.tcl to
stabilize module iteration order. Without this, abc_new visits
modules in dict insertion order which can vary between runs,
producing different netlists from the same RTL input.

The sort is enabled by default and can be disabled by setting
SYNTH_SORT_BEFORE_ABC=-1.

Closes The-OpenROAD-Project#4056

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
# 2025-03-26: Temporary workaround before yosys fix.
# Sort design objects before abc execution to avoid non-deterministic result.
# Enabled by default and can be disabled with SYNTH_SORT_BEFORE_ABC=-1.
if { ![env_var_equals SYNTH_SORT_BEFORE_ABC -1] } {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add to variables.yaml and use POLA 0/1 as boolean and 0 default

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oharboe
Oops. Weird value '-1' was there. I'll fix it.

By the way, I intended to make the sorting on-by-default to avoid the ND issue. The sorting runtime is trivial.
Please let me know if you disagree to the sorting on-by-default.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On by default is good

Address review feedback: use standard ORFS 0/1 boolean convention
with $::env() instead of env_var_equals with -1. The variable
default is set in variables.yaml (separate commit).

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Print an informational message when the sort workaround is active
so users can confirm it is being applied.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Add the variable definition with default: 1 (on-by-default) following
the standard ORFS 0/1 boolean convention.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Regenerate FlowVariables.md from variables.yaml to include the
new SYNTH_SORT_BEFORE_ABC variable.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
…ate/OpenROAD-flow-scripts into secure-workaround-yosys-nd

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Update metrics from CI run after master merge. Fix nangate45/swerv_wrapper
GRT congestion by reducing PLACE_DENSITY_LB_ADDON from 0.08 to 0.05.

designs/asap7/aes-block/rules-base.json updates:
| Metric                                        | Old      | New      | Type     |
| ------                                        | ---      | ---      | ----     |
| cts__timing__setup__tns                       |  -2890.0 |  -3330.0 | Failing  |
| cts__timing__hold__tns                        |  -5670.0 |  -4970.0 | Tighten  |
| globalroute__timing__setup__tns               |  -3020.0 |  -2880.0 | Tighten  |
| globalroute__timing__hold__ws                 |    -25.9 |    -49.0 | Failing  |
| globalroute__timing__hold__tns                |  -1080.0 |  -1360.0 | Failing  |
| finish__timing__setup__ws                     |    -58.1 |    -56.7 | Tighten  |
| finish__timing__setup__tns                    |   -897.0 |   -614.0 | Tighten  |
| finish__timing__hold__tns                     |    -90.0 |   -201.0 | Failing  |

See metric_update_table.md for full metric update details.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
designs/rapidus2hp/hercules_idecode/rules-verific.json updates:
| Metric                                        | Old      | New      | Type     |
| ------                                        | ---      | ---      | ----     |
| placeopt__design__instance__count__stdcell    |   232846 |   232170 | Tighten  |
| cts__design__instance__count__setup_buffer    |    20248 |    20189 | Tighten  |
| cts__design__instance__count__hold_buffer     |    20248 |    20189 | Tighten  |
| globalroute__antenna_diodes_count             |      202 |      201 | Tighten  |
| globalroute__timing__setup__tns               |  -0.0993 |   -0.295 | Failing  |
| finish__timing__setup__tns                    |  -0.0993 |   -0.296 | Failing  |

designs/rapidus2hp/gcd/rules-verific.json updates:
| Metric                                        | Old      | New      | Type     |
| ------                                        | ---      | ---      | ----     |
| placeopt__design__instance__count__stdcell    |      688 |      677 | Tighten  |
| cts__timing__setup__ws                        |  -0.0221 |  -0.0275 | Failing  |
| cts__timing__setup__tns                       |   -0.475 |    -0.19 | Tighten  |
| globalroute__timing__setup__ws                |  -0.0302 |  -0.0378 | Failing  |
| globalroute__timing__setup__tns               |   -0.753 |   -0.616 | Tighten  |
| finish__timing__setup__ws                     |  -0.0302 |  -0.0378 | Failing  |
| finish__timing__setup__tns                    |   -0.753 |   -0.616 | Tighten  |

designs/rapidus2hp/hercules_is_int/rules-verific.json updates:
| Metric                                        | Old      | New      | Type     |
| ------                                        | ---      | ---      | ----     |
| placeopt__design__instance__count__stdcell    |   646997 |   642088 | Tighten  |
| cts__design__instance__count__setup_buffer    |    56261 |    55834 | Tighten  |
| cts__design__instance__count__hold_buffer     |    56261 |    55834 | Tighten  |
| globalroute__antenna_diodes_count             |      567 |      562 | Tighten  |
| globalroute__timing__setup__tns               |   -107.0 |   -145.0 | Failing  |
| finish__timing__setup__tns                    |   -107.0 |   -145.0 | Failing  |

designs/rapidus2hp/ibex/rules-verific.json updates:
| Metric                                        | Old      | New      | Type     |
| ------                                        | ---      | ---      | ----     |
| globalroute__timing__setup__tns               |   -0.152 |    -2.78 | Failing  |
| finish__timing__setup__tns                    |   -0.152 |    -2.86 | Failing  |

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Synthesis] nangate45/swerv_wrapper QoR fluctuation due to Yosys abc_new nondeterminism

4 participants