Reintroduce `mock_area` feature #146

eszpotanski · 2024-09-13T11:21:47Z

This PR reintroduces mock_area feature: creating additional mock area targets, which scale CORE_AREA and DIE_AREA, and use them to run floorplan and generate_abstract.

This PR also contains some minor README cleanup, like removing information about canonicalize targets.

oharboe · 2024-09-13T11:58:47Z

I expected the .lef file from the mocked area to replace the regular .lef file for lb_32x128_generate_abstract.

Smoketest, changing area doesn't seem to change size of .lef or trigger a rebuild:

oyvind@gale:~/bazel-orfs$ git diff
diff --git a/BUILD b/BUILD
index 1810b4f..4bff9b3 100644
--- a/BUILD
+++ b/BUILD
@@ -101,7 +101,7 @@ LB_VERILOG_FILES = ["test/rtl/lb_32x128.sv"]
 orfs_flow(
     name = "lb_32x128",
     abstract_stage = "floorplan",
-    mock_area = 1.0,
+    mock_area = 0.8,
     stage_args = LB_STAGE_ARGS,
     stage_sources = LB_STAGE_SOURCES,
     verilog_files = LB_VERILOG_FILES,
oyvind@gale:~/bazel-orfs$ bazel build lb_32x128_generate_abstract
INFO: Analyzed target //:lb_32x128_generate_abstract (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:lb_32x128_generate_abstract up-to-date:
  bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef
  bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lib
INFO: Elapsed time: 0.080s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
oyvind@gale:~/bazel-orfs$ grep " SIZE " bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef
  SIZE 58.482 BY 116.28 ;
oyvind@gale:~/bazel-orfs$ git diff
diff --git a/BUILD b/BUILD
index 1810b4f..e46378d 100644
--- a/BUILD
+++ b/BUILD
@@ -101,7 +101,7 @@ LB_VERILOG_FILES = ["test/rtl/lb_32x128.sv"]
 orfs_flow(
     name = "lb_32x128",
     abstract_stage = "floorplan",
-    mock_area = 1.0,
+    mock_area = 1,
     stage_args = LB_STAGE_ARGS,
     stage_sources = LB_STAGE_SOURCES,
     verilog_files = LB_VERILOG_FILES,
oyvind@gale:~/bazel-orfs$ bazel build lb_32x128_generate_abstract
INFO: Analyzed target //:lb_32x128_generate_abstract (1 packages loaded, 11 targets configured).
INFO: Found 1 target...
Target //:lb_32x128_generate_abstract up-to-date:
  bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef
  bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lib
INFO: Elapsed time: 0.166s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
oyvind@gale:~/bazel-orfs$ grep " SIZE " bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef
  SIZE 58.482 BY 116.28 ;

oharboe

Needs some fixes...

mock_area.tcl

openroad.bzl

oharboe · 2024-09-13T12:00:38Z

.github/workflows/ci.yml

@@ -40,6 +40,7 @@ jobs:
          - "tag_array_64x184_report"
          - "lb_32x128_generate_abstract"
          - "lb_32x128_test_generate_abstract"
+          - "lb_32x128_generate_abstract_mock_area"


We could make this only an action and not a new target, though debugging locally with _mock_area_deps could be useful...

Do you mean to not test lb_32x128_generate_abstract_mock_area target as separate job in matrix element? Or to not create *_mock_area targets at all?

not to create *_mock_area at all. To the user this is an implementation detail.

This target is no longer present

eszpotanski · 2024-09-13T12:22:13Z

lb_32x128_generate_abstract doesn't mock area, it's a normal target.
Only the *_mock_area targets are influenced by mock_area parameter.

mock_area parameter is used in mock_area.tcl, therefore only targets from lb_32x128_mock_area are rebuilt when it changes:

oharboe · 2024-09-13T12:25:06Z

lb_32x128_generate_abstract doesn't mock area, it's a normal target. Only the *_mock_area targets are influenced by mock_area parameter.

I see. I don't actually need or want extra targets, mocking area is just a property of the existing target, we're scaling the size of the target.

When mocking area, lb_32x128_geneate_abstract should generate a .lib file and mocked .lef file. With mock_area not specified, the real .lef file should be used.

oharboe · 2024-09-13T12:41:37Z

See https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/091a06bfa82f55e01fcdb456efe9580a3b2d8a63/flow/scripts/floorplan.tcl#L67

This won't work:

$ cat bazel-bin/objects/asap7/lb_32x128/base/scaled_area.env
export DIE_AREA="0 0 246.95250000000001 490.90500000000003"
export CORE_AREA="1.539 1.62  245.43  489.24"

It needs to be:

export DIE_AREA="0 0 246.95250000000001 490.90500000000003"
export CORE_AREA="1.539 1.62  245.43  489.24"
export CORE_UTILIZATION=

oharboe · 2024-09-13T12:52:51Z

As a simplification, you can use args instead of stage_args parameter and let bazel-orfs distribute the parameters to the requisite stages...

Something like:

-LB_STAGE_ARGS = {
-    "synth": SRAM_SYNTH_ARGUMENTS,
-    "floorplan": SRAM_FLOOR_PLACE_ARGUMENTS | {
-        "CORE_UTILIZATION": "40",
+LB_ARGS = (SRAM_SYNTH_ARGUMENTS |
+    SRAM_FLOOR_PLACE_ARGUMENTS | {
+        "CORE_UTILIZATION": "5",
         "CORE_ASPECT_RATIO": "2",
-    },
-    "place": SRAM_FLOOR_PLACE_ARGUMENTS | {"PLACE_DENSITY": "0.65"},
-}
+        "PLACE_DENSITY": "0.65"})
 
 LB_STAGE_SOURCES = {
     "synth": [":constraints-sram"],
@@ -102,8 +99,8 @@ LB_VERILOG_FILES = ["test/rtl/lb_32x128.sv"]
 orfs_flow(
     name = "lb_32x128",
     abstract_stage = "floorplan",
-    mock_area = 1.0,
-    stage_args = LB_STAGE_ARGS,
+    mock_area = 1.5,
+    args = LB_ARGS,

eszpotanski · 2024-09-13T12:56:03Z

I see. I don't actually need or want extra targets, mocking area is just a property of the existing target, we're scaling the size of the target.

The mock_area.tcl requires results from non-mocked floorplan (https://github.com/The-OpenROAD-Project/bazel-orfs/blob/main/mock_area.tcl#L1), so if there is no extra targets, we will end up in situation where each time mock_area is changed floorplan will be built twice (first non-mocked and then mocked).
Currently, results of non-mocked stages are cached, so whole flow is faster.

Do you want to have no extra targets, or is it OK to e.g. rename current targets, like:

*_mock_area to default names, e.g. lb_32x128_generate_abstract_mock_area to lb_32x128_generate_abstract, and
append suffix to default names, e.g. lb_32x128_generate_abstract to lb_32x128_generate_abstract_non_mocked?

oharboe · 2024-09-13T13:06:40Z

It is only generate_abstract that needs the mocked .lef file from the mocked floorplan stage, so the regular flow should be unchanged based on a mocked floorplan. Only generate_abstract has to run again, the .lef file from generate_abstract should be overwritten by the mock area .lef

Signed-off-by: Eryk Szpotanski <[email protected]>

eszpotanski · 2024-09-13T14:01:17Z

Applied suggestions with export CORE_UTILIZATION=, LB_ARGS and changed flow behavior, so the mock_area parameter overrides *_generate_abstract targets - _generate_abstract produces mocked results if mock_area is used.

oharboe

I don't see a difference in size for .lef file when I change mock_area:

$ bazel build lb_32x128_generate_abstract && grep " SIZE " bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef
[deleted]
SIZE 163.62 BY 326.52 ;

README.md

oharboe · 2024-09-13T14:05:56Z

Also, when I change abstract stage, it doesn't rebuild.

To recap: I expect a mocked .lef replacing the real .lef file during generate_abstract and the .lib file unchanged for the target below when using mock_area:

$ bazel build lb_32x128_generate_abstract && grep " SIZE " bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef
INFO: Analyzed target //:lb_32x128_generate_abstract (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:lb_32x128_generate_abstract up-to-date:
  bazel-bin/results/asap7/lb_32x128/mock_area/lb_32x128.lef
  bazel-bin/results/asap7/lb_32x128/mock_area/lb_32x128.lib
INFO: Elapsed time: 0.055s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
  SIZE 163.62 BY 326.52 ;
oyvind@gale:~/bazel-orfs$ git diff
diff --git a/BUILD b/BUILD
index 20bdea0..02d25a4 100644
--- a/BUILD
+++ b/BUILD
@@ -93,7 +93,7 @@ LB_VERILOG_FILES = ["test/rtl/lb_32x128.sv"]
 
 orfs_flow(
     name = "lb_32x128",
-    abstract_stage = "floorplan",
+    abstract_stage = "route",
     mock_area = 1.0,
     args = LB_ARGS,
     stage_sources = LB_STAGE_SOURCES,

oharboe · 2024-09-13T14:45:20Z

mock_area.tcl

 [expr $scale*([$die_bbox xMax] - ([$die_bbox xMax] - [$core_bbox xMax]))] \
 [expr $scale*([$die_bbox yMax] - ([$die_bbox yMax] - [$core_bbox yMax]))]\""
+tee $file "export CORE_UTILIZATION="


The need for this line would have been caught by The-OpenROAD-Project/OpenROAD-flow-scripts#2344

Signed-off-by: Eryk Szpotanski <[email protected]>

eszpotanski · 2024-09-13T14:55:19Z

This mock_area implementation is based on previous version, which only used synth, floorplan and generate_abstract, therefore abstract_stage is ignored.

I adjusted it and abstract_stage is used properly.

Results were placed in mock_area variant and not the base - now it's fixed and tested with grep " SIZE " bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef command.

oharboe · 2024-09-13T15:04:28Z

$ git diff
$ bazel build lb_32x128_generate_abstract && grep " SIZE " bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef
[deleted]
  SIZE 58.482 BY 116.28 ;
$ git diff
diff --git a/BUILD b/BUILD
index 20bdea0..766c2fe 100644
--- a/BUILD
+++ b/BUILD
@@ -94,7 +94,7 @@ LB_VERILOG_FILES = ["test/rtl/lb_32x128.sv"]
 orfs_flow(
     name = "lb_32x128",
     abstract_stage = "floorplan",
-    mock_area = 1.0,
+    mock_area = 1.5,
     args = LB_ARGS,
     stage_sources = LB_STAGE_SOURCES,
     verilog_files = LB_VERILOG_FILES,
$ bazel build lb_32x128_generate_abstract && grep " SIZE " bazel-bin/results/asap7/lb_32x128/base/lb_32x128.lef
[deleted]
  SIZE 87.696 BY 174.744 ;

eszpotanski force-pushed the 65089-mock-area branch from b6a7a50 to ddf864a Compare September 13, 2024 11:39

oharboe self-requested a review September 13, 2024 11:58

oharboe requested changes Sep 13, 2024

View reviewed changes

mock_area.tcl Outdated Show resolved Hide resolved

openroad.bzl Outdated Show resolved Hide resolved

oharboe reviewed Sep 13, 2024

View reviewed changes

eszpotanski added 2 commits September 13, 2024 15:53

mock_area: Save scaled variables to given file

4ebd2e5

Signed-off-by: Eryk Szpotanski <[email protected]>

make: Source file with additional variables

835f6c9

Signed-off-by: Eryk Szpotanski <[email protected]>

eszpotanski force-pushed the 65089-mock-area branch from ddf864a to 7f448a4 Compare September 13, 2024 13:53

oharboe requested changes Sep 13, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

oharboe reviewed Sep 13, 2024

View reviewed changes

eszpotanski force-pushed the 65089-mock-area branch from 7f448a4 to c8c3d6d Compare September 13, 2024 14:50

eszpotanski added 3 commits September 13, 2024 16:51

openroad: Add mock_area targets implementeation

25ebe55

Signed-off-by: Eryk Szpotanski <[email protected]>

BUILD: Mock area for lb_32x128

6809f73

Signed-off-by: Eryk Szpotanski <[email protected]>

README: Add mock_area description

20f7250

Signed-off-by: Eryk Szpotanski <[email protected]>

eszpotanski force-pushed the 65089-mock-area branch from c8c3d6d to 20f7250 Compare September 13, 2024 14:51

oharboe merged commit 6d2d73e into The-OpenROAD-Project:main Sep 13, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reintroduce `mock_area` feature #146

Reintroduce `mock_area` feature #146

eszpotanski commented Sep 13, 2024

oharboe commented Sep 13, 2024 •

edited

Loading

oharboe left a comment

oharboe Sep 13, 2024

eszpotanski Sep 13, 2024

oharboe Sep 13, 2024

eszpotanski Sep 13, 2024

eszpotanski commented Sep 13, 2024

oharboe commented Sep 13, 2024

oharboe commented Sep 13, 2024

oharboe commented Sep 13, 2024

eszpotanski commented Sep 13, 2024

oharboe commented Sep 13, 2024 •

edited

Loading

eszpotanski commented Sep 13, 2024

oharboe left a comment

oharboe commented Sep 13, 2024

oharboe Sep 13, 2024

eszpotanski commented Sep 13, 2024

oharboe commented Sep 13, 2024

Reintroduce mock_area feature #146

Reintroduce mock_area feature #146

Conversation

eszpotanski commented Sep 13, 2024

oharboe commented Sep 13, 2024 • edited Loading

oharboe left a comment

Choose a reason for hiding this comment

oharboe Sep 13, 2024

Choose a reason for hiding this comment

eszpotanski Sep 13, 2024

Choose a reason for hiding this comment

oharboe Sep 13, 2024

Choose a reason for hiding this comment

eszpotanski Sep 13, 2024

Choose a reason for hiding this comment

eszpotanski commented Sep 13, 2024

oharboe commented Sep 13, 2024

oharboe commented Sep 13, 2024

oharboe commented Sep 13, 2024

eszpotanski commented Sep 13, 2024

oharboe commented Sep 13, 2024 • edited Loading

eszpotanski commented Sep 13, 2024

oharboe left a comment

Choose a reason for hiding this comment

oharboe commented Sep 13, 2024

oharboe Sep 13, 2024

Choose a reason for hiding this comment

eszpotanski commented Sep 13, 2024

oharboe commented Sep 13, 2024

Reintroduce `mock_area` feature #146

Reintroduce `mock_area` feature #146

oharboe commented Sep 13, 2024 •

edited

Loading

oharboe commented Sep 13, 2024 •

edited

Loading