[GitHub] [tvm] chiwwang commented on pull request #8220: [DOCS] Add docs for Pass Instrument
chiwwang commented on pull request #8220: URL: https://github.com/apache/tvm/pull/8220#issuecomment-876082490 Thanks you all!! I learn so much from all of feedbacks :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tvm] chiwwang commented on pull request #8220: [DOCS] Add docs for Pass Instrument
chiwwang commented on pull request #8220: URL: https://github.com/apache/tvm/pull/8220#issuecomment-873715155 Thanks @zackcquic @tkonolige !! @areusch you might want to check if this PR is ready for merging. Thanks :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tvm] chiwwang commented on pull request #8220: [DOCS] Add docs for Pass Instrument
chiwwang commented on pull request #8220: URL: https://github.com/apache/tvm/pull/8220#issuecomment-870176117 Thank you @tkonolige for so detailed reviewing! And sorry for my some careless grammar mistakes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tvm] chiwwang commented on pull request #8220: [DOCS] Add docs for Pass Instrument
chiwwang commented on pull request #8220: URL: https://github.com/apache/tvm/pull/8220#issuecomment-870176117 Thank you @tkonolige for so detailed reviewing! And sorry for my some careless grammar mistakes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tvm] chiwwang commented on pull request #8220: [DOCS] Add docs for Pass Instrument
chiwwang commented on pull request #8220: URL: https://github.com/apache/tvm/pull/8220#issuecomment-867389341 Gentle ping to see whether we have any comment on this change. Thanks! @areusch @tkonolige @ZihengJiang -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tvm] chiwwang commented on pull request #8220: [DOCS] Add docs for Pass Instrument
chiwwang commented on pull request #8220: URL: https://github.com/apache/tvm/pull/8220#issuecomment-860725987 Ah! I saw what confused me. It's the level of "Pass Registration" section. The current _pass_infra.rst_ section-hierachy is **Pass Infrastructure** (Topest) - The Design **The design of backend and frontend are described here.** * C++ Backend - PassContext - Pass Constructs - Module-Level Passes - Function-Level Passes - Sequential Passes * Pass Registration <-This section has the same level with Backend/Frontend. * Python Frontend - PassContext - Pass Objects Now I add Pass Instrument as: **Pass Infrastructure** (Topest) - The Design **The design of backend and frontend are described here.** * C++ Backend - PassContext - Pass Constructs - Module-Level Passes - Function-Level Passes - Sequential Passes - Pass Registration <- May I fix this to have the same level with other sub-sections in C++ backend? - Pass Instruments <--- Added in this PR. - Built-in Instrument <--- Added in this PR. * Python Frontend - PassContext - Pass Objects - Pass Instrument <--- Added in this PR. - Override Instruments in Current PassContext <--- Added in this PR. This might looks matching with descriptions in "The Design" section. Or, could we isolate Pass Instrument, and have anther topest section as **Pass Infrastructure**? May I know your thoughts @zackcquic @areusch ? Thanks a lot! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tvm] chiwwang commented on pull request #8220: [DOCS] Add docs for Pass Instrument
chiwwang commented on pull request #8220: URL: https://github.com/apache/tvm/pull/8220#issuecomment-859617614 Thanks for prompt feedbacks @zackcquic @tkonolige ! Here are some comments for Zack's questions: 1. What happens when exceptions occur in different instrument point. Added in pass_infra.txt. But it is a little long. You might want to take a look again. 2. Standard Instrument section: PassTimingInstrument, PrintBefore(TODO), PrintAfter(TODO), .. I think it might be better to maintain these in the __doc__ of related Python class/function. So I add example to instrument.py. 3. Global PassContext and override_instrument examples Done. Sorry for not aware of this approach. 4. use_pass_infra.py's comments should be updated, sorry, I forgot to update it. Done. 5. conf.py should be updated. Done. But actually it seems to automatically append un-listed tutorials to the end. How do you think about the current order of tutorial? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
