add Select combinators

[albertchen-sifive]

This is a sketch of a Select library for diplomatic nodes and LazyModules. The motivation behind this is to enable fine-grained selection of node bundles for things like monitoring, without having to override the NodeImp.monitor method or change a LazyModule's Parameters or modify any design code.

Related issue:

Type of change: feature request

Impact: API addition (no impact on existing code)

Development Phase: proposal

Release Notes

[jackkoenig]

I see a lot of wildcard type parameters here. Is it possible someone may someday want to care about the first two type parameters of node? If so, we should probably go ahead and add them because adding them later will be very painful for anyone using this type.

That being said I don't know much about best practices and maintaining backwards compatibility in using type parameters.

[hcook]

In this case, those parameters are kind of implicitly captured by the EdgeInParams type member, which inevitably will have members of the "missing" DI and UI types. So I guess I don't see much immediate practical value in offering those through this API. I do wonder whether since these classes are part of the public collect interface, should they become part of top-level diplomacy API instead of embedded in this object Select?

[hcook]

without having to override the NodeImp.monitor method or change a LazyModule's Parameters or modify any design code.

This claim is appealing and interesting. Ideally we would actually retire the baked-in monitor injection infrastructure and use the new methodology, rather than having two ways of doing the same thing.

I wonder about applying the technique in practice. Specifically, can you demonstrate how one would replace the extant examples of DisableMonitors in the code base with Select, while still achieving the same outcome? (i.e. monitors attached to certain edges but not others). Honestly we should also probably re-evaluate which DisableMonitors are actually necessary after Chisel 3.4, but for the sake of this exercise, take it as a given that the existing distribution of monitors needs to be preserved.

What I am getting at is, while having to "modify any design code" to provide the DisableMonitors directive is a conflation of interests, it is actually quite expedient in terms of being like "turn it off for this subgraph" and I am curious what the equivalent Select/collect code would look like in practice.

[azidar]

LGTM, except for a few minor changes.

[azidar]

Suggested change

	`Select.collectInwardEdges` takes a `BaseNode` and a parital function. It
	`Select.collectInwardEdges` takes a `BaseNode` and a partial function. It

[azidar]

I think this is an unused import.

[albertchen-sifive]

What I am getting at is, while having to "modify any design code" to provide the DisableMonitors directive is a conflation of interests, it is actually quite expedient in terms of being like "turn it off for this subgraph" and I am curious what the equivalent Select/collect code would look like in practice.

I added some mdoc examples https://github.com/chipsalliance/rocket-chip/blob/select-helpers/docs/src/diplomacy/select_tutorial.md. do you think the examples cover this use case or should there be more examples and/or select methods?

[hcook]

@albertchen-sifive Nice! I love these examples, they are very cool.

I don't think they quite hit the (admittedly-high) bar I am setting, which requires targeting specific pieces of a diplomatic graph at an even finer granularity. See this function for an example. We only disable monitoring for the connections coming into the tile that treat tile devices as slaves; we don't deactivate monitoring for the connections going out of the tile where the tile is the master device; these pieces of the graph are both put inside the same module hierarchy. This different treatment is because the slaves might have unique names and address ranges, checking of which by a Monitor breaks tile deduplication. I will freely admit that this is a bit of an esoteric use case that we might be able to avoid entirely with better test code extraction. And maybe a solution like a (later-inlined) LazyModule wrapper scope that can serve as the target of your Select would be an appropriate solution? @azidar @jackkoenig Any thoughts?

Another example I would be interested in seeing is the equivalent of this Config alteration, allowing the monitor injection to be toggled off through the Config-layer interface. What would such a thing look like if we were deciding whether or not to deploy an application of one of your Selects to a rocket-chip design? Would it be applied in the TestHarness class? The Generator App?

You don't have to answer either of these questions now in order to merge in this PR, but I'd consider them the next phase of development in terms of deploying these capabilities in the wild.

[albertchen-sifive]

@hcook thanks for the examples. I'll leave those for another PR.