[IR][Pass][Instrument] Pass instrument framework #7952

zackcquic · 2021-04-30T08:26:22Z

This commit provides utilities to instrument passes:

Add a new namespace tvm.instrument
Introduce PassInstrument and PassInstrumentor to PassContext

Example

passes_mem = #... Impl of memory instrument
passes_time = tvm.instrument.PassesTimeInstrument()

with tvm.transform.PassContext(
   pass_instrumentor=PassInstrumentor([passes_mem, passes_time])):

   tvm.relay.build(mod, 'llvm')

   passes_mem.rendor()
   passes_time.rendor()

Integrate existing PassContext::Trace() and timing profile

Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

Hi @altanh @tqchen:
I tried to integrate current passes profile mechanisms and make it more extendable, usage is as the commit's code example. Many parts are inspired by LLVM and MLIR.

How do you think?

This is my first attempt to TVM :), I have read through the guideline, but it there are stilling something wrong, please let me know.

Regards,
Zack

This commit provides utilies to instrument passes: 1. Add a new namespace tvm.instrument 2. Introduce PassInstrument and PassInstrumentor to PassContext Example ------- passes_mem = #... Impl of memory instrument passes_time = tvm.instrument.PassesTimeInstrument() with tvm.transform.PassContext( pass_instrumentor=PassInstrumentor([passes_mem, passes_time])): tvm.relay.build(mod, 'llvm') passes_mem.rendor() passes_time.rendor() 3. Integrate existing PassContext::Trace() and timing profile

tkonolige · 2021-04-30T19:33:36Z

Thanks for the contribution @zackcquic. In order to avoid duplicating code it might be worth unifying this with the runtime profiling framework (available here https://github.com/apache/tvm/blob/main/include/tvm/runtime/profiling.h). I have a branch (which I haven't submitted yet) that allows users to extend which kinds of information are collected. You can view the branch here: https://github.com/tkonolige/incubator-tvm/tree/profiler_papi. With this branch, I think the profiler supports all the features you want.

tqchen · 2021-04-30T20:32:10Z

Thank you @zackcquic ! Looks like a good addition, do you mind also create an RFC discussion post?

I agree it is important to have pass instrumentations, would be good to know how can it interact with Trace, since some of the callbacks might be similar and we need to unify the interface. On the design side, I think the runtime profiling and pass instrumentation might be different enough that might worth two separate solutions(perhaps a bit of reuse of timer if needed) As the former have more complexity wrt to GPU timer etc, while the later allows more statistics to be collected

zackcquic · 2021-05-01T00:21:57Z

@tqchen @tkonolige Here is the RFC:
https://discuss.tvm.apache.org/t/pass-instrument-framework-proposal/9874

And here is some reply for previous questions:

I would like to separate runtime profiling and pass instrumentation, too. This PR only focuses on pass profiling mechanisms like the passes time profiling, wants to make it easier to add more passes profiling implementations.

You might notice that this PR introduces a new namespace tvm.intrument. It intends to cover all instrument related (not limited to pass, but this PR only shows pass instrument), instead of mixing instrumentation/profiling code with transformation codes in tvm.transform. RuntimeProfiling could be add to this namespace, eg: tvm.instrument.Runtime.Profiling.

jroesch · 2021-05-01T00:43:57Z

I will try and comment over the weekend, thanks for the PR!

zackcquic · 2021-05-05T02:27:51Z

Hi @tqchen @jroesch

The CI failed, because of the following message:

docker: write /var/lib/docker/tmp/GetImageBlob707223609: no space left on device.

See 'docker run --help'.

script returned exit code 125

I think it may be not related to the change but the CI environment, right?

And another thing is the workflow:

1 workflow awaiting approval
First-time contributors need a maintainer to approve running workflows

I think it may require someone to approve, right?
Or I need to do something else?

zackcquic · 2021-05-05T22:41:39Z

I will try and comment over the weekend, thanks for the PR!

Hi @jroesch:

Just a gentle ping to receive feedback on this PR.

Thanks,
Zack

python/tvm/ir/instrument.py

include/tvm/ir/instrument.h

python/tvm/ir/transform.py

include/tvm/ir/instrument.h

tqchen · 2021-05-12T13:04:32Z

Sorry for the long delay. @zackcquic I have added review comments. One thing that would be helpful to get put more thoughts on is the naming. in particular what is the argument name when passing a list of pass instrumentors.

cc @junrushao1994 @vinx13 @altanh @zhiics @yzhliu Please also help to take a look

zackcquic · 2021-05-13T02:18:11Z

@tqchen Thanks a lot for review.

zackcquic · 2021-05-16T10:02:21Z

@tqchen I just made modification based on comments.

include/tvm/ir/instrument.h

tqchen · 2021-05-17T22:07:15Z

cc @jroesch @Hzfengsy @junrushao1994 @vinx13 @areusch @mbrookhart @yzhliu would be great if you can help to review this PR

zackcquic · 2021-05-18T05:07:46Z

cc @jroesch @Hzfengsy @junrushao1994 @vinx13 @areusch @mbrookhart @yzhliu would be great if you can help to review this PR

zackcquic · 2021-05-18T05:11:32Z

@tqchen requested changes updated

altanh · 2021-05-18T16:25:52Z

thanks for the PR and sorry for delay, I'll review this today!

altanh

Thanks for the changes, this looks much better now! Mostly spelling nits here, but also could you please add back the test for the PassTimingInstrument? Looks like it got removed here (previously was in tests/python/relay/test_pass_profiler.py).

include/tvm/ir/transform.h

python/tvm/ir/transform.py

tests/python/relay/test_pass_manager.py

zackcquic · 2021-05-25T01:59:29Z

Thanks for the changes, this looks much better now! Mostly spelling nits here, but also could you please add back the test for the PassTimingInstrument? Looks like it got removed here (previously was in tests/python/relay/test_pass_profiler.py).

I renamed test_pass_profiler.py to test_pass_instrument.py, and PassTimingInstrument are tested in this file.

altanh · 2021-05-25T05:06:06Z

Thanks for the changes, this looks much better now! Mostly spelling nits here, but also could you please add back the test for the PassTimingInstrument? Looks like it got removed here (previously was in tests/python/relay/test_pass_profiler.py).

I renamed test_pass_profiler.py to test_pass_instrument.py, and PassTimingInstrument are tested in this file.

Thanks, sorry I missed it (looks like github collapsed the file and I scrolled too fast...)

areusch

thanks for the detailed survey of pass instrumentation APIs and all of the test cases, @zackcquic ! I just have a few more questions about things and otherwise I think it's looking great.

tests/python/relay/test_pass_instrument.py

src/ir/transform.cc

areusch · 2021-05-26T17:22:08Z

src/ir/transform.cc

+    } catch (const Error& e) {
+      LOG(INFO) << "Pass instrumentation entering pass context failed.";
+      LOG(INFO) << "Disable pass instrumentation.";
+      pass_ctx_node->instruments.clear();


here is where I'm suggesting you should call pi->ExitPassContext() for all pi whose EnterPassContext has successfully returned so far.

Done and test case updated, thanks a lot.

zackcquic · 2021-05-27T09:10:23Z

@areusch @tqchen change request are all done.
Thanks a lot for reviewing efforts. 👍

areusch

thanks @zackcquic for working through this!

zackcquic · 2021-05-28T03:59:36Z

Hi @tqchen,

Just a ping for reviewing :), thanks a lot.

