Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

Bump to chisel 6 release #705

Merged
merged 8 commits into from
Feb 5, 2024
Merged

Bump to chisel 6 release #705

merged 8 commits into from
Feb 5, 2024

Conversation

kammoh
Copy link
Contributor

@kammoh kammoh commented Jan 31, 2024

Hi,
I'm trying to help with expediating a chiseltest 6.0.0 release as 6.0-SNAPSHOT does not work with the chisel-6.0.0 release.
I also noticed the Verilator coverage support was broken with Verilator versions >= 5.012 (and probably >=5).
Tested only on macOS. All tests (excluding VCS backend, but including Verilator, Icarus, and formal) pass. Tests were run as:
sbt "testOnly chiseltest.** -- -l RequiresVcs"

Please let me know if any changes are required or otherwise free to make any modifications.

Summary of changes:

  • Bumped chisel -> 6.0.0
  • Fixed missing (moved) 'Width' class
  • Bumped scala -> 2.13.12
  • Fixed compilation issues due to 'scala.io' namespace clash
  • bump sbt -> 1.9.8
  • fix Verilator 5.x coverage
  • fix Verilator 5.x tests

- Bumped chisel -> 6.0.0
 - Fixed missing (moved) 'Width' class
- Bumped scala -> 2.13.12
 - Fixed compilation issues due to 'scala.io' namespace clash
- bump sbt -> 1.9.8
- fix Verilator 5.x coverage
- fix Verilator 5.x tests
Copy link
Collaborator

@ekiwi ekiwi left a comment

Choose a reason for hiding this comment

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

Two small things that need to be addressed. Otherwise, this is awesome! Thanks a lot!

project/build.properties Show resolved Hide resolved
src/main/scala/chiseltest/simulator/ChiselBridge.scala Outdated Show resolved Hide resolved
- bump sbt-ci-release sbt plugin
- bump jna dependency
and encourage users to report it for further investigation and possible conversions/workarounds.
Copy link
Collaborator

@ekiwi ekiwi left a comment

Choose a reason for hiding this comment

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

This looks great! Will merge once all the tests pass.

I will try to make a 6.0.0 release of chiseltest soon.

@ekiwi
Copy link
Collaborator

ekiwi commented Jan 31, 2024

The failures in the formal tests are because of a new ConstType: chipsalliance/chisel@b55bfd0

I think we could probably just drop that info for now.

The sbt test on mac that is failing might be sporadic. I will restart it soon

- add jvm 21 to tests
- bump ubuntu (only verilator test for now) to 22.04 (LTS)
due to verilator/verilator#3993
(hopefully we can build newer versions of verilator on 20.04)
@kammoh
Copy link
Contributor Author

kammoh commented Jan 31, 2024

The link to the issue on the last commit comment should be -> verilator/verilator#2505
Edit:
In short due to bison changes we won't be able to test verilator versions prior to 4.040 on Ubuntu 22.04.

@kammoh
Copy link
Contributor Author

kammoh commented Jan 31, 2024

I sometimes see the exception on actual tests on mac. I'm not really familiar with the internals of the new schedular. Is there an easy way to fix it?

@ekiwi
Copy link
Collaborator

ekiwi commented Jan 31, 2024

I sometimes see the exception on actual tests on mac. I'm not really familiar with the internals of the new schedular. Is there an easy way to fix it?

That would probably take me a day or two and I don't really have that time right now. I will try to think about whether there is a quick workaround.

@kammoh
Copy link
Contributor Author

kammoh commented Jan 31, 2024

That would probably take me a day or two and I don't really have that time right now. I will try to think about whether there is a quick workaround.

It's ok with me if you'd rather hold back for the 6.0.0 release to include that fix, but in that case would it be possible to have a 6.0-SNAPSHOT release with these changes sooner?

@ekiwi
Copy link
Collaborator

ekiwi commented Jan 31, 2024

It's ok with me if you'd rather hold back for the 6.0.0 release to include that fix, but in that case would it be possible to have a 6.0-SNAPSHOT release with these changes sooner?

Yes. Once you fix the failure of the formal testsuite by dealing with the ConstType I will try to find a workaround.

@kammoh
Copy link
Contributor Author

kammoh commented Jan 31, 2024

Thanks!

@kammoh
Copy link
Contributor Author

kammoh commented Feb 5, 2024

I couldn't find much documentation on ConstType but it seems it should be safe to just take and convert the inner type. All tests pass with this workaround.

@ekiwi
Copy link
Collaborator

ekiwi commented Feb 5, 2024

I couldn't find much documentation on ConstType but it seems it should be safe to just take and convert the inner type

Thanks! I think so, too.

@ekiwi
Copy link
Collaborator

ekiwi commented Feb 5, 2024

Could you move the Verilator 5 CI tests to a separate PR? Seems like they are timing out right now and there is no reason they should keep this PR from getting merged. Thanks!

@kammoh
Copy link
Contributor Author

kammoh commented Feb 5, 2024

Typo in comment (should be "remove") but I'm leaving it for now. It would be great if you could fix it on merge.

@ekiwi ekiwi merged commit c9917be into ucb-bar:main Feb 5, 2024
29 checks passed
@ekiwi
Copy link
Collaborator

ekiwi commented Feb 5, 2024

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants