if stopping after packing skip unnecessary placement setup

[KA7E]

If running packing but not placement, return before unnecessary placement data structure setup.

Description

If packing returns success, added check to see if placement will run; if not, return.

Related Issue

Motivation and Context

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

[vaughnbetz]

CI on the google cloud runners is down again, unfortunately, so lots of tests didn't run. @AlexandreSinger is looking into it.
Please run tests on one design with:
--route
--analysis
--place
--pack

to make sure all the flows work with this change. From code inspection I am worried --route and --analysis might not work with the change.

[vaughnbetz]

Will this work if --route is specified? In that case packing is skipped (read from file), placement and skipped (read from file) but routing is performed. Same question is --analysis is specified. This change looks like it could break those flows.

[KA7E]

doPlacement is an enum:

enum e_stage_action {
STAGE_SKIP = 0,
STAGE_LOAD,
STAGE_DO,
STAGE_AUTO
};

So the value checked on line 386 (!vpr_setup.PlacerOpts.doPlacement) is only true when doPlacement is set to STAGE_SKIP. If Analysis or Routing is run but Placement is not, doPlacement is set to STAGE_LOAD, which does not trigger the early return in line 387.

[KA7E]

Oh right it should check all the flags not just placement.

…

On Thu, Aug 8, 2024, 12:47 vaughnbetz ***@***.***> wrote: ***@***.**** requested changes on this pull request. CI on the google cloud runners is down again, unfortunately, so lots of tests didn't run. @AlexandreSinger <https://github.com/AlexandreSinger> is looking into it. Please run tests on one design with: --route --analysis --place --pack to make sure all the flows work with this change. From code inspection I am worried --route and --analysis might not work with the change. ------------------------------ In vpr/src/base/vpr_api.cpp <#2680 (comment)> : > @@ -383,6 +383,9 @@ bool vpr_flow(t_vpr_setup& vpr_setup, t_arch& arch) { if (!pack_success) { return false; //Unimplementable } Will this work if --route is specified? In that case packing is skipped (read from file), placement and skipped (read from file) but routing is performed. Same question is --analysis is specified. This change looks like it could break those flows. — Reply to this email directly, view it on GitHub <#2680 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AM6ZD4SSOT2CMWAMT7AL3G3ZQOOLZAVCNFSM6AAAAABMFO4DMCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMRYGM4TIOBQGU> . You are receiving this because you authored the thread.Message ID: <verilog-to-routing/vtr-verilog-to-routing/pull/2680/review/2228394805@ github.com>