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

[VTA] add support to event counters #3347

Merged
merged 9 commits into from
Jun 13, 2019
Merged

[VTA] add support to event counters #3347

merged 9 commits into from
Jun 13, 2019

Conversation

vegaluisjose
Copy link
Member

This PR adds infrastructure (software and hardware) to support event counters in the VTA Chisel implementation. Only counting cycles at the moment.

This is how cycles are now displayed on vta unittest (test_vta_insn.py)

$ python3 test_vta_insn.py
Initialize VTACommandHandle...
Load/store test took 619 clock cycles
Padded load test took 8579 clock cycles
GEMM schedule:default test took 648 clock cycles
GEMM schedule:smt test took 755 clock cycles
ALU SHL imm:True test took 1067 clock cycles
ALU MAX imm:True test took 1067 clock cycles
ALU MAX imm:False test took 1654 clock cycles
ALU ADD imm:True test took 1067 clock cycles
ALU ADD imm:False test took 1654 clock cycles
ALU SHR imm:True test took 1067 clock cycles
Relu test took 1738 clock cycles
Shift/scale test took 386 clock cycles
Close VTACommandhandle...

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.

One question regarding the event counters in this example: does cycle count get reset every time we invoke the simulator on a new "workload"?

@tmoreau89
Copy link
Contributor

I suggest that @tqchen also takes a look at how profile information is retrieved

vta/python/vta/testing/simulator.py Outdated Show resolved Hide resolved
Returns
-------
stats : dict
Current profiler statistics
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears that we are not returning a dictionary here; perhaps we should to accommodate for more profile information (and use the "cycles" as a key for the cycle count)

Copy link
Member Author

@vegaluisjose vegaluisjose Jun 12, 2019

Choose a reason for hiding this comment

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

We are not because I am just returning a cycle element at the moment. I was thinking that once we think on a second counter, we can added then.

@vegaluisjose
Copy link
Member Author

The event counter can be set to zero from software and hardware. In this particular case, the counter is counting only when this condition is met

  when (io.launch && !io.finish) {
    cycle_cnt := cycle_cnt + 1.U
  } .otherwise {
    cycle_cnt := 0.U
  }

This basically means when VTA launches but is not finished. I also added this to tsim-driver just to show how to set the counter from zero but in this case really does not matter.

Also, the idea of this PR is to provide a mechanism (hardware) that can automatically generate event-counter-register and map addresses accordingly in hardware, see how the VCR looks now

The "final" take on the number of counters and what we would want to count should come next after this.

* can count the total number of clock cycles spent in a VTA run by checking
* launch and finish signals.
*
* The event counter value is passed to the VCR module via the ecnt port, so
Copy link
Member

Choose a reason for hiding this comment

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

Can we talk a little bit about how we add a new counter, maybe pointers to the places that need to be changed? it looks like the event count vector has a length set somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I can add more. The ecnt vector params are taken from here

see Line 38 from link above

  val vp = p(ShellKey).vcrParams

The design patter around params is that every time you see something like myModuleParams" somewhere, then you should go to myModule` to see what parameters are defined there.

The idea here is that if the number of event counters or nECnt is changed in VCR then the interface will automatically change as well, without needing to do it manually. The EventCounters.scala file will just implement "what you want to count" logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, I just added a little bit of more info on how to add counters.

@tmoreau89 tmoreau89 merged commit 7bf2ff2 into apache:master Jun 13, 2019
@vegaluisjose vegaluisjose deleted the dev branch June 14, 2019 05:42
wweic pushed a commit to wweic/tvm that referenced this pull request Jun 26, 2019
* add support to event counters in VTA

* fix comment

* fix event-counter interface parameter

* no longer needed

* add sim back

* add docs to event counters

* fix docs

* add more details about event counting

* make dpi-module docs more accurate
wweic pushed a commit to neo-ai/tvm that referenced this pull request Jun 27, 2019
* add support to event counters in VTA

* fix comment

* fix event-counter interface parameter

* no longer needed

* add sim back

* add docs to event counters

* fix docs

* add more details about event counting

* make dpi-module docs more accurate
tqchen pushed a commit to tqchen/tvm that referenced this pull request Mar 29, 2020
* add support to event counters in VTA

* fix comment

* fix event-counter interface parameter

* no longer needed

* add sim back

* add docs to event counters

* fix docs

* add more details about event counting

* make dpi-module docs more accurate
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