Enum deficiencies

[ducky64]

• Enum values can't be used in testing, since they only return a Chisel-land data type rather than a Scala-land Int.
• nodeType may be meaningless / boilerplate - I don't see a case for anything other than UInt. That nodeType exists also forces Chisel to keep the Bits.fromInt method which isn't exposed or used elsewhere.
• Enum values aren't type-safe, they return generic Chisel types like UInt which aren't treated differently. Whether this is desirable is an open question, much like the Scala debate between Enumeration and case objects

Thoughts?

[aswaterman]

Re: point 2: I'd favor creating Enum(n) which implicitly does the same thing as Enum(UInt(log2Ceil(n)), n) currently does. You could then deprecate Enum(nodeType, n) in new-chisel3. I agree, it's not really obvious what an SInt enum would do, since there's no facility to start at anything besides 0.

(In C, having signed enums is occasionally useful, because you're allowed to set the start number to something negative. I don't see a compelling reason to follow their lead in this case, though.)

[aswaterman]

Re: the other points: despite its downsides, it's still useful.

[ducky64]

Yes, we definitely do need to keep the current Enum (perhaps with nodeType deprecated, and assertion-checked to be a UInt so we can remove Bits.fromInt), but it would be interesting to think of what a more featured enum system would look like.

[ducky64]

So in the attempt to deprecate old style enums I came across some more issues:

• The Symbol-based enum generators (def apply[T <: Bits](nodeType: T, l: Symbol *) and def apply[T <: Bits](nodeType: T, l: List[Symbol]) are really old code, and are used in neither chisel3 tests nor rocket-chip. Since they're not compile-time-checked and (in my opinion) strictly inferior to the list one, I'm thinking of removing those completely (without even a deprecation phase).
• Technically, enums can take a nodeType of specified width, though this doesn't see use in rocket either, and it's unclear whether this would ever be a good idea (since the user has no control over the numeric values). I'm thinking of dropping this feature and having chisel3 assert out if a width is specified.

Thoughts?

[mwachs5]

I have to say that as a new Chisel user, the first point,
"Enum values can't be used in testing, since they only return a Chisel-land data type rather than a Scala-land Int." immediately bit me, and made me wonder what the point of Enums is.

Actually I'm still not sure what the point is vs using a non-Chisel Scala Enum (however you like doing that) since the Enum-ness is not reflected in the actual emitted Verilog. If you're going to go through the trouble of creating a special Chisel Enum, why not emit the Verilog equivalent so that the Verilog would have meaningful names/values in them (something which most Waveform viewers can pick up on). I wish that there was a way to not throw this Verilog feature away.

[ducky64]

So right now even Chisel doesn't know the names you've given to the enum variables, this is because of the way it's structured.

It seems Scala Enumerations are written completely in Scala (though using some fancy reflection method) and it should be possible to get names from those, so a more full-featured version of Chisel enums can be written like that.

There's also another style of doing enums in Scala which involves defining the type as a trait and the values as case objects extending that trait. Macros or reflection can then be used to get all the enumeration values. This style allows greater flexibility at the expense of more lines of code (it's typical to define one case object per line).

One of those two above patterns could be adapted to support Chisel-specific functionality like enum type bitwidth, UInt literal conversion, and maybe name propagation.

[colinschmidt]

Adding the names to the output seems really cool/useful, but I think that FIRRTL would most likely not preserve this. (constant-prop and whatnot)

[schoeberl]

In the ideal world enums should preserve their names down to HDL code. At least in VHDL you can have symbolic names and let the synthesize tool decide which encoding is most efficient (e.g., simple binary or one-hot). Cheers, Martin

…

On 29 Nov, 2016, at 21:22, Colin Schmidt ***@***.***> wrote: Adding the names to the output seems really cool/useful, but I think that FIRRTL would most likely not preserve this. (constant-prop and whatnot) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#373 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAntmK_tZy4fKrlXTaqLwXoVqGBpKMsIks5rDIlpgaJpZM4K16ku>.

[seldridge]

One old issue with this, the current Chisel Enum does suffer from the 22-limit on tuples. The map-based Enum was the only way of getting around this that I was aware of. It would be good if there was a fallback method that didn't require the use of the Compatibility package.

[jackkoenig]

A minor issue with the current Enum(n) implementation is that you cannot use capitalized val names (common style for constants). Because Enum names things via unapply, Scala attempts to match capitalized variables rather than binding new ones.

[chick]

Resolved by Strong Enums