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

Cleanup harness generation and add missing vl_finish(). #208

Merged
merged 1 commit into from
Aug 20, 2018

Conversation

ucbjrl
Copy link
Contributor

@ucbjrl ucbjrl commented Aug 2, 2018

I believe this solves the mystery of why some versions of verilator appear to fail with a:

Undefined symbols for architecture x86_64:
  "vl_finish(char const*, int, char const*)", referenced from:
      VL_FINISH_MT(char const*, int, char const*) in verilated.o
ld: symbol(s) not found for architecture x86_64

The harness generated by chisel3/src/main/resources/chisel3/top.cpp contains a definition of vl_finish() while the harness generated by chisel-testers/src/main/scala/chisel3/iotesters/VerilatorBackend.scala doesn't.

Tests run with pure Chisel3 and FIRRTL will pass without problems, while tests using chisel-testers will fail at the link stage.

This PR converts multiple codeBuffer.append()s with individual C++ source lines into two codeBuffer.append()s with triple quotes and adds the missing vl_finish() definition.

@ucbjrl ucbjrl added the bugfix label Aug 2, 2018
@ucbjrl ucbjrl added this to the 1.2.2 milestone Aug 2, 2018
@ucbjrl ucbjrl requested a review from chick August 2, 2018 19:48
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.

This makes the harness code much easier to read.

@ucbjrl ucbjrl merged commit ab6a6f8 into master Aug 20, 2018
@ucbjrl ucbjrl deleted the cleanupharness branch August 20, 2018 16:08
@ucbjrl ucbjrl modified the milestones: 1.2.2, 1.2.6, 1.2.3 Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants