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

✨ Remove dangling PIs and redundant POs during network parsing #321

Closed

Conversation

simon1hofmann
Copy link
Collaborator

@simon1hofmann simon1hofmann commented Nov 2, 2023

Description

Remove dangling PIs and redundant POs during network conversion.

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

@simon1hofmann simon1hofmann added the enhancement New feature or request label Nov 2, 2023
@simon1hofmann simon1hofmann self-assigned this Nov 2, 2023
@simon1hofmann simon1hofmann changed the title ✨ Add options to remove dangling PIs and POs ✨ Add possibility to remove dangling PIs and redundant POs Nov 2, 2023
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Merging #321 (a59c86d) into main (481331f) will decrease coverage by 0.07%.
Report is 3 commits behind head on main.
The diff coverage is 100.00%.

❗ Current head a59c86d differs from pull request most recent head c377770. Consider uploading reports for the commit c377770 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #321      +/-   ##
==========================================
- Coverage   96.06%   95.99%   -0.07%     
==========================================
  Files         102      100       -2     
  Lines       10086     9966     -120     
==========================================
- Hits         9689     9567     -122     
- Misses        397      399       +2     
Files Coverage Δ
...thms/network_transformation/network_conversion.hpp 82.53% <100.00%> (+0.28%) ⬆️
.../fiction/algorithms/physical_design/orthogonal.hpp 93.80% <100.00%> (-1.39%) ⬇️
include/fiction/io/network_reader.hpp 65.95% <100.00%> (ø)

... and 19 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cb380a...c377770. Read the comment docs.

@simon1hofmann simon1hofmann changed the title ✨ Add possibility to remove dangling PIs and redundant POs ✨ Remove dangling PIs and redundant POs during network parsing Nov 2, 2023
Copy link
Contributor

github-actions bot commented Nov 2, 2023

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

github-actions bot commented Nov 7, 2023

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

const auto nets = reader.get_networks();
const auto tec_net = *nets.front();

return tec_net;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: constness of 'tec_net' prevents automatic move [performance-no-automatic-move]

    return tec_net;
           ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

"equivalent"};

fiction::orthogonal_physical_design_stats orthogonal_stats{};
fiction::post_layout_optimization_stats optimization_stats{};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'optimization_stats' of type 'fiction::post_layout_optimization_stats' can be declared 'const' [misc-const-correctness]

Suggested change
fiction::post_layout_optimization_stats optimization_stats{};
fiction::post_layout_optimization_stats const optimization_stats{};

"equivalent"};

fiction::orthogonal_physical_design_stats orthogonal_stats{};
fiction::post_layout_optimization_stats optimization_stats{};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: unused variable 'optimization_stats' [clang-diagnostic-unused-variable]

    fiction::post_layout_optimization_stats optimization_stats{};
                                            ^

std::vector<mockturtle::gate> gates{};

// parameters for technology mapping
mockturtle::map_params map_params{};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'map_params' of type 'mockturtle::map_params' can be declared 'const' [misc-const-correctness]

Suggested change
mockturtle::map_params map_params{};
mockturtle::map_params const map_params{};

// parameters for technology mapping
mockturtle::map_params map_params{};

const auto read_genlib_result = lorina::read_genlib(library_stream, mockturtle::genlib_reader{gates});
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: Value stored to 'read_genlib_result' during its initialization is never read [clang-analyzer-deadcode.DeadStores]

    const auto read_genlib_result = lorina::read_genlib(library_stream, mockturtle::genlib_reader{gates});
               ^
Additional context

experiments/mnt_bench/ortho_layouts_row.cpp:89: Value stored to 'read_genlib_result' during its initialization is never read

    const auto read_genlib_result = lorina::read_genlib(library_stream, mockturtle::genlib_reader{gates});
               ^

// parameters for technology mapping
mockturtle::map_params map_params{};

const auto read_genlib_result = lorina::read_genlib(library_stream, mockturtle::genlib_reader{gates});
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: unused variable 'read_genlib_result' [clang-diagnostic-unused-variable]

    const auto read_genlib_result = lorina::read_genlib(library_stream, mockturtle::genlib_reader{gates});
               ^


fiction::orthogonal_physical_design_stats orthogonal_stats{};
fiction::hexagonalization_stats hexagonalization_stats{};
fiction::post_layout_optimization_stats optimization_stats{};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'optimization_stats' of type 'fiction::post_layout_optimization_stats' can be declared 'const' [misc-const-correctness]

Suggested change
fiction::post_layout_optimization_stats optimization_stats{};
fiction::post_layout_optimization_stats const optimization_stats{};


fiction::orthogonal_physical_design_stats orthogonal_stats{};
fiction::hexagonalization_stats hexagonalization_stats{};
fiction::post_layout_optimization_stats optimization_stats{};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: unused variable 'optimization_stats' [clang-diagnostic-unused-variable]

    fiction::post_layout_optimization_stats optimization_stats{};
                                            ^

// rewrite network cuts using the given re-synthesis function
const auto cut_net = mockturtle::cut_rewriting(net, resynthesis_function, cut_params);
// compute depth
mockturtle::depth_view depth_cut_net{cut_net};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'depth_cut_net' of type 'mockturtle::depth_view<names_view<technology_network>>' (aka 'depth_view<mockturtle::names_viewfiction::technology_network>') can be declared 'const' [misc-const-correctness]

Suggested change
mockturtle::depth_view depth_cut_net{cut_net};
mockturtle::depth_view const depth_cut_net{cut_net};

fiction::restore_names(net, mapped_network);

// compute depth
mockturtle::depth_view depth_mapped_network{mapped_network};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'depth_mapped_network' of type 'mockturtle::depth_view<names_view<technology_network>>' (aka 'depth_view<mockturtle::names_viewfiction::technology_network>') can be declared 'const' [misc-const-correctness]

Suggested change
mockturtle::depth_view depth_mapped_network{mapped_network};
mockturtle::depth_view const depth_mapped_network{mapped_network};

fiction::write_fgl_layout(gate_level_layout, fmt::format("{}/{}_Bestagon_ROW_ortho_UnOpt_UnOrd.fgl", layouts_folder, benchmark));

// check equivalence
fiction::equivalence_checking_stats eq_stats{};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'eq_stats' of type 'fiction::equivalence_checking_stats' can be declared 'const' [misc-const-correctness]

Suggested change
fiction::equivalence_checking_stats eq_stats{};
fiction::equivalence_checking_stats const eq_stats{};

@simon1hofmann simon1hofmann marked this pull request as draft November 28, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants