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

Add vcs backend support #58

Open
wants to merge 4 commits into
base: master
from

Conversation

@chick
Copy link
Contributor

chick commented Aug 21, 2019

  • Create new VcsBackendAnnotation
  • Create support utilities for Vcs building tools
    • CopyVpiFiles move vpi and header files into place
    • GenVcsVerilogHarness Compile stuff
    • VcsAnnotations control behavior of the Vcs backend
  • Create VcsBackend, a trivial sub-classing of VerilatorBackend
  • Create VcsExecutive
    • constructs VcsBackEnd requirements calls the constructor
  • Create SimpleVcsTests
  • Added String to Uniquify Verilator and Vcs Tests
  • Test are ignored if VCS not present
chick added 2 commits Aug 21, 2019
- Create new VcsBackendAnnotation
- Create support utilities for Vcs building tools
  - CopyVpiFiles move vpi and header files into place
  - GenVcsVerilogHarness  Compile stuff
  - VcsAnnotations control behavior of the Vcs backend
- Create VcsBackend, a trivial sub-classing of VerilatorBackend
- Create VcsExecutive
  - constructs VcsBackEnd requirements calls the constructor
- Create SimpleVcsTests
- Added String to Uniquify Verilator and Vcs Tests
@chick chick requested a review from ducky64 Aug 21, 2019
@chick chick self-assigned this Aug 21, 2019
@chick

This comment has been minimized.

Copy link
Contributor Author

chick commented Aug 21, 2019

I am trying to find a setup where I can test this, will keep DO NOT MERGE label until then
But code should be reviewable now

@chick chick removed the DO NOT MERGE label Aug 21, 2019
@ducky64 ducky64 added this to the 0.1 Basic usability milestone Sep 16, 2019
Copy link
Member

ducky64 left a comment

A few minor comments, looks fine otherwise. I mainly looked at the test code, we could probably significantly clean up the main implementation some other day.

Files.createFile(vpiTabFilePath)
} catch {
case _: FileAlreadyExistsException =>
System.out.format("")

This comment has been minimized.

Copy link
@ducky64
@@ -8,7 +8,7 @@ import chisel3.tester.internal.VerilatorBackendAnnotation
import org.scalatest._

class VerilatorBasicTests extends FlatSpec with ChiselScalatestTester with Matchers {
behavior of "Testers2"
behavior of "Testers2_Verilator"

This comment has been minimized.

Copy link
@ducky64

ducky64 Sep 18, 2019

Member

Maybe cleaner as "Testers2 with Verilator"? I don't think you need an underscore in quotes?

import org.scalatest._

class VcsBasicTests extends FlatSpec with ChiselScalatestTester with Matchers {
behavior of "Testers2_Vcs"

This comment has been minimized.

Copy link
@ducky64

ducky64 Sep 18, 2019

Member

ditto, "testesr2 with vcs"?

}
}

it should "fail on poking outputs" in {

This comment has been minimized.

Copy link
@ducky64

ducky64 Sep 18, 2019

Member

This seems more like a testers2 infrastructure exception, not sure if it belongs in a VCS test?

}
}

it should "fail with user-defined message" in {

This comment has been minimized.

Copy link
@ducky64

ducky64 Sep 18, 2019

Member

Ditto, is this VCS-specific? I don't think so, or at least the control paths are redundant with the fail-on-expect simple case?

chick added 2 commits Sep 18, 2019
- Fixed CopyVpiFiles
  - looks like this might fail if early file was there but a late one wasn't
  - this code came from chisel-testers
- Fix top level test names
  - behavior of "Testers2 with Vcs"
  - behavior of "Testers2 with Verilator"
- Shortened VCS to just see if a simple circuit will build and simulate.
  - in long run we still need mechanism to run an arbitrary test with arbitrary backend
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.