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

Overload connect for StrongEnum annotations #915

Closed
wants to merge 1 commit into from
Closed

Conversation

seldridge
Copy link
Member

Possible fix for Strong Enum annotations problems. Currently, the annotations generated by StrongEnum are invalid.

I tried to figure out a way to handle this by fixing the bind overload, but I couldn't seem to find how to differentiate bind in the following two cases:

// case 1
val expected_state = VecInit(sNone, sNone) // for some enum sNone
// case 2
val y: UInt = ???
val x = expected_state(y) // <- this access

Currently, this produces annotations of the following form for the former:

  {
    "class":"chisel3.core.EnumAnnotations$EnumComponentAnnotation",
    "target":"StrongEnumFSMTester.StrongEnumFSMTester.expected_state[0]",
    "enumTypeName":"chiselTests.StrongEnumFSM$State"
  },
  {
    "class":"chisel3.core.EnumAnnotations$EnumComponentAnnotation",
    "target":"StrongEnumFSMTester.StrongEnumFSMTester.expected_state[1]",
    "enumTypeName":"chiselTests.StrongEnumFSM$State"
  }

And for the latter:

  {
    "class":"chisel3.core.EnumAnnotations$EnumComponentAnnotation",
    "target":"StrongEnumFSMTester.StrongEnumFSMTester.expected_state[value]",
    "enumTypeName":"chiselTests.StrongEnumFSM$State"
  }

The latter shouldn't be emitted as this is pointing to an invalid target. This PR changes it so that only the former is emitted.

Questions:

  • Does this need to also override bulkConnect?

Related issue: chipsalliance/firrtl#922

Type of change: other enhancement

Impact: API modification

Development Phase: implementation

Release Notes

This switches from using an overloaded bind to an overloaded connect when
emitting StrongEnum annotations. Use of bind proved difficult to
differentiate between assignment and subindexing.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge requested a review from a team as a code owner October 29, 2018 17:23
@hngenc
Copy link
Contributor

hngenc commented Oct 29, 2018

Unfortunately, this will still fail when we connect a vector element to something else, as in the following two cases:

val i = WireInit(0.U)
expected_state(i) := sNone
// Fails with: java.lang.NumberFormatException: For input string: "_T_1"
expected_state(0.U) := sNone
// Fails with: Illegal component name: expected_state[UInt<1>("h00")]

@seldridge
Copy link
Member Author

@hngenc: Thanks for taking at this PR and for digging into the details of the problem underlying this. Yeah, connect overriding breaks down there. I'll close this in favor of a forthcoming better approach...

@seldridge seldridge closed this Oct 29, 2018
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.

2 participants