Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use relative paths for build artifacts and sources #32

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

lpawelcz
Copy link
Contributor

This PR reduces the usage of BUILD_DIR env var in favor of relative paths to build artifacts and sources.
It was required to also change the build directory when using the docker flow from ORFS directory to bazel build directory (the same as in local flow) to keep the consistency between the flows in order to use relative paths.

This was tested in the following configurations:

  • docker flow build in bazel-orfs
  • local flow build in bazel-orfs
  • docker flow build in megaboom
  • local flow build in megaboom
  • docker flow build in megaboom with bazel-orfs dependency specified through local_path_override
  • local flow build in megaboom with bazel-orfs dependency specified through local_path_override

@oharboe
Copy link
Collaborator

oharboe commented Apr 24, 2024

I still see lots of absolute paths being used when I run the _make script generated:

bazel build L1MetadataArray_test_floorplan_make
bazel-bin/L1MetadataArray_test_floorplan_make do-floorplan
[deleted output]
[INFO ODB-0227] LEF file: /home/oyvind/OpenROAD-flow-scripts/flow/platforms/asap7/lef/asap7_tech_1x_201209.lef, created 24 layers, 9 vias

Copy link
Collaborator

@oharboe oharboe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also cd BUILD_DIR in the generated _make scripts

purge memory targets, now supported in ORFS directly.

@@ -90,7 +103,7 @@ docker run --name "bazel-orfs-$uuid" --rm \
bash -c \
"set -ex
. ./env.sh
cd \$FLOW_HOME
cd \$BUILD_DIR
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing from the generated _make scripts, near as I can see

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is because in the local flow we are already placed in the right build directory. In the docker flow it is required strictly because of the container default working directory being /OpenROAD-flow-scripts/.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out that with most recent modifications it was actually beneficial to also do that in orfs

@@ -1,7 +1,7 @@
.PHONY: memory
memory: $(RESULTS_DIR)/mem.json
python3 $(BUILD_DIR)/scripts/mem_dump.py $(RESULTS_DIR)/mem.json
python3 $(MEMORY_DUMP_PY) $(RESULTS_DIR)/mem.json
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ORFS now supports memory target natively, purge memory target from bazel-orfs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we can remove those but would that be OK for you if we handle this in a separate follow-up PR?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@lpawelcz
Copy link
Contributor Author

I still see lots of absolute paths being used when I run the _make script generated:

bazel build L1MetadataArray_test_floorplan_make
bazel-bin/L1MetadataArray_test_floorplan_make do-floorplan
[deleted output]
[INFO ODB-0227] LEF file: /home/oyvind/OpenROAD-flow-scripts/flow/platforms/asap7/lef/asap7_tech_1x_201209.lef, created 24 layers, 9 vias

