Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HW] Add hw.path.to_here op #6261

Closed
wants to merge 3 commits into from
Closed

[HW] Add hw.path.to_here op #6261

wants to merge 3 commits into from

Conversation

mortbopet
Copy link
Contributor

The hw.path.to_here operation represents a path through the instance hierarchy which starts from the top-level module and ends at the current instance wherein the hw.path.to_here is located. The operation can be lowered to hw.hierpath and is useful in cases where we want to track the absolute path of an instance in the design, but not yet want to use hw.hierpath operations due to the extra overhead involved in maintaining said op if the instance hierarchy is modified.

The operation (and pass) should only be used in cases where modules are expected to be unique within the design, i.e. only a single instance exists for any given module in between the module instance and the top level module (note: this does not imply that the entire module hierarchy must be elaborated); thus the user must either ensure this by design, or elaborate parts of the design accordingly.

The `hw.path.to_here` operation represents a path through the instance hierarchy which starts from the top-level module and ends at the current instance wherein the `hw.path.to_here` is located. The operation can be lowered to `hw.hierpath` and is useful in cases where we want to track the absolute path of an instance in the design, but not yet want to use `hw.hierpath` operations due to the extra overhead involved in maintaining said op if the instance hierarchy is modified.

The operation (and pass) should only be used in cases where modules are expected to be unique within the design, i.e. only a single instance exists for any given module in between the module instance and the top level module (note: this does not imply that the entire module hierarchy must be elaborated); thus the user must either ensure this by design, or elaborate parts of the design accordingly.
@seldridge
Copy link
Member

seldridge commented Oct 6, 2023

The operation (and pass) should only be used in cases where modules are expected to be unique within the design, i.e. only a single instance exists for any given module in between the module instance and the top level module (note: this does not imply that the entire module hierarchy must be elaborated); thus the user must either ensure this by design, or elaborate parts of the design accordingly.

This is a pretty huge restriction! It's also one that I don't think should be enforced. Put differently, if this is enforced how would you enforce it? Ideally it should be enforced with verifiers, however, this can create a "modal" verification problem where verification succeeds if you run an elaboration pre-pass.

A related issue is that this locks you into what is the "top". Most designs have at least two "top"s that matter: (1) the design and (2) the test bench. Even the DUT/Testharness model is still too trivial and most designs have many public modules which may have their own testharness (the design is a graph not a tree) and the public modules may instantiate no common modules (the design is a disconnected graph). (See discussions around changing FIRRTL to support multiple disconnected graphs: chipsalliance/firrtl-spec#130.)

Two ideas:

  1. Hierarchical paths are tedious to maintain across hierarchy manipulations. This is fundamental to maintaining path information. However, it can be lessened with utilities for updating hierarchical paths. Some of this already exists, e.g., HierPathOp::{inlineModule, updateModule, truncateAtModule, /* etc. */}. Are there particular pain points that you have or that you are hitting?
  2. Hierarchical paths being represented as an array of inner symbols is possibly not its final form... There is an alternative representation that uses SSA values passed through the hierarchy to do the same thing. I'm reluctant to go rewrite hierarchical paths, yet again, though. 😅

Copy link
Contributor

@darthscsi darthscsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would strongly recommend against going this route. Hierpath is annoying to maintain, but it's a single canonical representation which captures a lot of the cases not handled by this approach. Having multiple representations is generally bad as it adds complexity. Having restrictions in HW such as an op assumes certain circuit or instantiation (uniqueness) properties should be a non-starter.

Path representations are routed through hierpath precisely so that there is a single representation (with all instances at a known point in the ir hierarchy) which can be operated on with convenient functions. The right thing to do is to flush out those functions rather than making a new mechanism.

@mortbopet mortbopet marked this pull request as draft October 9, 2023 08:56
@mortbopet
Copy link
Contributor Author

Thank you @seldridge and @darthscsi for both of the comments! (marking this as draft because we're still thinking about the best way to go about this problem).
The update functions that @seldridge mentions would indeed lessen the burden, but wouldn't remove it entirely - in reality, i'd like to completely avoid having to reason about the top-level at all.
To give some context, this is for PD work, where we want to attach constraints to things that happen inside a module, regardless of where that module gets instantiated.

Those PD constraints needs to be specified with (ideally) perfect accuracy, and not using some wildcard flag that is good enough to match whatever we need to match. The thinking that introduced this hw.path.to_here op was to have a perfect, absolute path. The opposite way to attach those constraints is to provide a parent module to which the constraints should apply, e.g. as in #6262:

proc reg_0_multicycle_config { parent } {
  // parent|mod1|... represents an instance hierarchy that has evolved over the set of passes that was applied
  set_multicycle_path -hold 1 -setup 2 -from [get_registers {$parent|reg_0}] -to [get_registers {$parent|mod1|...|reg2}]
}

I'm thinking that a better, alternate, approach to this would be to introduce a construct/pass (most likely in the MSFT dialect since that's the de-facto home for our PD constructs) that can create a TCL configuration tree at a late stage, which would effectively do the same thing. S.t. the user only provides that config procedure the top level, and then things flow from there:

proc reg_0_multicycle_config { parent } {
  set_multicycle_path -hold 1 -setup 2 -from [get_registers {$parent|reg_0}] -to [get_registers {$parent|mod1|...|reg2}]
}

proc ..._config { parent } {
  reg_0_multicycle_config($parent|...)
}

proc mod1_config { parent } {
  ..._config($parent|mod1)
}

proc top_config { parent } {
  mod1_config($parent)
}

@teqdruid thoughts?

@mortbopet mortbopet closed this Oct 11, 2023
@darthscsi
Copy link
Contributor

It sounds like the problem is you are trying to put the constraints outside the module, but talk about the module? Is there a way to keep the specification of the PD constraints inside the module it applies to?

@mortbopet
Copy link
Contributor Author

@darthscsi that is the approach taken in #6277.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants