mpl: ignore the macro being placed in overlap checks - #11322
mpl: ignore the macro being placed in overlap checks#11322naveenvenk17 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request prevents a macro from checking overlap with itself in findOverlappedMacros inside rtl_mp.cpp. It also updates Bazel test targets and introduces several new tests, including place_macro_right_angle_rotation. The review feedback points out that the golden file place_macro_right_angle_rotation.defok was placed in the wrong directory (testcases/), which will cause Bazel test failures, and should be moved to the parent test directory.
| set def_file [make_result_file place_macro_right_angle_rotation.def] | ||
| write_def $def_file | ||
|
|
||
| diff_files place_macro_right_angle_rotation.defok $def_file |
There was a problem hiding this comment.
The golden file place_macro_right_angle_rotation.defok is added under src/mpl/test/testcases/, but this diff_files call expects it in the current directory (src/mpl/test/). Additionally, the Bazel glob in src/mpl/test/BUILD (glob(["place_macro_right_angle_rotation.*"])) will not find it inside testcases/, which will cause the Bazel test to fail. To resolve this and align with the repository's conventions, please move the golden file from src/mpl/test/testcases/place_macro_right_angle_rotation.defok to src/mpl/test/place_macro_right_angle_rotation.defok.
6a5edfd to
a25e3af
Compare
|
Updated for current master. The golden output now comes from the existing parent test location, so there is no separate misplaced The regression now first marks the target macro as placed, then moves it. That exercises the self-overlap path directly. I also squashed the branch into one signed commit. Validation on the GCP VM:
|
AcKoucher
left a comment
There was a problem hiding this comment.
@naveenvenk17 Thanks for fixing this!
a25e3af to
0578744
Compare
Signed-off-by: Naveen Venkat <archgen.guest@nyayanidhi.in>
0578744 to
73d1ab9
Compare
|
Thanks, I split that setup out.
The new regression has its own expected log and DEF files, plus Bazel and CMake registration. I rebased this on current master and validated it on the GCP VM:
I also had the final test split reviewed separately before the last test run; there were no blocking findings. |
AcKoucher
left a comment
There was a problem hiding this comment.
@naveenvenk17 Please, do not force-push after a review, because we lose history.
| $macro setLocation 40000 40000 | ||
| $macro setPlacementStatus PLACED | ||
|
|
||
| place_macro -macro_name macro -location {20 20} -orientation R90 |
There was a problem hiding this comment.
Please, use an input DEF that does not require setting -orientation that way we keep the test case narrower.
|
You are right. I rebased and force-pushed to keep the branch current, but that removed review history. I will keep any further updates as additive commits and will not force-push this PR again. |
Fixes #11269.
place_macro sets the target macro location before checking for overlaps. The overlap scan then finds that same placed macro and reports a self-overlap, so a valid placement fails unless overlap checking is disabled.
Skip the instance under placement while keeping the checks against every other placed macro. The existing right-angle placement regression now marks the macro placed before calling place_macro, covering the failing path.
Validation on the clean OpenROAD VM: