Preserve literals across .asTypeOf #4168

jackkoenig · 2024-06-12T03:51:12Z

Built on top of #4160 so should merge this one after that one (might need rebase but git seems smarter about cherry-picks these days).

This is very valuable for ChiselSim-style testing because it means that you can much more easily construct literals in tests.

Contributor Checklist

Did you add Scaladoc to every public function/method?
Did you add at least one test demonstrating the PR?
Did you delete any extraneous printlns/debugging code?
Did you specify the type of improvement?
Did you add appropriate documentation in docs/src?
Did you request a desired merge strategy?
Did you add text to be included in the Release Notes for this change?

Type of Improvement

API modification

Desired Merge Strategy

Squash

Release Notes

Casting a literal (of any type) to another type with .asTypeOf will result in a literal of the new type. For non-literals, the FIRRTL representation will now be a little bit more efficient.

Reviewer Checklist (only modified by reviewer)

Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
Did you mark the proper milestone (Bug fix: 3.6.x, 5.x, or 6.x depending on impact, API modification or big change: 7.0)?
Did you review?
Did you check whether all relevant Contributor checkboxes have been checked?
Did you do one of the following when ready to merge:
- Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
- Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

jackkoenig · 2024-06-12T05:05:37Z

I was hoping to backport this because I think it's potentially a very useful feature for ChiselSim and 6.x is closer to 3.6.x which makes for easier porting, but alas it seems that it is a really common pattern to use 0.U.asTypeOf(new MyBundle) to initialize a Wire of type MyBundle that then as a few fields connected to.

This change is still good and worth making, but it needs to be on a major version since it will break those uses (they just need to add a WireInit).

jackkoenig · 2024-06-13T04:58:13Z

Converted to draft temporarily. I want to deprecate assigning to the wire returned by .asTypeOf (and backport to 6.x) before merging this. I am almost done with the PR doing that deprecation (which involves adding my long desired .readOnly feature!), should open it tomorrow.

jackkoenig · 2024-06-25T20:39:49Z

This is ready to go

jackkoenig · 2024-06-26T20:09:56Z

src/test/scala/chiselTests/stage/WarningConfigurationSpec.scala


