bugfix: allow unsetting variables in configuration files (#593)

## Misc. Enhancements/Bugfixes * Fixed undefining variables in configuration files being ignored. * Restored `VERILOG_POWER_DEFINE` as an optional variable. ## Steps * `Yosys.JsonHeader`, `Verilator.Lint`, `Odb.WriteVerilogHeader` * Handled undefined `VERILOG_POWER_DEFINE`. --------- Signed-off-by: Kareem Farid <[email protected]> Co-authored-by: Mohamed Gaber <[email protected]>
efabless · Nov 10, 2024 · 771a73e · 771a73e
1 parent ee52af1
commit 771a73e
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 29 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -14,36 +14,55 @@
 ## Documentation
 -->
 
-# 2.2.4
+# 2.2.5
+
+## Steps
+
+* `Yosys.JsonHeader`, `Verilator.Lint`, `Odb.WriteVerilogHeader`
+
+  * Fixed `VERILOG_POWER_DEFINE` not being optional which was an unintentional
+    break from OpenLane 1.
+
+    * Default value is still `USE_POWER_PINS`, but it can be explicitly unset.
 
 ## Misc. Enhancements/Bugfixes
 
+* `openlane.config`: Fixed issue where preprocessor would ignore explicitly-set
+  null values in configuration files.
+
+# 2.2.4
+
+## Tool Updates
+
 * `yosys-sby`: Overlaid new hash for `yosys-0.46` tag because of a tag update
   upstream.
 
 # 2.2.3
 
 ## Misc. Enhancements/Bugfixes
 
-* Fixed incorrect error message when subtituting a step with one that has a nonexistent ID.
+* Fixed incorrect error message when subtituting a step with one that has a
+  nonexistent ID.
 
 # 2.2.2
 
 ## Steps
 
 * `Odb.*`
+
   * Fixed OpenROAD dropping user-set `PYTHONPATH` values.
-  
+
 ## Tool Updates
 
-* Use `NIX_PYTHONPATH` instead of `PYTHONPATH` in Docker and devshells
-  to avoid collisions with user-set `PYTHONPATH` variables.
+* Use `NIX_PYTHONPATH` instead of `PYTHONPATH` in Docker and devshells to avoid
+  collisions with user-set `PYTHONPATH` variables.
 
 # 2.2.1
 
 This patch has no functional changes to OpenLane proper.
 
 ## Tool Updates
+
 * `flake.createOpenLaneShell` now gets OpenLane from `python3.pkgs`.
 * Fixed issue with `flake.createOpenLaneShell` where plugins would not get
   included due to an operator precedence issue.

diff --git a/openlane/config/preprocessor.py b/openlane/config/preprocessor.py
@@ -400,7 +400,7 @@ def process_dict_recursive(
         else:
             processed = value
 
-        if processed is not None:
+        if not key.startswith(PDK_PREFIX) and not key.startswith(SCL_PREFIX):
             ref[key] = processed
             symbols[current_key_path] = processed
 

diff --git a/openlane/scripts/odbpy/power_utils.py b/openlane/scripts/odbpy/power_utils.py
@@ -322,13 +322,13 @@ def set_power_connections(input_json, reader: OdbReader):
 @click.option(
     "--power-define",
     type=str,
-    required=True,
+    required=False,
 )
 @click_odb
 def write_verilog_header(
     output_vh: str,
     input_json: str,
-    power_define: str,
+    power_define: Optional[str],
     reader: OdbReader,
 ):
     input_dict = json.load(open(input_json))
@@ -373,13 +373,14 @@ def write_verilog_header(
         # Write module
         print("// Auto-generated by OpenLane", file=f)
         print(f"module {design_name}(", file=f)
-        print(f"`ifdef {power_define}", file=f)
         last_pos = f.tell()
-        for decl in pg_decls:
-            print(f"  {decl}", file=f, end="")
-            last_pos = f.tell()
-            print(",", file=f)
-        print("`endif", file=f)
+        if power_define is not None:
+            print(f"`ifdef {power_define}", file=f)
+            for decl in pg_decls:
+                print(f"  {decl}", file=f, end="")
+                last_pos = f.tell()
+                print(",", file=f)
+            print("`endif", file=f)
         for decl in signal_decls:
             print(f"  {decl}", file=f, end="")
             last_pos = f.tell()

diff --git a/openlane/scripts/pyosys/json_header.py b/openlane/scripts/pyosys/json_header.py
@@ -35,13 +35,20 @@ def json_header(
     blackbox_models = extra["blackbox_models"]
 
     includes = config["VERILOG_INCLUDE_DIRS"] or []
-    defines = (config["VERILOG_DEFINES"] or []) + [
-        f"PDK_{config['PDK']}",
-        f"SCL_{config['STD_CELL_LIBRARY']}",
-        "__openlane__",
-        "__pnr__",
-        config["VERILOG_POWER_DEFINE"],
-    ]
+    defines = (
+        (config["VERILOG_DEFINES"] or [])
+        + [
+            f"PDK_{config['PDK']}",
+            f"SCL_{config['STD_CELL_LIBRARY']}",
+            "__openlane__",
+            "__pnr__",
+        ]
+        + (
+            []
+            if config.get("VERILOG_POWER_DEFINE") is None
+            else [config.get("VERILOG_POWER_DEFINE")]
+        )
+    )
 
     d = ys.Design()
     d.add_blackbox_models(

diff --git a/openlane/steps/odb.py b/openlane/steps/odb.py
@@ -309,7 +309,7 @@ class WriteVerilogHeader(OdbpyStep):
     config_vars = OdbpyStep.config_vars + [
         Variable(
             "VERILOG_POWER_DEFINE",
-            str,
+            Optional[str],
             "Specifies the name of the define used to guard power and ground connections in the output Verilog header.",
             deprecated_names=["SYNTH_USE_PG_PINS_DEFINES", "SYNTH_POWER_DEFINE"],
             default="USE_POWER_PINS",
@@ -324,14 +324,20 @@ def get_subcommand(self) -> List[str]:
 
     def get_command(self) -> List[str]:
         state_in = self.state_in.result()
-        return super().get_command() + [
+        command = super().get_command() + [
             "--output-vh",
             os.path.join(self.step_dir, f"{self.config['DESIGN_NAME']}.vh"),
             "--input-json",
             str(state_in[DesignFormat.JSON_HEADER]),
-            "--power-define",
-            self.config["VERILOG_POWER_DEFINE"],
         ]
+        if self.config.get("VERILOG_POWER_DEFINE") is not None:
+            command += ["--power-define", self.config["VERILOG_POWER_DEFINE"]]
+        else:
+            self.warn(
+                "VERILOG_POWER_DEFINE undefined. Verilog Header will not include power ports."
+            )
+
+        return command
 
     def run(self, state_in, **kwargs) -> Tuple[ViewsUpdate, MetricsUpdate]:
         views_updates, metrics_updates = super().run(state_in, **kwargs)

diff --git a/openlane/steps/pyosys.py b/openlane/steps/pyosys.py
@@ -42,7 +42,7 @@
     ),
     Variable(
         "VERILOG_POWER_DEFINE",
-        str,
+        Optional[str],
         "Specifies the name of the define used to guard power and ground connections in the input RTL.",
         deprecated_names=["SYNTH_USE_PG_PINS_DEFINES", "SYNTH_POWER_DEFINE"],
         default="USE_POWER_PINS",
@@ -142,7 +142,7 @@ def _parse_yosys_check(
     ),
     Variable(
         "VERILOG_POWER_DEFINE",
-        str,
+        Optional[str],
         "Specifies the name of the define used to guard power and ground connections in the input RTL.",
         deprecated_names=["SYNTH_USE_PG_PINS_DEFINES", "SYNTH_POWER_DEFINE"],
         default="USE_POWER_PINS",

diff --git a/openlane/steps/verilator.py b/openlane/steps/verilator.py
@@ -44,7 +44,7 @@ class Lint(Step):
         ),
         Variable(
             "VERILOG_POWER_DEFINE",
-            str,
+            Optional[str],
             "Specifies the name of the define used to guard power and ground connections in the input RTL.",
             deprecated_names=["SYNTH_USE_PG_PINS_DEFINES", "SYNTH_POWER_DEFINE"],
             default="USE_POWER_PINS",
@@ -124,12 +124,14 @@ def run(self, state_in: State, **kwargs) -> Tuple[ViewsUpdate, MetricsUpdate]:
                     model_set.add(str_model)
                     model_list.append(str_model)
         defines = [
-            self.config["VERILOG_POWER_DEFINE"],
             f"PDK_{self.config['PDK']}",
             f"SCL_{self.config['STD_CELL_LIBRARY']}",
             "__openlane__",
             "__pnr__",
         ]
+        if verilog_power_define := self.config.get("VERILOG_POWER_DEFINE"):
+            defines += [verilog_power_define]
+
         defines += self.config["LINTER_DEFINES"] or self.config["VERILOG_DEFINES"] or []
 
         if len(model_list):