Skip to content

Resolve implicit-width-truncation warnings - #92

Closed
tynan-jdwk wants to merge 1 commit into
ucb-bar:masterfrom
tynan-jdwk:fix-width-truncation
Closed

Resolve implicit-width-truncation warnings#92
tynan-jdwk wants to merge 1 commit into
ucb-bar:masterfrom
tynan-jdwk:fix-width-truncation

Conversation

@tynan-jdwk

Copy link
Copy Markdown

firtool-1.154.0 and later warn on all implicit width truncations (llvm/circt#10621). These width truncations were flagged in stock Chipyard builds.

For all width truncations, use the equivalent Connectable operator (:<=), along with .squeeze. This has the same behavior as the original := connections, since those truncate when the producer in a connection is wider than the consumer.

firtool-1.154.0 and later warn on all implicit width truncations. These
width truncations were flagged in stock Chipyard builds.

For all width truncations, use the equivalent `Connectable` operator
(`:<=`), along with `.squeeze`. This has the same behavior as the
original `:=` connections, since those truncate when the producer in a
connection is wider than the consumer.

@nibrunieAtSi5 nibrunieAtSi5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tynan-jdwk, did you verify on a few configurations that the emitted verilog was indeed identical? (and more generally, besides checking that the warning were removed, how was this PR tested?)

@tynan-jdwk

Copy link
Copy Markdown
Author

Thanks for taking a look @nibrunieAtSi5! I tested on two configurations from Chipyard: RocketConfig (which is a 64-bit config: WithNHugeCores), plus RV32RocketConfig (which is a 32-bit config: WithNHugeCores with WithRV32).

For the RV64 build, only these files were different:

  • DivSqrtRawFN_small_e5_s11.sv
  • DivSqrtRawFN_small_e8_s24.sv
  • DivSqrtRawFN_small_e11_s53.sv

For the RV32 build, only these files were different:

  • DivSqrtRawFN_small_e5_s11.sv
  • DivSqrtRawFN_small_e8_s24.sv

I'm attaching an archive which has those files built from the existing master branch, plus this new fix-width-truncation branch. It also has the firtool warnings.

hardfloat-width-truncation.tar.gz

Annoyingly, some signals get renamed due to these changes, but if you reverse the renaming, the new files are text-identical to the old ones.

@tynan-jdwk

Copy link
Copy Markdown
Author

Well, the Connectable operators were only added in Chisel 3.6.0 (chipsalliance/chisel#2806). Should we remove the Chisel 3.5.6 test in CI, or would you rather I rework these connections to use explicit bit-slices to avoid using Connectable?

@aswaterman

Copy link
Copy Markdown
Member

CI is failing because :<= and squeeze don't exist in earlier versions. @jackkoenig what's your recommendation as to how to proceed? Is it time to drop 3.5.6 support?

@jackkoenig

Copy link
Copy Markdown

I would drop 3.5.6 support (and thus use of the old Scala FIRRTL Compiler).

Because it's only 7 lines I think it is also fine to just use explicit bit extraction if you prefer to keep the old Chisel around.

@aswaterman

Copy link
Copy Markdown
Member

Because it's only 7 lines I think it is also fine to just use explicit bit extraction if you prefer to keep the old Chisel around.

I think this is the better choice, since we don't know who else is using this repo, and we should favor reducing disruption given the ease of doing so in this case. See #93.

aswaterman added a commit that referenced this pull request Aug 19, 2026
Addresses #92 without backwards incompatibility.

Signed-off-by: Andrew Waterman <andrew@sifive.com>
@aswaterman

Copy link
Copy Markdown
Member

Superseded by #93

@aswaterman aswaterman closed this Aug 19, 2026
@tynan-jdwk
tynan-jdwk deleted the fix-width-truncation branch August 19, 2026 05:06
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.

4 participants