-
Notifications
You must be signed in to change notification settings - Fork 298
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
[FIRRTL] Relax ODS defined FIRRTL base subtypes to allow type aliases #5652
Conversation
d9bd286
to
8b01bca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, mostly mechanical, but this is a major change.
Maybe converting one major .fir
lit test to type alias, should be good for this PR. The alias would be dropped at LowerToHW
though, until the other PR is merged.
@@ -1042,7 +1042,7 @@ void Emitter::emitExpression(SpecialConstantOp op) { | |||
|
|||
// NOLINTNEXTLINE(misc-no-recursion) | |||
void Emitter::emitExpression(SubfieldOp op) { | |||
auto type = op.getInput().getType(); | |||
BundleType type = op.getInput().getType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implicit casts are helpful !!
f445876
to
ff4c119
Compare
@@ -46,17 +46,19 @@ circuit test_mod : %[[{"class": "circt.testNT", "data": "a"}]] | |||
|
|||
; VERILOG-IR: sv.verbatim{{.*}}Standard header to adapt well known macros to our needs | |||
; VERILOG-IR: sv.macro.def @INIT_RANDOM_PROLOG_ | |||
type V3 = UInt<1>[3] | |||
type V2 = UInt<1>[2] | |||
|
|||
module test_mod : | |||
input clock : Clock | |||
input a: UInt<1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems type alias for integers is not well supported in HW/Comb. There are still a lot of places where normal llvm casts are used. So i added type alias only for vectors.
ff4c119
to
2519eeb
Compare
This PR introduces
BaseTypeAliasOr
helper struct to create a type which accepts both type aliases and a specified type.Unfortunately this PR makes it hard to use
TypedValue<Foo>
which is returned by standard ODS generated getters since now the type will becomeTypedValue<BaseTypeAliasOr<Foo>>
.