tqchen · 2021-05-28T17:44:29Z

Thanks @zackcquic @areusch @tkonolige @altanh

tqchen · 2021-05-28T17:47:27Z

Thanks @zackcquic for putting all the hard work polishing the API, improving the code and bring the implementation.

It would be great if we can followup to add design docs and examples :)

zackcquic · 2021-05-28T23:20:55Z

Thanks @zackcquic for putting all the hard work polishing the API, improving the code and bring the implementation.

It would be great if we can followup to add design docs and examples :)

Sure, I will add it. Thank you very much.

* [IR][Pass][Instrument] Pass instrument framework This commit provides utilies to instrument passes: 1. Add a new namespace tvm.instrument 2. Introduce PassInstrument and PassInstrumentor to PassContext Example ------- passes_mem = #... Impl of memory instrument passes_time = tvm.instrument.PassesTimeInstrument() with tvm.transform.PassContext( pass_instrumentor=PassInstrumentor([passes_mem, passes_time])): tvm.relay.build(mod, 'llvm') passes_mem.rendor() passes_time.rendor() 3. Integrate existing PassContext::Trace() and timing profile * [IR][Pass][Instrument] Fix python test_pass_manager.py * Fix comment * Fix lint * Fix test_pass_annotation * Fix test_pass_annotation.py * Fix lint * Fix test_pass_annotation.py * Fix test_pass_annotation.py * Fix review comments * Fix tutorial use_pass_infra.py * Fix review comments * Fix review comments * Fix typo * Fix review comments * Fix review comments * Fix unittest error: test_cow_pass * Fix unittest error * Add more test cases for exceptions * Fix nit * Doc override_instruments() * Fix review comments * Fix lint * Fix EnterContext exception behavior

zackcquic added 2 commits April 30, 2021 22:53

[IR][Pass][Instrument] Fix python test_pass_manager.py

0ad48d4

jroesch self-assigned this May 1, 2021

tqchen requested changes May 12, 2021

View reviewed changes

tqchen added the status: need review label May 12, 2021

zackcquic added 2 commits May 16, 2021 17:41

Fix comment

dc33594

Fix lint

62ffb72

zackcquic added 5 commits May 16, 2021 21:39