-    it("should number AsTypeOfReadOnly as 8") {
-      val args = Array("--warn-conf", "id=8:e,any:s", "--throw-on-first-error")
+    it("should now error on AsTypeOfReadOnly") {
+      val args = Array("--throw-on-first-error")
      val e = the[Exception] thrownBy ChiselStage.emitCHIRRTL(new AsTypeOfReadOnly, args)
-      e.getMessage should include("[W008] Return values of asTypeOf will soon be read-only")
+      e.getMessage should include("Return values of asTypeOf are now read-only")
    }


So this is the first time we'll be elevating a warning to an error. I haven't really worked through it all, we could add a corresponding E008 for the old W008, but no other errors are numbered yet (and of course, unlike warnings, you will not be allowed to configure errors). Any thoughts?

it seems like if we want to add error numbers we should do that more holistically? separately from this PR?

core/src/main/scala/chisel3/ChiselEnum.scala

core/src/main/scala/chisel3/internal/package.scala

Casting a literal (of any type) to another type with .asTypeOf will result in a literal of the new type. For non-literals, the FIRRTL representation will now be a little bit more efficient.

`UnknownWidth` became a `case object` in this Chisel PR: chipsalliance/chisel#4242 The `connectFromBits` method was removed in this Chisel PR: chipsalliance/chisel#4168

`UnknownWidth` became a `case object` in this Chisel PR: chipsalliance/chisel#4242 The `connectFromBits` method was removed in this Chisel PR: chipsalliance/chisel#4168 The `connectFromBits` API was replaced with the `_fromUInt` API in this Chisel PR: chipsalliance/chisel#4782 The sbt update is necessary to maintain compatibility with the latest Scala compiler version.

`UnknownWidth` became a `case object` in this Chisel PR: chipsalliance/chisel#4242 The `connectFromBits` method was removed in this Chisel PR: chipsalliance/chisel#4168 The `connectFromBits` API was replaced with the `_fromUInt` API in this Chisel PR: chipsalliance/chisel#4782 The `NumObject` trait was replaced by the `Num` object in this PR: chipsalliance/chisel#4768 The sbt update is necessary to maintain compatibility with the latest Scala compiler version.

`UnknownWidth` became a `case object` in this Chisel PR: chipsalliance/chisel#4242 The `connectFromBits` method was removed in this Chisel PR: chipsalliance/chisel#4168 The `connectFromBits` API was replaced with the `_fromUInt` API in this Chisel PR: chipsalliance/chisel#4782 The `NumObject` trait was replaced by the `Num` object in this PR: chipsalliance/chisel#4768 Wrapping unary negation (`unary_-%`) was deprecated in this PR: chipsalliance/chisel#4829 The sbt update is necessary to maintain compatibility with the latest Scala compiler version.

`UnknownWidth` became a `case object` in this Chisel PR: chipsalliance/chisel#4242 The `connectFromBits` method was removed in this Chisel PR: chipsalliance/chisel#4168 The `connectFromBits` API was replaced with the `_fromUInt` API in this Chisel PR: chipsalliance/chisel#4782 Wrapping unary negation (`unary_-%`) was deprecated in this PR: chipsalliance/chisel#4829 The sbt update is necessary to maintain compatibility with the latest Scala compiler version.

* Update Chisel from 6.5.0 to 7.0.0 `UnknownWidth` became a `case object` in this Chisel PR: chipsalliance/chisel#4242 The `connectFromBits` method was removed in this Chisel PR: chipsalliance/chisel#4168 The `connectFromBits` API was replaced with the `_fromUInt` API in this Chisel PR: chipsalliance/chisel#4782 Wrapping unary negation (`unary_-%`) was deprecated in this PR: chipsalliance/chisel#4829 The sbt update is necessary to maintain compatibility with the latest Scala compiler version. * Migrate from Chisel testers to ChiselSim The `chisel3.testers` package was removed here: chipsalliance/chisel#4746 It doesn't look like ChiselSim has the same `Boolean` result from `simulate`, so we must check for simulation errors by catching exceptions. * Replace ChiselRunners trait with direct use of ChiselSim * Update to latest scalafmt * lint: Apply latest scalafmt settings * Update to latest scalatest version * Fix missing sbt command in GitHub CI For background: actions/setup-java#712 Also updated JDK to 21 (latest LTS), updated some GitHub actions to their latest versions, and using install-circt GitHub action to get firtool. * Fix out-of-range literals in tests An 8-bit signed integer with a binary point at 2 effectively has a 6-bit signed number for the value to the left of the decimal point. That means it can represent values in the range [-2^5, 2^5-1], or [-32, 31]. After chipsalliance/chisel#4786 was merged, the `55` and `56` literals are now correctly flagged as being out-of-range for this type. I've subtracted 32 from these out-of-range values (changing the MSB from 1 to 0), and the tests now pass. * Update Chisel 7 snapshot from 639-5df5515f to 678-dca5fc11 This update includes this Chisel PR, which deprecated everything in the `firrtl` package: chipsalliance/chisel#4878 Bump `firtool` version to latest as well. * Update Chisel version to 7.0.0-RC1 Also update CI versions of firtool and verilator to latest. * Update Chisel version to 7.5.0

jackkoenig added the Feature New feature, will be included in release notes label Jun 12, 2024

jackkoenig added this to the 6.x milestone Jun 12, 2024

jackkoenig requested review from mwachs5 and seldridge June 12, 2024 03:51

jackkoenig force-pushed the preserve-literals-asTypeOf branch from d285777 to 78a7e9a Compare June 12, 2024 04:11

jackkoenig modified the milestones: 6.x, 7.0 Jun 12, 2024

jackkoenig added API Modification and removed Feature New feature, will be included in release notes labels Jun 12, 2024

jackkoenig marked this pull request as draft June 13, 2024 04:57

This was referenced Jun 13, 2024

It would be nice if ProbeValues of Literals would automatically expand widths #4174

Open

Add support for marking things as readOnly #4190

Merged

mergify bot mentioned this pull request Jun 20, 2024

Add support for marking things as readOnly (backport #4190) #4194

Merged

14 tasks

jackkoenig force-pushed the preserve-literals-asTypeOf branch from 78a7e9a to 88f93e7 Compare June 25, 2024 20:39

jackkoenig marked this pull request as ready for review June 25, 2024 20:39

jackkoenig force-pushed the preserve-literals-asTypeOf branch from 88f93e7 to 982664d Compare June 25, 2024 22:11

jackkoenig commented Jun 26, 2024

View reviewed changes

jackkoenig force-pushed the preserve-literals-asTypeOf branch from 982664d to de6d98e Compare June 28, 2024 17:55

mwachs5 reviewed Jul 1, 2024

View reviewed changes

core/src/main/scala/chisel3/ChiselEnum.scala Outdated Show resolved Hide resolved

mwachs5 reviewed Jul 1, 2024

View reviewed changes

core/src/main/scala/chisel3/internal/package.scala Outdated Show resolved Hide resolved

jackkoenig force-pushed the preserve-literals-asTypeOf branch from de6d98e to ca5062c Compare July 2, 2024 21:29

Preserve literals across .asTypeOf

f487c92

Casting a literal (of any type) to another type with .asTypeOf will result in a literal of the new type. For non-literals, the FIRRTL representation will now be a little bit more efficient.

jackkoenig force-pushed the preserve-literals-asTypeOf branch from ca5062c to f487c92 Compare July 2, 2024 23:40

mwachs5 approved these changes Jul 3, 2024

View reviewed changes

jackkoenig merged commit 8f00067 into main Jul 3, 2024

jackkoenig deleted the preserve-literals-asTypeOf branch July 3, 2024 22:33

tymcauley mentioned this pull request Jul 16, 2024

Update Chisel from 6.5.0 to 7.0.0 ucb-bar/fixedpoint#11

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Preserve literals across .asTypeOf #4168

Preserve literals across .asTypeOf #4168

Uh oh!

jackkoenig commented Jun 12, 2024 •

edited

Loading

Uh oh!

jackkoenig commented Jun 12, 2024

Uh oh!

jackkoenig commented Jun 13, 2024

Uh oh!

jackkoenig commented Jun 25, 2024

Uh oh!

jackkoenig Jun 26, 2024

Uh oh!

mwachs5 Jul 1, 2024

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Preserve literals across .asTypeOf #4168

Preserve literals across .asTypeOf #4168

Uh oh!

Conversation

jackkoenig commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contributor Checklist

Type of Improvement

Desired Merge Strategy

Release Notes

Reviewer Checklist (only modified by reviewer)

Uh oh!

jackkoenig commented Jun 12, 2024

Uh oh!

jackkoenig commented Jun 13, 2024

Uh oh!

jackkoenig commented Jun 25, 2024

Uh oh!

jackkoenig Jun 26, 2024

Choose a reason for hiding this comment

Uh oh!

mwachs5 Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jackkoenig commented Jun 12, 2024 •

edited

Loading