Skip to content

Conversation

@vegaluisjose
Copy link
Member

This PR refactors the Verilator runtime to support multiple options, including enabling a profiler for hardware designs that support performance counters. Also, I added some documentation to header files.

After this gets merged, my next goal is to update the Verilator app in VTA to showcase how this profiling information can be used when running actual models.

@tmoreau89 @liangfu

auto dealloc = reinterpret_cast<VerilatorDeallocFunc>(lib_->GetSymbol("VerilatorDealloc"));
ICHECK(dealloc != nullptr);
dealloc(device_);
lib_->~VerilatorLibrary();
Copy link
Member

Choose a reason for hiding this comment

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

just out of curiosity, why do we explicitly call the deallocator instead of delete lib_?

Copy link
Member

@liangfu liangfu left a comment

Choose a reason for hiding this comment

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

Thanks @vegaluisjose for bringing the change, especially in enabling the profiler feature. The implementation looks good.

Free feel to merge when you think this is ready to merge. @vegaluisjose

Copy link
Contributor

@tmoreau89 tmoreau89 left a comment

Choose a reason for hiding this comment

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

Thanks LGTM

@tmoreau89 tmoreau89 merged commit fc48514 into apache:main Feb 16, 2021
@tmoreau89
Copy link
Contributor

Thank you @vegaluisjose @liangfu , the PR has been merged.

Lokiiiiii pushed a commit to Lokiiiiii/tvm that referenced this pull request Mar 2, 2021
* new experiment

* save

* refactor

* refactor library

* add profiler

* refactor

* refactor

* add docs

* update comment

* add deallocator
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Mar 2, 2021
* new experiment

* save

* refactor

* refactor library

* add profiler

* refactor

* refactor

* add docs

* update comment

* add deallocator
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.

3 participants