search

parallaxsw / OpenSTA

GNU General Public License v3.0
52 stars 23 forks source link

Smallfix to get `liberty_arcs_one2one` tests consistently passing in regression #86

Closed akashlevy closed 2 months ago

akashlevy commented 2 months ago

85 highlighted that liberty_arcs_one2one fails under different machine configurations (via Docker). This PR fixes the issue by using the summary mode of report_checks to avoid having rising/falling edges be reported in a non-deterministic way.

jjcherry56 commented 2 months ago

I think in this case you may want to use report_edges -from to query the graph directly instead of using reporting.

akashlevy commented 2 months ago

True, I'm still relying on equi-delay paths being reported in a certain order. Let me go fix that...

akashlevy commented 2 months ago

Ok, I switched to using report_edges, but I'm seeing two potential issues...

  1. I'm confused why there seem to be four timing arcs per edge instead of two? An inverter is negative unate, so I don't know why we are seeing something that looks non-unate... does OpenSTA calculate unateness from the Liberty function?
  2. Is the order in which the different arcs are reported a deterministic thing for report_edges?
jjcherry56 commented 2 months ago

The "function" is pretty ambiguous when you are talking about a bus. The bit port is a[0] but the function is on the bus port, not the bit port. It isn't smart enough to infer the timing sense so it defaults to non_unate. Just make it explicit like most libraries do.

I believe the reporting order should be deterministic. There was a big change to use object IDs instead of pointers for better determinism, but it may not be perfect. Are you seeing platform dependent results?

I want to see this broken into 2 different tests instead of reading 2 different verilog files. I traditionally use a one line comment at the beginning of a regression so head -1 can show what it is for. I only use file names to roughly group regressions and use numeric suffixes to distinguish them.

akashlevy commented 2 months ago

Got it. I've added the timing sense to the Liberty files as suggested, which resolved the issue with non-unate arcs.

I have split and renamed the test cases. Added comments to line 1 of all tests describing what they do. Cross-platform regression runs are going through on my Mac as well as the two Linux Dockers.

Let me know if anything else is needed. Might be helpful to have a set of test guidelines in doc/CodingGuidelines.txt so contributors can follow that.