Add DynamicGroup for runtime-defined option groups#5229
Add DynamicGroup for runtime-defined option groups#5229uenoku wants to merge 3 commits intochipsalliance:mainfrom
Conversation
seldridge
left a comment
There was a problem hiding this comment.
High-level: is there a way to do this which doesn't make everything string-based?
| * @example {{{ val platform = new DynamicGroup("Platform", Seq("FPGA", "ASIC")) }}} | ||
| */ | ||
| class DynamicGroup(val groupName: String, caseNames: Seq[String])(implicit _sourceInfo: SourceInfo) { | ||
| import chisel3.internal.Builder |
There was a problem hiding this comment.
Nit: this is fine to include up top.
| /** Dynamic option group that accepts a name and case names as runtime parameters. | ||
| * @example {{{ val platform = new DynamicGroup("Platform", Seq("FPGA", "ASIC")) }}} | ||
| */ | ||
| class DynamicGroup(val groupName: String, caseNames: Seq[String])(implicit _sourceInfo: SourceInfo) { |
There was a problem hiding this comment.
I don't love it due to the complexity, but this should probably implement the val naming with a suggestName method like other things in Chisel.
There was a problem hiding this comment.
I think I want to disallow that level of dynamic behavior. The group name is essentially user/designer facing API even when they are generated dynamically.
I can provide following level of trait based interface, though I didn't like it because of the amount of boilerplate. trait PlatformType {
def FPGA: Case
def ASIC: Case
}
val platform = DynamicGroup[PlatformType]("Platform", Seq("FPGA", "ASIC")) { group =>
new PlatformType {
def FPGA = group("FPGA")
def ASIC = group("ASIC")
}
}
platform.FPGAIdeally I wanted to do following but I thought scala3 reflection would be necessary to do this. trait PlatformType {
def FPGA: Case
def ASIC: Case
}
val platform = DynamicGroup[PlatformType]("Platform")
platform.FPGA |
|
We could make this work: trait PlatformType extends DynamicGroup {
object FPGA extends Case
object ASIC extends Case
}
val platform = DynamicGroup[PlatformType]("Platform")
// or
val Platform = DynamicGroup(new PlatformType {})Or something like that without Scala reflection |
|
62ae69d implements trait based API that uses macro. I disclose that I needed quite a bit help from AI for writing DynamicGroupIntf.scala and it might be sloppy, so it might be better to start with a previous commit that's much more simpler. Though I believe that macro correctly implements API we generally want. |
Add DynamicGroup trait-based API for creating reusable option groups at runtime. - Add dynamicGroups HashMap to cache DynamicGroup instances - Add getOrCreateDynamicGroup/getOrCreateDynamicGroupInstance for instance management - Validate Groups with same name have same cases and order - Group options by name in buildImpl to handle multiple Group objects
62ae69d to
8f29623
Compare
|
Simplified the builder implementation a bit more. Currently dynamic options are stored in builder separately for the early validation, but it might be redundant since they are checked in |
6341873 to
f4064c1
Compare
Add DynamicGroup trait-based API for creating reusable option groups at runtime.
Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
Introduces a new DynamicGroup class that allows option groups
to be defined with runtime parameters instead
Reviewer Checklist (only modified by reviewer)
3.6.x,5.x, or6.xdepending on impact, API modification or big change:7.0)?Enable auto-merge (squash)and clean up the commit message.Create a merge commit.