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.

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

Avoid strcpy inside of atom netlist. #1477

Merged

Conversation

litghost
Copy link
Collaborator

@litghost litghost commented Aug 12, 2020

Description

Saves around 20 out of 30 seconds in circuit cleaning on baselitex 7-series flow per VPR invocation.

Related Issue

Motivation and Context

We've noticed that the circuit cleaning time was large relative to the overall runtime (30 seconds into 120 second total run). A profile run showed about 20 seconds being spent in a simple string copy to determine if a model is either inpad/outpad.

How Has This Been Tested?

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

@probot-autolabeler probot-autolabeler bot added lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool labels Aug 12, 2020
Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

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

Looks good, but I have a suggestion for an additional comment.

@@ -257,6 +261,10 @@ class AtomNetlist : public Netlist<AtomBlockId, AtomPortId, AtomPinId, AtomNetId
vtr::vector_map<AtomBlockId, const t_model*> block_models_; //Architecture model of each block
vtr::vector_map<AtomBlockId, TruthTable> block_truth_tables_; //Truth tables of each block

//INPAD and OUTPUT models
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to comment these new member variables and why they exist:

Something like:
// Input IOs and output IOs always exist and have their own architecture models.
// While their models are already included in block_models_, we also store direct
// pointers to them to make checks of whether a block is an INPAD or OUTPAD fast,
// as such checks are common in some netlist operations (e.g. clean-up of an input netlist).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@vaughnbetz
Copy link
Contributor

Failure is spurious (just a known memory leak in the graphics test due to the lack of a sanitizer suppression file).

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants