[WIP][AP][APPack] Made max distance th aware of compressed grid - #3793
[WIP][AP][APPack] Made max distance th aware of compressed grid#3793AlexandreSinger wants to merge 1 commit into
Conversation
For sparse logical block types (like DSPs), I found that the max distance threshold was cutting far more connections that it should be. For DSP, the fact that they were in columns caused the distances between the columns to appear very far away; however, those two columns are relatively close in "DSP space". I added a method to the max distance threshold manager class that computes how many "valid tiles" for a given type exist between two points. This is similar to how the compressed grid works in placement. I considered using the compressed grid for this application; but I found that it does not translate well to APPack since molecules may exist in locations which do not align with a valid type. I chose to roll my own prefix-sum approach which ensures that computing the distance is always O(1). This is performanced for the max distance calculation for all logical block types (including CLBs); however, gain attenuation still uses flat position. This is to still encode positional information from global placement.
|
Marking as WIP as I run some tests, but should be ready for review. There will be some golden result failures, those will be fixed after this is reviewed and is ready to merge. |
github-actions
Bot
added
VPR
|
Note: It looks like I will need to retune the max distance thresholds. Unfortunately this slightly changes the definition of distance for max_dist_th; so the numbers no longer make sense. I will check titan_quick_qor; but if it looks bad I will have to do a quick sweep. |
|
Results on titan_quick_qor:
The device size did go down, but the number of DSP went up. I did change the DSPs to be tighter; perhaps I made them too tight. I could try to return DSP to its original value, but I am concerned that it may hurt the quality on fixed architectures without the multiplier change in another PR. What I might do is ask people to review this PR and then merge it into my other branch so it can be part of that feature. |
haydar-c
left a comment
haydar-c
left a comment
There was a problem hiding this comment.
Thanks @AlexandreSinger! I added a question about retuning and naming functions. Otherwise, looks good to me!
| float dist = get_manhattan_distance(mol_loc, cluster_gain_stats.flat_cluster_position); | ||
| float dist = appack_ctx.max_distance_threshold_manager.get_distance_between_points(mol_loc, | ||
| cluster_gain_stats.flat_cluster_position, | ||
| cluster_type); |
There was a problem hiding this comment.
The dist here is on "valid hops / compressed grid" units. I am not sure how the max_dist parameters tuned (for compressed or regular grid). Would that require to retune the max_dist parameters of scale and offset for the blocks that are affected?
There was a problem hiding this comment.
I just saw your comment about retuning and it answers this question.
| * | ||
| * loc1 and loc2 are assumed to lie within the device grid. | ||
| */ | ||
| float get_distance_between_points(const t_flat_pl_loc& loc1, |
There was a problem hiding this comment.
The naming here might be a bit misleading for the users of class. Indicating the distance returned is in the units of valid/compatible tiles might be better API naming (something like get_distance_between_points_in_compatible_tiles), but I understand that we don't want to make names too long. This is not blocking and up to you.
| * | ||
| * The loc1 and loc2 coordinates are assumed to lie within the device grid. | ||
| */ | ||
| static unsigned get_num_tiles_between(const vtr::PrefixSum2D<unsigned>& lookup, |
There was a problem hiding this comment.
The same naming comment as in get_distance_between_points to make it explicit that it is calculated in valid hops or compressed grid like distances.
2 participants
For sparse logical block types (like DSPs), I found that the max distance threshold was cutting far more connections that it should be. For DSP, the fact that they were in columns caused the distances between the columns to appear very far away; however, those two columns are relatively close in "DSP space".
I added a method to the max distance threshold manager class that computes how many "valid tiles" for a given type exist between two points. This is similar to how the compressed grid works in placement.
I considered using the compressed grid for this application; but I found that it does not translate well to APPack since molecules may exist in locations which do not align with a valid type. I chose to roll my own prefix-sum approach which ensures that computing the distance is always O(1).
This is performed for the max distance calculation for all logical block types (including CLBs); however, gain attenuation still uses flat position. This is to still encode positional information from global placement.