[Place] Independent Placement Verification

[AlexandreSinger]

Created a method that would independently verify the placement in the VPR flow. If a placement passes this verification, it is assumed that it can be used in routing without issue.

By design, this method does not use any global variables (everything needs to be passed in) and recomputes everything that it does not assume. This allows it to be independent of the placement flow being used, so this method can be used in the AP flow as well.

[AlexandreSinger]

@vaughnbetz This is the check_place changes we discussed on Friday. I split out all of the invariants from the check_place method and used it to create this method. The idea of the verify_placement method is to be an independent check to ensure that all the necessary data structures of placement are filled correctly such that the rest of the VPR flow has no issues (nothing more, nothing less). Please review.

I do have a FIXME in this code which I am not sure about. Are the macros required for the rest of the VPR flow? My intention for this code is that this MUST pass in order for the rest of the VPR flow to be used without issue; but I do not see uses of the pl_macros anywhere outside of placement (and drawing). If they are not used then I think this check should happen somewhere else. What do you think?

[vaughnbetz]

If the macros aren't respected generally routing will fail, as the main reason macros are created is for fixed routing patterns (no programmability). So I'd check them.

[soheilshahrouz]

A few checks are done in sync_grid_to_blocks() function. I feel those check belong to verify_placement.cpp.

[vaughnbetz]

This looks good to me. @AlexandreSinger feel free to merge when you are ready. I tend to agree with @soheilshahrouz that a few blank lines to break up concepts are helpful to readability, but will leave it to you to decide if /where to add any blank lines.

[vaughnbetz]

As we discussed the sync_grid_to_blocks() function call could/should be moved inside load_placement, which will make the overall control a bit cleaner. You could also move the post_place_sync () function call into both the end of placement and the end of load_placement if you like, as that would again simplify the overall control.

Merge whenever you see fit ...

[AlexandreSinger]

@soheilshahrouz @vaughnbetz I looked into the sync_grid_to_blocks() method in detail and found something troubling. All of the checks in this function are already in the verifier; however, there is an issue with how blocks are placed in the grid. The placer always places clusters in the root tile location of large blocks (and leaves the other locations in that tile alone); but this method places clusters in ALL locations of a tile (so a 2x2 tile would have the cluster placed in 4 tile locations).

I found that some strong tests (specifically the flat router) rely on this placement style.

This inconsistency makes it impossible to make a full verifier. You would need to call the verifier, then call sync_grid_to_blocks() afterwards and never call the verifier again (since the output of this method would not pass the verified since the verifier is checking that only the root tile location has a placed block). I decided to not let this block this PR; however I have raised an issue on this topic: see issue #2801

I will raise a separate PR to fix this issue, but we should come to a consensus on which of these two is correct. Do we only want to place the clusters at the root tile locations? Or do we want to place the clusters at all tile locations of the large tile?

@vaughnbetz Are you ok with me merging this PR?

[vaughnbetz]

We should only place blocks at the root position in the grid; that's faster, so it's what we want to do during placement. Not sure why sync_blocks_to_grid isn't doing that, but I think we should change it and see if anything breaks (it shouldn't).

I'm OK with merging.

[danahow]

If you want to mark each and every grid covered by a tile, fill in non-root grids with a pointer to the associated root grid. These extra pointers never need to change.

[vaughnbetz]

That's what happens now; the grid has non-root node locations point back to the root node.