Add foundational manual placement utilities (place_row, place_column) by Divinesoumyadip · Pull Request #9764 · The-OpenROAD-Project/OpenROAD

Divinesoumyadip · 2026-03-14T15:11:29Z

Resolves #4041.
This PR introduces foundational TCL utility scripts to enable quick manual placement of instances in structured rows or columns. This is essential for building structured datapath blocks, register files, and analog-like layouts where automated placement might not meet specific density or symmetry requirements.

Places a list of instances in a vertical stack. place_row: Places a list of instances in a horizontal row.Automated snapping: Uses OpenDB (odb) to calculate cell dimensions and ensure legalized placement.Configurable spacing and placement status (FIRM, PLACED, LOCKED).

gemini-code-assist

Code Review

This pull request introduces new TCL utility scripts for manual placement (place_row, place_column) and refactors parts of the C++ codebase for clarity. The C++ changes replace the generic commitTree method with more descriptive addTreeUsage and removeTreeUsage methods, improving the GridGraph API. The new TCL scripts are functional but have some areas for improvement. I've identified a bug in the argument parsing logic that can lead to a crash, and also pointed out significant code duplication that affects maintainability. Additionally, a minor style issue in a CMake file was noted.

gemini-code-assist · 2026-03-14T15:13:15Z

src/utl/scripts/manual_place.tcl

+  proc parse_args {args_list keys} {
+    array set parsed {}
+    set len [llength $args_list]
+    for {set i 0} {$i < $len} {incr i} {
+      set arg [lindex $args_list $i]
+      if {[string match "-*" $arg]} {
+        set key [string range $arg 1 end]
+        if {[lsearch $keys $key] != -1} {
+          set parsed($key) [lindex $args_list [incr i]]
+        } else {
+          utl::error UTL 4041 "Unknown argument: $arg"
+        }
+      }
+    }
+    return [array get parsed]
+  }


The parse_args procedure has a bug. If an argument key (like -spacing) is provided as the last argument without a corresponding value, the script will fail with an invalid expression error. This is because lindex on an out-of-bounds index returns an empty string, which is then passed to expr. The argument parser should validate that a value exists for each key.

I suggest rewriting the loop to handle this case correctly. I've also changed lsearch to lsearch -exact to prevent partial matches on key names.

proc parse_args {args_list keys} { array set parsed {} set len [llength $args_list] for {set i 0} {$i < $len} {incr i} { set arg [lindex $args_list $i] if {[string match "-*" $arg]} { set key [string range $arg 1 end] if {[lsearch -exact $keys $key] != -1} { if {$i + 1 >= $len} { utl::error UTL 4041 "Missing value for argument: $arg" } set parsed($key) [lindex $args_list [incr i]] } else { utl::error UTL 4041 "Unknown argument: $arg" } } } return [array get parsed] }

gemini-code-assist · 2026-03-14T15:13:15Z

src/utl/CMakeLists.txt

         NAMESPACE utl
         I_FILE    src/Logger.i
         SCRIPTS   src/Utl.tcl
+                   scripts/manual_place.tcl  # <--- YOUR NEW SCRIPT ADDED HERE


The comment # <--- YOUR NEW SCRIPT ADDED HERE is informal and not suitable for a production codebase. It should be removed to maintain a professional coding style.

scripts/manual_place.tcl

gemini-code-assist · 2026-03-14T15:13:15Z

src/utl/scripts/manual_place.tcl

+  proc place_column { args } {
+    array set opts [parse_args $args {instances start_x start_y spacing status}]
+
+    if {![info exists opts(instances)] || ![info exists opts(start_x)] || ![info exists opts(start_y)]} {
+      utl::error UTL 4042 "Usage: place_column -instances <list> -start_x <x> -start_y <y> \[-spacing <s>\] \[-status <PLACED|FIRM|LOCKED>\]"
+    }
+
+    set block [ord::get_db_block]
+    set current_y $opts(start_y)
+    set x $opts(start_x)
+    set spacing [expr {[info exists opts(spacing)] ? $opts(spacing) : 0}]
+    set status [expr {[info exists opts(status)] ? $opts(status) : "FIRM"}]
+
+    foreach inst_name $opts(instances) {
+      set inst [$block findInst $inst_name]
+      if {$inst == "NULL"} {
+        utl::warn UTL 4043 "Instance $inst_name not found. Skipping."
+        continue
+      }
+
+      # Set location and status
+      $inst setLocation $x $current_y
+      $inst setPlacementStatus $status
+
+      # Get height of the cell to calculate next position
+      set master [$inst getMaster]
+      set height [$master getHeight]
+
+      set current_y [expr {$current_y + $height + $spacing}]
+    }
+    utl::info UTL 4044 "Successfully placed [llength $opts(instances)] instances in a column."
+  }
+
+  proc place_row { args } {
+    array set opts [parse_args $args {instances start_x start_y spacing status}]
+
+    if {![info exists opts(instances)] || ![info exists opts(start_x)] || ![info exists opts(start_y)]} {
+      utl::error UTL 4045 "Usage: place_row -instances <list> -start_x <x> -start_y <y> \[-spacing <s>\] \[-status <PLACED|FIRM|LOCKED>\]"
+    }
+
+    set block [ord::get_db_block]
+    set current_x $opts(start_x)
+    set y $opts(start_y)
+    set spacing [expr {[info exists opts(spacing)] ? $opts(spacing) : 0}]
+    set status [expr {[info exists opts(status)] ? $opts(status) : "FIRM"}]
+
+    foreach inst_name $opts(instances) {
+      set inst [$block findInst $inst_name]
+      if {$inst == "NULL"} {
+        utl::warn UTL 4046 "Instance $inst_name not found. Skipping."
+        continue
+      }
+
+      # Set location and status
+      $inst setLocation $current_x $y
+      $inst setPlacementStatus $status
+
+      # Get width of the cell to calculate next position
+      set master [$inst getMaster]
+      set width [$master getWidth]
+
+      set current_x [expr {$current_x + $width + $spacing}]
+    }
+    utl::info UTL 4047 "Successfully placed [llength $opts(instances)] instances in a row."
+  }


The procedures place_column and place_row share a lot of common logic, such as argument parsing, checking for required arguments, setting defaults, and iterating through instances. This duplication can make future maintenance harder.

Consider refactoring the common logic into a private helper procedure, for example _place_line, which would take an orientation argument ("row" or "column"). The public procedures place_row and place_column would then become simple wrappers around this helper. This would centralize the placement logic and improve maintainability.

github-actions · 2026-03-14T15:14:35Z