Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enabled BLIF buffer elimination in preparation for InOuts #1520

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Enabled BLIF buffer elimination in preparation for InOuts #1520

wants to merge 2 commits into from

Conversation

djns99
Copy link

@djns99 djns99 commented Sep 1, 2020 •
edited
Loading

Description

Enables the logic for eliminating the redundant final buffer in the BLIF output. This is a necessary precursor to InOuts as the extra buffer is not driven by the input correctly

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:

  • All new and existing tests passed

@probot-autolabeler probot-autolabeler bot added lang-cpp C/C++ code Odin Odin II Logic Synthesis Tool: Unsorted item labels Sep 1, 2020
@djns99 djns99 force-pushed the BLIFFinalBufferEliminate branch from 9a5d35c to 01c8756 Compare September 1, 2020 23:44
@djns99
Copy link
Author

djns99 commented Sep 2, 2020

@kmurray This changes Odin II's BLIF output slightly which seems to have had some QoR changes for VPR. The issues on Travis appear to be improvements. What do I need to do to regenerate the golden results to make this pass?

@djns99 djns99 force-pushed the BLIFFinalBufferEliminate branch from 01c8756 to 58d57ce Compare September 2, 2020 01:10
@probot-autolabeler probot-autolabeler bot added the VTR Flow VTR Design Flow (scripts/benchmarks/architectures) label Sep 5, 2020
@djns99 djns99 force-pushed the BLIFFinalBufferEliminate branch from 56059e7 to 57a4a57 Compare September 5, 2020 00:28
@djns99
Copy link
Author

djns99 commented Sep 5, 2020 •
edited
Loading

Running the regression tests (with -create_golden to regenerate golden) for basic, strong and nightly I get this error:

vtr_func_formal:          k6_frac_N10_40nm.xml/stereovision3.v/common           			file        :  abc.lec.out
failed: Couldn't determine Logical Equivalence status between stereovision3.odin.blif <-> sv_chip3_hierarchy_no_mem_post_synthesis.blif (took 3.21 seconds)

I assume this means ABC somehow broke with these changes?

My concern here is that I believe this modification must definitionally be identical behaviour, since the driving net can only have a fanout of one (otherwise I do not do this optimisation) so the drivers of this net must necessarily assign directly to the output wire.

The only thing I can think is that somehow there is a combinational loop formed somewhere, but I dont see how this could be possible since fanout = 1 and we dont have InOuts yet (and if we did it would be a bug in stereovision not Odin II)

@jeanlego Thoughts?

jeanlego
jeanlego reviewed Sep 10, 2020
View reviewed changes
ODIN_II/SRC/output_blif.cpp
if (net->num_fanout_pins <= 1) {
for (int i = 0; i < net->num_driver_pins; i++) {
npin_t* driver = net->driver_pins[i];
if (driver->name != NULL && ((driver->node->type == MULTIPLY) || (driver->node->type == HARD_IP) || (driver->node->type == MEMORY) || (driver->node->type == ADD) || (driver->node->type == MINUS))) {
Copy link
Contributor

@jeanlego jeanlego Sep 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I don't forsee this being used for anything else but output. can you assert that node->type is OUTPUT_NODE
I would rather not see this used anywhere else in the future unless theres a good reason for it, so guarding it should make that clear

Copy link
Author

Choose a reason for hiding this comment

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

I assume you mean the input node? The driver can be anything that drives an output yeah?

@vaughnbetz
Copy link
Contributor

Does @jeanlego's suggested code change above fix the logical equivalence failure or is there still an issue with that?

@jeanlego
Copy link
Contributor

jeanlego commented Sep 10, 2020 •
edited
Loading

Does @jeanlego's suggested code change above fix the logical equivalence failure or is there still an issue with that?

Sadly no but I'm out for the week so I can't pull the code to diff the resulting blif

@djns99 djns99 force-pushed the BLIFFinalBufferEliminate branch 2 times, most recently from fc252f3 to d313bb7 Compare September 14, 2020 03:22
@djns99 djns99 force-pushed the BLIFFinalBufferEliminate branch from d313bb7 to be50572 Compare September 15, 2020 04:18
@vaughnbetz
Copy link
Contributor

Assigning this to @sdamghan to see if he wants to finish this PR, or abandon it.

@sdamghan
Copy link
Member

@vaughnbetz not sure why this is left, I was not aware of the progress of this PR. Will investigate more in the future

@poname poname deleted the BLIFFinalBufferEliminate branch January 24, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeanlego jeanlego jeanlego left review comments

Assignees

@sdamghan sdamghan

Labels
lang-cpp C/C++ code Odin Odin II Logic Synthesis Tool: Unsorted item tests VTR Flow VTR Design Flow (scripts/benchmarks/architectures)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@djns99 @vaughnbetz @jeanlego @sdamghan