Skip to content
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

Remove Deprecated Usages of chisel3.Driver, CircuitForm #1481

Merged
merged 8 commits into from
Jun 23, 2020

Conversation

seldridge
Copy link
Member

Converts all tests to not use Chisel 3.3/FIRRTL 1.3 deprecated features, including:

  • ChiselStage methods are used instead of those in the BackendCompilationUtilities trait, e.g., for elaboration
  • Any custom transforms in tests are converted to use the Dependency API
  • All usages of chisel3.Driver are removed

In doing this, a new utility is added to extract a specific cause type from a stack trace:

  • A new utility, extractCause[A <: Throwable : ClassTag], is added that rethrows a nested A if it exists

A few deprecations are added:

  • Some lingering Driver infrastructure (e.g., ChiselExecutionResult) is deprecated

Related issue:

Type of change: other enhancement

Impact: no functional change

Development Phase: implementation

Release Notes

None.

Copy link
Contributor

@chick chick left a comment

Choose a reason for hiding this comment

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

Ouch, And I thought reviewing Firrtl PR #1700 was bad.
There are couple nits but this looks ready to go to me.

src/main/scala/chisel3/compatibility.scala Outdated Show resolved Hide resolved
src/test/scala/chiselTests/ChiselSpec.scala Outdated Show resolved Hide resolved
seldridge and others added 8 commits June 22, 2020 19:33
Adds a new method, chiselTests.Util.containsCause, that will search
for a polymorphic exception anywhere in a stack trace. This is useful
if exceptions may move around (e.g., if they are suddenly wrapped in a
StageError).

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
This migrates the tests to Chisel 3.4/FIRRTL 1.4. This primarily
involves removing usages of deprecated methods including:

- Remove usages of Driver
- Use ChiselStage methods instead of BackendCompilationUtilities
  methods
- Use Dependency API for custom transforms
- Use extractCause to unpack StackError

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Co-authored-by: Chick Markley <chick@qrhino.com>
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
@seldridge seldridge merged commit 9f44b59 into master Jun 23, 2020
@mergify mergify bot added the Backported This PR has been backported label Jun 23, 2020
seldridge added a commit that referenced this pull request Jun 23, 2020
…1492)

* Add containsCause exception search testing util
* Use ChiselStage in Tests
* Remove Driver usage in Emitter
* Remove Driver usage in ChiselSpec
* Remove Driver usage from Chisel._ package
* Deprecate Driver Execution classes
* Code simplification in internal Chisel._ method
* Clarify chiselTests.Utils.extractCause Scaladoc

Co-authored-by: Chick Markley <chick@qrhino.com>
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
@ucbjrl
Copy link
Contributor

ucbjrl commented Jun 23, 2020

Unfortunately, this commit (and its 3.3.x backport #1492) appear to cause chisel-testers to break:

 ...
[info] ScalaTest
[info] Run completed in 2 minutes, 25 seconds.
[info] Total number of tests run: 110
[info] Suites: completed 48, aborted 0
[info] Tests: succeeded 100, failed 10, canceled 0, ignored 1, pending 0
[info] *** 10 TESTS FAILED ***
[error] Failed: Total 110, Failed 10, Errors 0, Passed 100, Ignored 1
[error] Failed tests:
[error] 	examples.DecoupledAdderTester
[error] 	examples.SmallOdds4TesterSpec
[error] 	examples.SmallOdds3TesterSpec
[error] 	examples.DynamicMemorySearchTester
[error] 	examples.MaxNSpec
[error] 	examples.GCDTester
[error] 	examples.DecoupledRealGCDTester
[error] 	examples.HelloSpec
[error] 	examples.AdderTester
[error] 	examples.RouterUnitTesterSpec
[error] (Test / test) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 196 s (03:16), completed Jun 23, 2020, 4:45:43 PM

Compilation exited abnormally with code 1 at Tue Jun 23 16:45:44

with failures such as:

[info]   firrtl.passes.PassExceptions: firrtl.passes.CheckInitialization$RefNotInitializedException:  @[DecoupledAdder.scala 65:33] : [module DecoupledAdderTests]  Reference device_under_test is not fully initialized.
[info]    : device_under_test.io.in.valid <= VOID
[info] firrtl.passes.CheckInitialization$RefNotInitializedException:  @[DecoupledAdder.scala 65:33] : [module DecoupledAdderTests]  Reference device_under_test is not fully initialized.
[info]    : device_under_test.io.in.bits.b <= VOID
[info] firrtl.passes.CheckInitialization$RefNotInitializedException:  @[DecoupledAdder.scala 65:33] : [module DecoupledAdderTests]  Reference device_under_test is not fully initialized.
[info]    : device_under_test.io.in.bits.a <= VOID
[info] firrtl.passes.CheckInitialization$RefNotInitializedException:  @[DecoupledAdder.scala 65:33] : [module DecoupledAdderTests]  Reference device_under_test is not fully initialized.
[info]    : device_under_test.io.out.ready <= VOID
[info] firrtl.passes.PassException: 4 errors detected!
[info]   firrtl.passes.PassExceptions: firrtl.passes.CheckInitialization$RefNotInitializedException:  @[GCDUnitTest.scala 45:33] : [module GCDUnitTester]  Reference device_under_test is not fully initialized.
[info]    : device_under_test.io.a <= VOID
[info] firrtl.passes.CheckInitialization$RefNotInitializedException:  @[GCDUnitTest.scala 45:33] : [module GCDUnitTester]  Reference device_under_test is not fully initialized.
[info]    : device_under_test.io.b <= VOID
[info] firrtl.passes.CheckInitialization$RefNotInitializedException:  @[GCDUnitTest.scala 45:33] : [module GCDUnitTester]  Reference device_under_test is not fully initialized.
[info]    : device_under_test.io.e <= VOID
[info] firrtl.passes.PassException: 3 errors detected!
[info]   ...

seldridge added a commit that referenced this pull request Jun 24, 2020
This reverts TesterDriver.scala changes from
6e03f63.
@seldridge
Copy link
Member Author

seldridge commented Jun 24, 2020

@ucbjrl: I think this was only related to changes to the TesterDriver. I've narrowly reverted just that change in #1496.

And thanks for catching the regression!

seldridge added a commit that referenced this pull request Jun 24, 2020
This reverts TesterDriver.scala changes from
6e03f63.
@jackkoenig jackkoenig deleted the driver-deprecations branch July 7, 2021 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported This PR has been backported code improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants