updated cluster_flops enablement check#4143
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request redefines the CLUSTER_FLOPS variable to enable multi-bit flip-flop clustering and introduces CLUSTER_FLOPS_ARGS for passing additional arguments. The changes affect the documentation, variables schema, and global placement script. Feedback recommends avoiding manual edits to the auto-generated FlowVariables.md file and suggests using a more consistent boolean check style in the Tcl script.
| | <a name="CLUSTER_FLOPS"></a>CLUSTER_FLOPS| Enable clustering of flip-flops into multi-bit flip-flops, if the platform PDK includes multi-bit flip-flops| 0| | ||
| | <a name="CLUSTER_FLOPS_ARGS"></a>CLUSTER_FLOPS_ARGS| Additional arguments passed to the cluster_flops command.| | |
There was a problem hiding this comment.
This file is automatically generated from flow/scripts/variables.yaml (as noted on line 87). Manual edits to these tables and lists are likely to be overwritten. It is recommended to update the documentation by running the generation script after modifying variables.yaml to ensure consistency and avoid maintenance issues.
flow/scripts/global_place.tcl
Outdated
|
|
||
| if { [env_var_exists_and_non_empty CLUSTER_FLOPS] } { | ||
| cluster_flops {*}[env_var_or_empty CLUSTER_FLOPS_ARGS] | ||
| if { [env_var_equals CLUSTER_FLOPS 1] } { |
There was a problem hiding this comment.
Using env_var_equals CLUSTER_FLOPS 1 is inconsistent with the boolean check style used elsewhere in this script (e.g., lines 9, 18, and 30), which use direct truthiness checks. For consistency and to allow for standard Tcl boolean values (like true or on), consider using a direct check, as the variable is guaranteed to have a default value from variables.yaml.
if { $::env(CLUSTER_FLOPS) } {
There was a problem hiding this comment.
I guess 9b589d2 was a mistake and should just be reverted back to this form.
There was a problem hiding this comment.
I think initially I was trying to avoid CLUSTER_FLOPS_ARGS and have CLUSTER_FLOPS do both but it didn't work out.
reverted CLUSTER_FLOPS check Signed-off-by: Jeff Ng <jeffng@precisioninno.com>
openroad-ci
force-pushed
the
update-cluster-flops-check
branch
from
040c328 to
567f1df
Compare
3 participants
Previous check was enabling cluster_flops if the CLUSTER_FLOPS flow variable was 0 (was checking existence and not "").
Updated docs to include basic CLUSTER_FLOPS_ARGS description.