-
Notifications
You must be signed in to change notification settings - Fork 615
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
Fix widths for literal values in Bundle literals #4082
Conversation
Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation.
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.
approving based on the tests!
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.
LGTM
Mega-nit throughout about full-sentence comments not ending in periods.
@@ -122,7 +122,6 @@ private[chisel3] object ir { | |||
} | |||
|
|||
/** Provides a mechanism that LitArgs can have their width adjusted | |||
* to match other members of a VecLiteral |
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.
Stray change.
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.
Not stray actually, I am calling the function from Bundle literals now lol.
Super annoyingly, the check for literals being too wide causes things to fail. This is basically a case of implicit truncation. Instead of erroring on it, I'm going to just truncate in this PR (since this will backport to 3.6.x). I will then follow with a PR that turns it into a warning for master and 6.x |
This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel.
* Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57) # Conflicts: # core/src/main/scala/chisel3/internal/firrtl/IR.scala # src/test/scala/chiselTests/BundleLiteralSpec.scala # src/test/scala/chiselTests/ConnectableSpec.scala # src/test/scala/chiselTests/VecLiteralSpec.scala
* Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57) # Conflicts: # core/src/main/scala/chisel3/internal/firrtl/IR.scala # src/test/scala/chiselTests/ConnectableSpec.scala # src/test/scala/chiselTests/VecLiteralSpec.scala
* Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57)
* Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57) # Conflicts: # core/src/main/scala/chisel3/internal/firrtl/IR.scala # src/test/scala/chiselTests/ConnectableSpec.scala # src/test/scala/chiselTests/VecLiteralSpec.scala
* Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57)
) * Fix widths for literal values in Bundle literals (#4082) * Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57) * Waive false binary compatibility failure Also move waivers from files to build.sbt. --------- Co-authored-by: Jack Koenig <[email protected]>
) * Fix widths for literal values in Bundle literals (#4082) * Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57) # Conflicts: # core/src/main/scala/chisel3/internal/firrtl/IR.scala # src/test/scala/chiselTests/BundleLiteralSpec.scala # src/test/scala/chiselTests/ConnectableSpec.scala # src/test/scala/chiselTests/VecLiteralSpec.scala * Resolve backport conflicts and waive false MiMa failure. --------- Co-authored-by: Jack Koenig <[email protected]>
* Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57) # Conflicts: # core/src/main/scala/chisel3/internal/firrtl/IR.scala # src/test/scala/chiselTests/ConnectableSpec.scala # src/test/scala/chiselTests/VecLiteralSpec.scala
) * Fix widths for literal values in Bundle literals (#4082) * Fix widths for literal values in Bundle literals Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. * Implicitly truncate too-wide literals in Bundle literals This is to fix a bug in concatenation without breaking things relying on implicit truncation. This will be a warning in newer versions of Chisel. (cherry picked from commit 3939e57) # Conflicts: # core/src/main/scala/chisel3/internal/firrtl/IR.scala # src/test/scala/chiselTests/ConnectableSpec.scala # src/test/scala/chiselTests/VecLiteralSpec.scala * Waive false binary compatibility failure Also move waivers from files to build.sbt. --------- Co-authored-by: Jack Koenig <[email protected]>
Contributor Checklist
docs/src
?Type of Improvement
Desired Merge Strategy
Release Notes
Previously, the user-specified (or unspecified minimum width) of the literal would be used in some operations like concatenation. For literal values that are too-wide, they will now truncate to the correct width. This will become a warning (then later an error) in newer major versions of Chisel.
Reviewer Checklist (only modified by reviewer)
3.6.x
,5.x
, or6.x
depending on impact, API modification or big change:7.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.