Fix test_pass_annotation

d358e99

Fix test_pass_annotation.py

ffcfef0

Fix lint

0cce273

Fix test_pass_annotation.py

9ed716c

Fix test_pass_annotation.py

9f34b7c

tqchen requested changes May 17, 2021

View reviewed changes

include/tvm/ir/instrument.h Show resolved Hide resolved

Fix review comments

3e407c0

zackcquic closed this May 18, 2021

zackcquic reopened this May 18, 2021

Fix tutorial use_pass_infra.py

5b578b1

zackcquic requested a review from areusch May 23, 2021 12:12

Add more test cases for exceptions

06ee84d

tkonolige approved these changes May 24, 2021

View reviewed changes

altanh suggested changes May 24, 2021

View reviewed changes

zackcquic added 2 commits May 25, 2021 09:31

Fix nit

58c456d

Doc override_instruments()

0dd603f

altanh approved these changes May 25, 2021

View reviewed changes

areusch requested changes May 25, 2021

View reviewed changes

tests/python/relay/test_pass_instrument.py Outdated Show resolved Hide resolved

tests/python/relay/test_pass_instrument.py Outdated Show resolved Hide resolved

tests/python/relay/test_pass_instrument.py Outdated Show resolved Hide resolved

src/ir/transform.cc Outdated Show resolved Hide resolved

zackcquic added 2 commits May 26, 2021 01:41

Fix review comments

7c504d8

Fix lint

3cbdcb0

zackcquic requested a review from areusch May 25, 2021 17:48

areusch reviewed May 26, 2021

View reviewed changes

Fix EnterContext exception behavior

291c03c

zackcquic requested a review from areusch May 26, 2021 22:49

areusch approved these changes May 27, 2021

View reviewed changes

tqchen approved these changes May 28, 2021

View reviewed changes

tqchen merged commit ece644c into apache:main May 28, 2021

tqchen added status: accepted and removed status: need review labels May 28, 2021

zackcquic deleted the dev branch May 29, 2021 03:17

chiwwang mentioned this pull request Jun 9, 2021

[DOCS] Add docs for Pass Instrument #8220

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IR][Pass][Instrument] Pass instrument framework #7952

[IR][Pass][Instrument] Pass instrument framework #7952

zackcquic commented Apr 30, 2021

tkonolige commented Apr 30, 2021

tqchen commented Apr 30, 2021 •

edited

Loading

zackcquic commented May 1, 2021

jroesch commented May 1, 2021

zackcquic commented May 5, 2021 •

edited

Loading

zackcquic commented May 5, 2021

tqchen commented May 12, 2021

zackcquic commented May 13, 2021 •

edited

Loading

zackcquic commented May 16, 2021

tqchen commented May 17, 2021

zackcquic commented May 18, 2021

zackcquic commented May 18, 2021

altanh commented May 18, 2021

altanh left a comment

zackcquic commented May 25, 2021 •

edited

Loading

altanh commented May 25, 2021

areusch left a comment

areusch May 26, 2021

zackcquic May 26, 2021 •

edited

Loading

zackcquic commented May 27, 2021

areusch left a comment

zackcquic commented May 28, 2021

tqchen commented May 28, 2021

tqchen commented May 28, 2021

zackcquic commented May 28, 2021

[IR][Pass][Instrument] Pass instrument framework #7952

[IR][Pass][Instrument] Pass instrument framework #7952

Conversation

zackcquic commented Apr 30, 2021

Example

tkonolige commented Apr 30, 2021

tqchen commented Apr 30, 2021 • edited Loading

zackcquic commented May 1, 2021

jroesch commented May 1, 2021

zackcquic commented May 5, 2021 • edited Loading

zackcquic commented May 5, 2021

tqchen commented May 12, 2021

zackcquic commented May 13, 2021 • edited Loading

zackcquic commented May 16, 2021

tqchen commented May 17, 2021

zackcquic commented May 18, 2021

zackcquic commented May 18, 2021

altanh commented May 18, 2021

altanh left a comment

Choose a reason for hiding this comment

zackcquic commented May 25, 2021 • edited Loading

altanh commented May 25, 2021

areusch left a comment

Choose a reason for hiding this comment

areusch May 26, 2021

Choose a reason for hiding this comment

zackcquic May 26, 2021 • edited Loading

Choose a reason for hiding this comment

zackcquic commented May 27, 2021

areusch left a comment

Choose a reason for hiding this comment

zackcquic commented May 28, 2021

tqchen commented May 28, 2021

tqchen commented May 28, 2021

zackcquic commented May 28, 2021

tqchen commented Apr 30, 2021 •

edited

Loading

zackcquic commented May 5, 2021 •

edited

Loading

zackcquic commented May 13, 2021 •

edited

Loading

zackcquic commented May 25, 2021 •

edited

Loading

zackcquic May 26, 2021 •

edited

Loading