From 9de4e9be23fe6f77cb11b06ec39e1d8a9c6c1d85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98yvind=20Harboe?= Date: Thu, 23 Apr 2026 21:29:00 +0200 Subject: [PATCH] feat: orfs_flow user_arguments for project-specific env vars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a user_arguments dict to orfs_flow() that skips the variables.yaml spell-check and flows through every stage. Rejects keys that shadow a known ORFS variable so those still route through arguments=. Lets downstream projects pass vars consumed only by their own .tcl/.mk (e.g. ARRAY_COLS in mock-array) without patching variables.yaml. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Øyvind Harboe --- private/flow.bzl | 24 +++++++++++++++++-- test/BUILD | 24 +++++++++++++++++++ test/spelling_error/test_spelling_errors.sh | 9 +++++++ .../user_args_multiple_shadow/BUILD | 13 ++++++++++ test/spelling_error/user_args_pass/BUILD | 16 +++++++++++++ test/spelling_error/user_args_shadow/BUILD | 12 ++++++++++ 6 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 test/spelling_error/user_args_multiple_shadow/BUILD create mode 100644 test/spelling_error/user_args_pass/BUILD create mode 100644 test/spelling_error/user_args_shadow/BUILD diff --git a/private/flow.bzl b/private/flow.bzl index 49d0a0f2..683cd84f 100644 --- a/private/flow.bzl +++ b/private/flow.bzl @@ -18,7 +18,14 @@ load( "orfs_squashed", "orfs_synth_rule", ) -load("//private:stages.bzl", "STAGE_METADATA", "check_variables", "get_sources", "get_stage_args") +load( + "//private:stages.bzl", + "ALL_VARIABLE_TO_STAGES", + "STAGE_METADATA", + "check_variables", + "get_sources", + "get_stage_args", +) # Stages with an ODB that open.tcl can load for web_save_report. _HTML_STAGES = ["floorplan", "place", "cts", "grt", "route", "final"] @@ -167,6 +174,7 @@ def orfs_flow( stage_arguments = {}, renamed_inputs = {}, arguments = {}, + user_arguments = {}, extra_arguments = {}, extra_configs = {}, abstract_stage = None, @@ -198,6 +206,10 @@ def orfs_flow( Use stage_arguments only to override the automatic stage assignment. renamed_inputs: dictionary keyed by ORFS stages to rename inputs arguments: dictionary of additional arguments to the flow, automatically assigned to stages + user_arguments: dictionary of project-specific env vars to expose to every stage without + validating against ORFS variables.yaml. Use for vars read only by user-supplied .tcl/.mk + (e.g. ARRAY_COLS in a project's MACRO_PLACEMENT_TCL). Keys that collide with known ORFS + variables are rejected — route those through 'arguments' instead. extra_arguments: dictionary keyed by ORFS stages with lists of .json argument file labels. These .json files are merged into the stage config, providing computed arguments that flow through OrfsInfo to subsequent stages. @@ -229,6 +241,14 @@ def orfs_flow( """ check_variables(arguments.keys(), "arguments") check_variables(sources.keys(), "sources") + shadowed = sorted([k for k in user_arguments if k in ALL_VARIABLE_TO_STAGES]) + if shadowed: + fail( + "user_arguments contains known ORFS variable(s): {shadowed}. ".format( + shadowed = ", ".join(shadowed), + ) + + "Use arguments= for ORFS variables; reserve user_arguments= for project-specific env vars.", + ) if abstract_stage and last_stage: fail("abstract_stage and last_stage are mutually exclusive") if variant == "base": @@ -245,7 +265,7 @@ def orfs_flow( stage_sources = stage_sources, stage_arguments = stage_arguments, renamed_inputs = renamed_inputs, - arguments = arguments, + arguments = arguments | user_arguments, extra_arguments = extra_arguments, extra_configs = extra_configs, abstract_stage = abstract_stage, diff --git a/test/BUILD b/test/BUILD index 76374c13..90b9cf2e 100644 --- a/test/BUILD +++ b/test/BUILD @@ -306,6 +306,30 @@ orfs_flow( verilog_files = LB_VERILOG_FILES, ) +# Exercises user_arguments: project-specific env vars that intentionally +# bypass the variables.yaml spell-check. Loading this target proves the +# feature works; `bazelisk test //...` picks it up via build_test below. +# buildifier: disable=duplicated-name +orfs_flow( + name = "lb_32x128", + arguments = LB_ARGS, + last_stage = "floorplan", + openroad = "//mock/openroad/src/bin:openroad", + previous_stage = {"floorplan": ":lb_32x128_synth"}, + stage_sources = LB_STAGE_SOURCES, + user_arguments = { + "ARRAY_COLS": "4", + "USER_PROJECT_KNOB": "42", + }, + variant = "user_args", + verilog_files = LB_VERILOG_FILES, +) + +build_test( + name = "lb_32x128_user_args_build_test", + targets = [":lb_32x128_user_args_floorplan"], +) + # Pin placement generation: shares synth, runs own floorplan without pinned IO. # buildifier: disable=duplicated-name orfs_flow( diff --git a/test/spelling_error/test_spelling_errors.sh b/test/spelling_error/test_spelling_errors.sh index fb0a9339..ca49e8c6 100755 --- a/test/spelling_error/test_spelling_errors.sh +++ b/test/spelling_error/test_spelling_errors.sh @@ -61,6 +61,15 @@ expect_failure multiple_typos "CORE_UTILIZATON, PLACE_DENSTY" # Control: valid variables load fine expect_success valid +# user_arguments bypasses the spell-check for project-specific env vars +expect_success user_args_pass + +# user_arguments refuses to shadow a known ORFS variable +expect_failure user_args_shadow "user_arguments contains known ORFS variable(s): CORE_UTILIZATION" + +# Multiple shadowed vars are listed sorted in the error +expect_failure user_args_multiple_shadow "CORE_UTILIZATION, PLACE_DENSITY" + echo echo "=== Results: ${pass} passed, ${fail} failed ===" [ "$fail" -eq 0 ] diff --git a/test/spelling_error/user_args_multiple_shadow/BUILD b/test/spelling_error/user_args_multiple_shadow/BUILD new file mode 100644 index 00000000..df17922e --- /dev/null +++ b/test/spelling_error/user_args_multiple_shadow/BUILD @@ -0,0 +1,13 @@ +# Multiple known ORFS variables placed in user_arguments. +# Expected: both are listed, sorted, in the shadow error. + +load("@bazel-orfs//:openroad.bzl", "orfs_flow") + +orfs_flow( + name = "user_args_multiple_shadow", + user_arguments = { + "PLACE_DENSITY": "0.5", + "CORE_UTILIZATION": "20", + }, + verilog_files = ["//:rtl/dummy.v"], +) diff --git a/test/spelling_error/user_args_pass/BUILD b/test/spelling_error/user_args_pass/BUILD new file mode 100644 index 00000000..358c0932 --- /dev/null +++ b/test/spelling_error/user_args_pass/BUILD @@ -0,0 +1,16 @@ +# user_arguments bypasses variables.yaml spell-check. +# Expected: package loads without errors. + +load("@bazel-orfs//:openroad.bzl", "orfs_flow") + +orfs_flow( + name = "user_args_pass", + arguments = { + "CORE_UTILIZATION": "20", + }, + user_arguments = { + "MY_PROJECT_KNOB": "42", + "ARRAY_COLS": "4", + }, + verilog_files = ["//:rtl/dummy.v"], +) diff --git a/test/spelling_error/user_args_shadow/BUILD b/test/spelling_error/user_args_shadow/BUILD new file mode 100644 index 00000000..c24e62a7 --- /dev/null +++ b/test/spelling_error/user_args_shadow/BUILD @@ -0,0 +1,12 @@ +# user_arguments must not shadow a known ORFS variable. +# Expected error: user_arguments contains known ORFS variable(s): CORE_UTILIZATION + +load("@bazel-orfs//:openroad.bzl", "orfs_flow") + +orfs_flow( + name = "user_args_shadow", + user_arguments = { + "CORE_UTILIZATION": "20", + }, + verilog_files = ["//:rtl/dummy.v"], +)