[WIP] PlacementAnnealer class #2800
Conversation
github-actions
bot
added
VPR
| @@ -86,6 +86,12 @@ enum class ScreenUpdatePriority { | |||
| MAJOR = 1 | |||
| }; | |||
|
|
|||
| #ifdef VTR_ENABLE_DEBUG_LOGGING | |||
| constexpr bool VTR_ENABLE_DEBUG_LOGGING_CONST_EXPR = true; | |||
There was a problem hiding this comment.
Could shorten to VTR_ENABLE_DEBUG_LOGGING_CE
| @@ -23,9 +23,6 @@ | |||
| #include "route_export.h" | |||
| #include "tatum/report/TimingPathCollector.hpp" | |||
|
|
|||
| #ifdef VTR_ENABLE_DEBUG_LOGGING | |||
| # include "move_utils.h" | |||
| #endif | |||
|
|
|||
| #ifdef WIN32 /* For runtime tracking in WIN32. The clock() function defined in time.h will * | |||
There was a problem hiding this comment.
I wonder if we still use these timer functions? I think we've moved to chrono?
There was a problem hiding this comment.
We probably shouldn't call clock(), and instead should use the various chrono & vtr utilities.
|
|
||
| #ifdef VTR_ENABLE_DEBUG_LOGGING | ||
| # include "move_utils.h" | ||
| #endif | ||
|
|
||
| #ifdef WIN32 /* For runtime tracking in WIN32. The clock() function defined in time.h will * |
There was a problem hiding this comment.
Can we get rid of this #ifdef and use chrono / vtr time utilities?
| if (in_quench) { | ||
| if (placer_opts.place_quench_algorithm.is_timing_driven() && placer_opts.place_agent_multistate) | ||
| current_move_generator = std::move(move_generator2); | ||
| return *move_generator2; |
There was a problem hiding this comment.
This seems to break unique_ptr semantics. @AlexandreSinger @MohamedElgammal any thoughts?
| get_bp_state_globals()->get_glob_breakpoint_state()->blocks_affected_by_move.clear(); | ||
| for (size_t i = 0; i < blocksAffected.moved_blocks.size(); i++) { | ||
| //size_t conversion is required since block_num is of type ClusterBlockId and can't be cast to an int. And this vector has to be of type int to be recognized in expr_eval class | ||
| BreakpointState* bp_state = get_bp_state_globals()->get_glob_breakpoint_state(); |
There was a problem hiding this comment.
If we were able to move the breakpiont state inside a placer context it would be cleaner I think. You don't have to do that if you can't see a clean way to do it though ....
| auto& cluster_ctx = g_vpr_ctx.clustering(); | ||
| float t_exit = 0.005 * costs.cost / cluster_ctx.clb_nlist.nets().size(); | ||
|
|
||
| if (placer_opts.anneal_sched.type == e_sched_type::DUSTY_SCHED) { |
There was a problem hiding this comment.
Could get rid of DUSTY_SCHED if you like ... not used.
| /* Get the setup slack analysis cost */ | ||
| //TODO: calculate a weighted average of the slack cost and wiring cost | ||
| delta_c = analyze_setup_slack_cost(setup_slacks_, placer_state_) * costs_.timing_cost_norm; | ||
| } else if (place_algorithm == e_place_algorithm::CRITICALITY_TIMING_PLACE) { |
There was a problem hiding this comment.
This is the default code path, so moving it to be first in the if would be slightly more clear.
There was a problem hiding this comment.
Can also say this placement algorithm (CRITICALITY_TIMING_PLACE) mixes timing and wiring terms, and works with somewhat stale timing data (periodically updated) to save CPU time.
|
|
||
| //For setup slack analysis, we first do a timing analysis to get the newest | ||
| //slack values resulted from the proposed block moves. If the move turns out | ||
| //to be accepted, we keep the updated slack values and commit the block moves. |
There was a problem hiding this comment.
This comment should be moved inside the if, as it is only for the SLACK_TIMING_PLACE option. Also would be good to say this option evaluates moves with up-to-date timing information, which is more expensive but more accurate.
| commit_setup_slacks(setup_slacks_, placer_state_); | ||
| } | ||
|
|
||
| if (place_algorithm == e_place_algorithm::CRITICALITY_TIMING_PLACE) { |
There was a problem hiding this comment.
Suggest putting this algorithm first, as it is the default. Also can make an if ... else if instead of two ifs
| // Clear the data structure containing block move info | ||
| blocks_affected_.clear_move_blocks(); | ||
|
|
||
| VTR_LOGV_DEBUG(g_vpr_ctx.placement().f_placer_debug, |
There was a problem hiding this comment.
This is a very long routine. Can anything be factored into helper functions?
This PR encapsulates simulated annealing functions and objects into a class.