[major] Layer-associated probe semantics and ABI #160
Conversation
seldridge
changed the title
seldridge
force-pushed
the
dev/seldridge/layer-associated-probe-updates
branch
from
e0e7480
to
3e35a6f
Compare
seldridge
force-pushed
the
dev/seldridge/layer-associated-probe-updates
branch
from
3e35a6f
to
75d192a
Compare
seldridge
force-pushed
the
dev/seldridge/layer-associated-probe-updates
branch
4 times, most recently
from
4a56b2f
to
d3dafd4
Compare
spec.md
Outdated
| Probe types may be colored with a layer to condition their existence on that layer being enabled. | ||
| For more information on layer-colored probes see [@sec:probe-types] and [@sec:layer-coloring]. |
There was a problem hiding this comment.
The phrase "to condition their existence" sounds a bit existential. The very fabric of reality urges a slight rewording :)
Perhaps: "Optionally, probe types may be colored with a layer. Layer coloring will indicate which layers must be enabled for a probe to be used."
Alternatively, if we want to resort to stronger language, we can do that too, but we would want to establish a deeper formalism for what that means. That is, we would need a technical term to describe when a probe is "not active" (or "disabled" or "erased" or whatever), and then we can cross-reference that term in the ABI to make that precise in terms of Verilog: that the probe does not exist when it's "not active".
There was a problem hiding this comment.
It's only problematic if the probes question their existence...
I switched to use a variant of your terse language in a fixup commit: 774cc11
I like the simple "enabled"/"disabled" language. I'd almost prefer something even shorter ("on"/"off"), but we can bikeshed this in a cleanup over the language in the spec.
seldridge
force-pushed
the
dev/seldridge/layer-associated-probe-updates
branch
from
d3dafd4
to
b43c9ef
Compare
| While `RWProbe<T>`{.firrtl} is strictly more powerful, `Probe<T>`{.firrtl} allows for more aggressive optimization. | ||
| You should consider using `Probe`{.firrtl} whenever you know you don't need to ability to force (see [@sec:forcing]). | ||
| An operation may only "read" from a probe whose layer color is enabled when the operation is enabled. | ||
| An operation that "writes" to a probe must be in a region whose layer color is enabled when the probe's layer color is enabled. |
There was a problem hiding this comment.
These are the two most important sentences in this PR.
Give these a lot of scrutiny @mmaloney-sf.
There was a problem hiding this comment.
Is there a meaningful difference between a layer and a layer color?
My impression is the term "color" is just a colorful way to speak of the association between a probe and the layer it works against.
Both of these sentences also confuse me a bit, trying to interpret them. Trying to dig into why:
- What does it mean for "the operation [to be] enabled"?
- The quotes around "read" and "write" make me wonder if these are formal statements or if they should be taken intuitively.
- You use the word "region" -- Should this be "layer" instead? Eg, "must be in a layer whose color is enabled".
- The phrase "whose color is enabled" is also weird. Intuitively, colors can be mixed, but not enabled or disabled.
- I feel like there's some subtle compound condition in the "write" restriction.
|
Synced with @mmaloney-sf offline. This is good enough to land. @mmaloney-sf will take over driving this with subsequent PRs by providing more formal semantics around describing the layers as a lattice (with the "colorless" root layer, i.e., the design, defined). |
Add language that describes the semantics of layer-colored probes. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Add the ability for a module to be declared with layers enabled. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Clarify and update semantics for the legality of operations which use layer-colored probes. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
seldridge
force-pushed
the
dev/seldridge/layer-associated-probe-updates
branch
from
b43c9ef
to
673aa08
Compare
Define semantics and an ABI for layer-associated probes.
This introduces a major change to the existing layer ABI where layers are now enabled with a Verilog text macro (`define) and not by including a file. This is done to unify the enabling of all layer collateral which can include modules/logic and new probe ports (which were not supported previously).This uses a basic ABI where all layer-colored probes are always emitted. We may want to change this in the future to use an ABI where Verilog macros are conditionally defined. However, it is much simpler to start without this. It is also not clear that such an ABI provides much over just requiring a user to enable a layer for the reference to be valid.