Consistent treatment of flag variables

[povik]

Description

There are a number of variables to turn things on/off, as documented on the "flow variables" page: https://openroad-flow-scripts.readthedocs.io/en/latest/user/FlowVariables.html

As far as I can tell those variables don't have consistent interpretation, for some variables they must be set to 1 for the documented effect (e.g. SKIP_CTS_REPAIR_TIMING), for other variables the existence of the variable is all that matters, the value is ignored (e.g. EQUIVALENCE_CHECK).

Suggested Solution

The obvious suggestion is to have a helper to check flags which would be used throughout the Tcl code; I have no idea if there's a fitting place to define one.

Additional Context

No response

[maliberty]

I agree things have gotten somewhat inconsistent.

I believe all steps start with load.tcl so that could be a place to put the utility (I see append_env_var is there which seems like a utility). You could also create a utility.tcl and source it from load.tcl to make a cleaner separation.

Are you interested in resolving this?

[povik]

Are you interested in resolving this?

Yes! If I get to it I will make the change.

[maliberty]

@povik are you working on this? @socclosure will take it up if you are not.

[povik]

@maliberty Not really, no

[socclosure]

Thanks Folks.I will look into this issue then.Regards Jignesh On Sep 17, 2024, at 10:05 AM, Martin Povišer ***@***.***> wrote: @maliberty Not really, no —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[socclosure]

@maliberty @povik
I am proposing to add a small makefile file target to print value and name of all user's configurable variables which can be used in all subsequent TCL code of any other targets.

For reference check "print_all_env_cfg" in Makefile in the "ISSUE_1935" branch of my local git Repo at
https://github.com/socclosure/OpenROAD-flow-scripts/blob/ISSUE_1935/flow/Makefile

Let me know your feedback,

[socclosure]

@maliberty @povik Any feedback on my proposal on code implementation of this task?

[maliberty]

There is a lot of progress on this recently in

I'm not sure how printing the variables ensures consistent treatment of them.

[socclosure]

As per my understanding the user wants to know the value & existence of each configuration variable which can be set in multiple files. So, I thought printing the name and value of each configuration variable can provide the information that the user is looking for. But it seems I might be missing something. Can you please elaborate on the requirements of this issue?

…

-REgards Jignesh Can you elaborate

On Mon, Oct 7, 2024 at 4:12 PM Matt Liberty ***@***.***> wrote: There is a lot of progress on this recently in image.png (view on web) <https://github.com/user-attachments/assets/c19d8ea8-a38b-4d1c-9e29-555ed1ef25e3> I'm not sure how printing the variables ensures consistent treatment of them. — Reply to this email directly, view it on GitHub <#1935 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BJW5OQE3ZMG6XHCUOBVNOFLZ2MIOZAVCNFSM6AAAAABGC63IWCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJYGEYDQOJSG4> . You are receiving this because you were assigned.Message ID: ***@***.*** com>

[socclosure]

@maliberty @povik Any feedback on my question or proposal?

[maliberty]

I don't see the relationship between your suggestion and the original request. The request is about the consistency about how a variable is set, not about printing anything out.

In the meantime @oharboe has made a lot of progress on this in a series of PRs. I'm not sure if there is much left to do here but he can comment.

[oharboe]

I would say this is solved, the first sweep is done.

Next up it would be valuable to have well thought out and tested PRs to address specific inconsistencies, one per PR.

For instance, I think it would be good to have a default value instead if negative names. SKIP_... values should be negated and a default of 1 should be added to variables.yaml. This is at the nitpicking level though.

[socclosure]

Thanks @maliberty and @oharboe for your feedback and closing this issue.