Yes, those are absolute paths but this is the ORFS thing. See: https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/be4b5b5473b4d7c2a54812d5405d043d14b9a057/flow/platforms/asap7/config.mk#L9.
PLATFORM_DIR depends on the FLOW_HOME variable which must point ORFS/flow directory (as in #31). I think it shouldn't be a problem in make issue scenario.
Reproducing the build should be unaffected because the person working on the issue will have FLOW_HOME and PLATFORM_DIR specific to his/her configuration.

When it comes to everything that comes from bazel-orfs: LEFs and other build artifacts, as well as e.g. verilog sources no longer use absolute paths. You can see that in e.g. config files:

L1MetadataArray_test_synth_config.mk

# Stage synth config
export SYNTH_HIERARCHICAL=1
export VERILOG_FILES=rtl/L1MetadataArray.sv
export SDC_FILE_CLOCK_PERIOD=results/asap7/L1MetadataArray/test/clock_period.txt
export ADDITIONAL_LIBS=bazel-out/k8-fastbuild/bin/results/asap7/tag_array_64x184/base/tag_array_64x184.lib

L1MetadataArray_test_floorplan_config.mk

# Stage floorplan config
export CORE_UTILIZATION=3
export RTLMP_FLOW=True
export CORE_MARGIN=2
export SDC_FILE=./constraints-top.sdc
export CORE_MARGIN=4
export PDN_TCL=${PLATFORM_DIR}/openRoad/pdn/BLOCKS_grid_strategy.tcl
export IO_CONSTRAINTS=./io.tcl
export MACROS=tag_array_64x184
export ADDITIONAL_LEFS=bazel-out/k8-fastbuild/bin/results/asap7/tag_array_64x184/base/tag_array_64x184.lef
export ADDITIONAL_LIBS=bazel-out/k8-fastbuild/bin/results/asap7/tag_array_64x184/base/tag_array_64x184.lib

@oharboe
Copy link
Collaborator

oharboe commented Apr 25, 2024

Here is another way to reproduce problems with absolute paths.

OBJECTS_DIR should not contain an absolute path:

oyvind@famine:~/bazel-orfs$ bazel-bin/L1MetadataArray_test_floorplan_make print-OBJECTS_DIR
[deleted]
OBJECTS_DIR = /home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test

Another case where this causes problems, run:

bazel-bin/L1MetadataArray_test_floorplan_make floorplan_issue

There should be no absolute paths here:

$ grep home vars-L1MetadataArray-asap7-test.sh 
export OBJECTS_DIR="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test"
export RTLMP_RPT_DIR="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/rtlmp"
export LOG_DIR="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/logs/asap7/L1MetadataArray/test"
export SDC_FILE_CLOCK_PERIOD="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/results/asap7/L1MetadataArray/test/clock_period.txt"
export RESULTS_DIR="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/results/asap7/L1MetadataArray/test"
export REPORTS_DIR="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/reports/asap7/L1MetadataArray/test"
export DONT_USE_SC_LIB="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/lib/merged.lib"
export SYNTH_STOP_MODULE_SCRIPT="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/mark_hier_stop_modules.tcl"
export DESIGN_DIR="/home/oyvind/.cache/bazel/_bazel_oyvind/6e078b44989a9ce1500a63165a20fd2a/execroot/_main/bazel-out/k8-fastbuild/bin/"
export DONT_USE_LIBS=" /home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/lib/asap7sc7p5t_AO_RVT_FF_nldm_211120.lib  /home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/lib/asap7sc7p5t_INVBUF_RVT_FF_nldm_220122.lib  /home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/lib/asap7sc7p5t_OA_RVT_FF_nldm_211120.lib  /home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/lib/asap7sc7p5t_SIMPLE_RVT_FF_nldm_211120.lib /home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/lib/asap7sc7p5t_SEQ_RVT_FF_nldm_220123.lib /home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/lib/tag_array_64x184.lib"
export GDS_FINAL_FILE="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/results/asap7/L1MetadataArray/test/6_final.gds"
export GDS_MERGED_FILE="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/results/asap7/L1MetadataArray/test/6_1_merged.gds"
export RTLMP_BLOCKAGE_FILE="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test/rtlmp/partition.txt.blockage"
export WORK_HOME_READ="/home/oyvind/bazel-orfs/bazel-out/k8-fastbuild/bin"

Here is an example of how make issue should replace references to the ORFS folder, so there should be no need for absolute paths:

export FLOW_HOME=${FLOW_HOME:-$(pwd)}
export RCX_RULES="${FLOW_HOME}/platforms/asap7/rcx_patterns.rules"

@lpawelcz
Copy link
Contributor Author

Oh, I see that now, thanks for pointing out. We will look into this

@oharboe
Copy link
Collaborator

oharboe commented Apr 25, 2024

Oh, I see that now, thanks for pointing out. We will look into this

This helps, but is not sufficient:

$ git diff
diff --git a/orfs b/orfs
index 258aef8..b76a6b3 100755
--- a/orfs
+++ b/orfs
@@ -16,7 +16,8 @@ else
     export BUILD_DIR=$WORKSPACE
 fi
 
-export WORK_HOME=$BUILD_DIR/$RULEDIR
+PWD_PHYSICAL=$(pwd -P)
+export WORK_HOME=$(echo $BUILD_DIR/$RULEDIR | sed "s|^$PWD_PHYSICAL|.|" | sed 's|^./||')
 
 # Make bazel-bin writable
 chmod -R +w $WORK_HOME

No absolute path:

oyvind@famine:~/bazel-orfs$ bazel-bin/L1MetadataArray_test_floorplan_make print-OBJECTS_DIR
[deleted]
OBJECTS_DIR = bazel-out/k8-fastbuild/bin/objects/asap7/L1MetadataArray/test

Still an absolute path:

oyvind@famine:~/bazel-orfs$ bazel-bin/L1MetadataArray_test_floorplan_make print-DESIGN_CONFIG
[deleted]
DESIGN_CONFIG = /home/oyvind/.cache/bazel/_bazel_oyvind/6e078b44989a9ce1500a63165a20fd2a/execroot/_main/bazel-out/k8-fastbuild/bin/L1MetadataArray_test_config.mk

Signed-off-by: Pawel Czarnecki <[email protected]>
Replace  with  to remove absolute paths
in env vars like DESIGN_CONFIG used in script-based and automated flows.

The effect of modification is that bash runfiles library is no longer needed
so it was removed.

Signed-off-by: Pawel Czarnecki <[email protected]>
@lpawelcz
Copy link
Contributor Author

We reworked the flow. Currently there is only one env var with absolute path which is SYNTH_ARGS that is to be fixed in The-OpenROAD-Project/OpenROAD-flow-scripts#1976.

With the following changes we were able to drop the bash runfiles library dependency and remove the template script used for generating make scripts. The organization of the rules is now a little bit simpler.
It would be good to have this PR merged first to serve as a new baseline for the further rework according to your comments from #27.

@oharboe oharboe merged commit 9e76444 into The-OpenROAD-Project:main Apr 26, 2024
8 checks passed
@lpawelcz lpawelcz deleted the 58412-relative-paths branch May 6, 2024 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants