[GitHub] [tvm] gromero commented on pull request #8086: [TVMC] Add support for the MLF to 'compile' command

2021-05-23 Thread GitBox


gromero commented on pull request #8086:
URL: https://github.com/apache/tvm/pull/8086#issuecomment-846736014


   Hi @areusch and @leandron PTAL. In that v2 I addressed your review comments, 
added 3 pytest tests to check tvmc + MLF, and rebased the code resolving the 
conflicts with recent changes (like those related to the addition of 
`--desired-layout`  and `--cross-options` flags). 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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [tvm] zhuzilin edited a comment on pull request #8085: [Relay][PRNG] Support generating data of any shape in threefry_generate

2021-05-23 Thread GitBox


zhuzilin edited a comment on pull request #8085:
URL: https://github.com/apache/tvm/pull/8085#issuecomment-846697224


   @tkonolige I've added the comment and rebased the code to main branch to fix 
the shape limit in `uniform`. Could you take another look? Thank you~


-- 
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] Beya2019 opened a new pull request #8116: [Hybrid Script]Fix some syntax errors

2021-05-23 Thread GitBox


Beya2019 opened a new pull request #8116:
URL: https://github.com/apache/tvm/pull/8116


   Fixed some syntax errors in the hybrid_script document.
   ```
   for i in range(5):
   s = 0 # declaration, this s will be a 1-array in lowered IR
   for j in range(5):
  s += a[i, j] # do something with sum
   b[i] = sum # you can still use sum in this level
   a[0] = s # you CANNOT use s here, even though it is allowed in conventional 
Python
   ```
   
   @tqchen @merrymercy @kevinthesun Would you please have a look at this? 
Thanks very much.


-- 
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] zhuzilin commented on pull request #8085: [Relay][PRNG] Support generating data of any shape in threefry_generate

2021-05-23 Thread GitBox


zhuzilin commented on pull request #8085:
URL: https://github.com/apache/tvm/pull/8085#issuecomment-846697224


   @tkonolige I've added the comment and rebase the code to main branch to fix 
the shape limit in `uniform`. Could you take another look? Thank you~


-- 
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] junrushao1994 commented on pull request #8114: [TensorIR][M2a] Verification of cached flags

2021-05-23 Thread GitBox


junrushao1994 commented on pull request #8114:
URL: https://github.com/apache/tvm/pull/8114#issuecomment-846631033


   CC: @tqchen @jroesch @comaniac @icemelon9 @zhiics


-- 
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] tqchen commented on pull request #8115: [APPS] Add logging to the bundle.

2021-05-23 Thread GitBox


tqchen commented on pull request #8115:
URL: https://github.com/apache/tvm/pull/8115#issuecomment-846610260


   cc @tkonolige @junrushao1994 


-- 
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] tqchen opened a new pull request #8115: [APPS] Add logging to the bundle.

2021-05-23 Thread GitBox


tqchen opened a new pull request #8115:
URL: https://github.com/apache/tvm/pull/8115


   Fixes https://github.com/apache/tvm/issues/7856
   


-- 
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] tqchen closed issue #8033: build tvm CMakeFiles/Makefile2:136: recipe for target 'CMakeFiles/tvm_objs.dir/all' failed

2021-05-23 Thread GitBox


tqchen closed issue #8033:
URL: https://github.com/apache/tvm/issues/8033


   


-- 
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] junrushao1994 opened a new pull request #8114: [TensorIR][M2a] Verification of cached flags (#390)

2021-05-23 Thread GitBox


junrushao1994 opened a new pull request #8114:
URL: https://github.com/apache/tvm/pull/8114


   This PR is part of the TensorIR upstreaming effort (#7527), stage M2a.
   
   In this PR, we implemented cached flag verification, which will check the 
correctness of the following flags if the schedule class is set to debug:
   - is_affine_binding
   - region_cover
   - is_stage_pipeline
   
   To enable the detection of the region cover property, we also bring in 
several integer set analysis along with this PR.


-- 
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




[tvm] branch main updated: [Docs] Update stale links (#8111)

2021-05-23 Thread marisa
This is an automated email from the ASF dual-hosted git repository.

marisa pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm.git


The following commit(s) were added to refs/heads/main by this push:
 new ebf80cb  [Docs] Update stale links (#8111)
ebf80cb is described below

commit ebf80cbc35a7cd9839b5adb213ed8d77738dcc3f
Author: Yuchen Jin 
AuthorDate: Sun May 23 09:53:31 2021 -0700

[Docs] Update stale links (#8111)
---
 docs/dev/pass_infra.rst   | 4 ++--
 docs/dev/relay_add_op.rst | 2 +-
 docs/dev/relay_add_pass.rst   | 4 ++--
 docs/langref/relay_type.rst   | 2 +-
 src/relay/analysis/annotated_region_set.h | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/docs/dev/pass_infra.rst b/docs/dev/pass_infra.rst
index 6bd4689..67ef30a 100644
--- a/docs/dev/pass_infra.rst
+++ b/docs/dev/pass_infra.rst
@@ -344,7 +344,7 @@ We've covered the concept of different level of passes and 
the context used for
 compilation. It would be interesting to see how easily users can register
 a pass.  Let's take const folding as an example. This pass has already been
 implemented to fold constants in a Relay function (found in
-`src/relay/pass/fold_constant.cc`_).
+`src/relay/transforms/fold_constant.cc`_).
 
 An API was provided to perform the ``Expr`` to ``Expr`` transformation.
 
@@ -536,7 +536,7 @@ optimization pipeline and debug Relay and tir passes, 
please refer to the
 
 .. _src/ir/transform.cc: 
https://github.com/apache/tvm/blob/main/src/ir/transform.cc
 
-.. _src/relay/pass/fold_constant.cc: 
https://github.com/apache/tvm/blob/main/src/relay/pass/fold_constant.cc
+.. _src/relay/transforms/fold_constant.cc: 
https://github.com/apache/tvm/blob/main/src/relay/transforms/fold_constant.cc
 
 .. _python/tvm/relay/transform/transform.py: 
https://github.com/apache/tvm/blob/main/python/tvm/relay/transform/transform.py
 
diff --git a/docs/dev/relay_add_op.rst b/docs/dev/relay_add_op.rst
index c5dce83..f9ade45 100644
--- a/docs/dev/relay_add_op.rst
+++ b/docs/dev/relay_add_op.rst
@@ -469,7 +469,7 @@ Adding a Gradient in C++
 Adding a gradient in C++ is similar to adding one in Python, but the
 interface for registering is slightly different.
 
-First, make sure ``src/relay/pass/pattern_utils.h`` is included. It provides
+First, make sure ``src/relay/transforms/pattern_utils.h`` is included. It 
provides
 helper functions for creating nodes in the Relay AST. Then, define the
 gradient in a similar fashion as in the Python example:
 
diff --git a/docs/dev/relay_add_pass.rst b/docs/dev/relay_add_pass.rst
index 0661df0..90211d0 100644
--- a/docs/dev/relay_add_pass.rst
+++ b/docs/dev/relay_add_pass.rst
@@ -397,10 +397,10 @@ the below code applies both the ``FoldConstant`` and 
``ToANormalForm`` passes
 More detail about registration can be found in :ref:`tvm-runtime-system` and 
more
 information about the pass manager interface can be found in :ref:`pass-infra`.
 Relay's standard passes are listed in `include/tvm/relay/transform.h`_ and 
implemented
-in `src/relay/pass/`_.
+in `src/relay/transforms/`_.
 
 .. _include/tvm/relay/transform.h: 
https://github.com/apache/tvm/blob/main/include/tvm/relay/transform.h
 
-.. _src/relay/pass/: https://github.com/apache/tvm/tree/main/src/relay/pass
+.. _src/relay/transforms/: 
https://github.com/apache/tvm/tree/main/src/relay/transforms
 
 .. _src/relay/transforms/fold_constant.cc: 
https://github.com/apache/tvm/blob/main/src/relay/transforms/fold_constant.cc
diff --git a/docs/langref/relay_type.rst b/docs/langref/relay_type.rst
index 0fc19b7..632c638 100644
--- a/docs/langref/relay_type.rst
+++ b/docs/langref/relay_type.rst
@@ -43,7 +43,7 @@ Relay to be ahead-of-time compiled and provides much more 
information about
 tensors for optimizations further in the compilation pipeline. Such 
optimizations
 can be implemented as passes, which are Relay-to-Relay AST transformations, and
 may use the inferred types (e.g., shape information) for making decisions about
-program transformations. For instance, :code:`src/relay/pass/fuse_ops.cc` gives
+program transformations. For instance, 
:code:`src/relay/transforms/fuse_ops.cc` gives
 an implementation of a pass that uses inferred tensor shapes to replace 
invocations
 of operators in a Relay program with fused operator implementations.
 
diff --git a/src/relay/analysis/annotated_region_set.h 
b/src/relay/analysis/annotated_region_set.h
index d9923cc..d225cb8 100644
--- a/src/relay/analysis/annotated_region_set.h
+++ b/src/relay/analysis/annotated_region_set.h
@@ -18,7 +18,7 @@
  */
 
 /*!
- * \file tvm/relay/pass/annotated_region_set.h
+ * \file tvm/relay/transforms/annotated_region_set.h
  * \brief Define data structures to extract and manipulate regions from
  * a relay function. Regions are denoted by region_begin and region_end
  * annotations that exist on all the input and output edges of the region.


[GitHub] [tvm] MarisaKirisame merged pull request #8111: [Docs] Update stale links

2021-05-23 Thread GitBox


MarisaKirisame merged pull request #8111:
URL: https://github.com/apache/tvm/pull/8111


   


-- 
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] MarisaKirisame commented on pull request #8111: [Docs] Update stale links

2021-05-23 Thread GitBox


MarisaKirisame commented on pull request #8111:
URL: https://github.com/apache/tvm/pull/8111#issuecomment-846592984


   Thanks @YuchenJin @junrushao1994 !


-- 
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] zackcquic commented on pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic commented on pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#issuecomment-846553311


   @tkonolige @areusch @altanh @tqchen Changes are done.


-- 
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] zackcquic commented on pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic commented on pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#issuecomment-846553197


   ### Collector of pass instrumentation instance
   **Following table shows the class and API that collects a set of pass 
instrumentations in different projects:**
   
   Note:
   - [IO] = Pass instrumentations are processed in order
   - [RO] = Pass instrumentations are processed in reverse order
   - Entry empty = No mapping API
   - mlir::PassInstrumentor provides thread-safety.
   
   | This PR (PassContext.instruments)  | llvm::PassInstrumentation 
  | mlir::PassInstrumentor |
   | -- | 
--- | -- |
   | [IO] bool InstrumentBeforePass()   | [IO] bool runBeforePass() 
  | [IO] void runBeforePass()  |
   | [IO] bool InstrumentAfterPass()| [IO] void runAfterPass()  
  | [RO] void runAfterPass()   |
   || [IO] void 
runAfterPassInvalidated() ||
   ||   
  | [RO] void runAfterPassFailed() |
   || [IO] void runBeforeAnalysis() 
  | [IO] void runBeforeAnalysis()  |
   || [IO] void runAfterAnalysis()  
  | [RO] void runAfterAnalysis()   |
   || [IO] void 
runAnalysisInvalidated()  ||
   || [IO] void runAnalysesCleared()
  ||
   | [IO] void InstrumentEnterPassContext() |   
  | [IO] void runBeforePipeline()  |
   | [IO] void InstrumentExitPassContext()  |   
  | [RO] void runAfterPipeline()   |
   ### Pass instrumentation instance
   **Following table shows the class and API to implement a pass 
instrumentation in different projects:**
   
   Note:
   -  In LLVM, a pass instrumentation provides a set of callbacks and register 
them in ```llvm::PassInstrumentationCallbacks```,
   which will be called in collectors at different instrumentation points.
   - In MLIR and this PR, pass instrumentation subclassing base class and 
overriding methods.
   
   | This PR (instrument::PassInstrumentNode)   | 
llvm::PassInstrumentationCallbacks | mlir::PassInstrumentation |
   | -  | 
-- | --|
   |  virtual bool ShouldRun()  | void 
registerShouldRunOptionalPassCallback(CallableT)  // CallableT = 
bool(StringRef, Any), called in llvm::PassInstrumentation | |
   | | void registerBeforeSkippedPassCallback(CallableT)  // CallableT = 
void(StringRef, Any), called in llvm::PassInstrumentation | |
   | | void registerBeforeNonSkippedPassCallback(CallableT)  // CallableT = 
void(StringRef, Any), called in llvm::PassInstrumentation | |
   | virtual void RunBeforePass()   | | virtual void 
runBeforePass() |
   | virtual void RunAfterPass()  | void registerAfterPassCallback(CallableT)  
 // CallableT = void(StringRef, Any, const PreservedAnalyses &), called in 
llvm::PassInstrumentation | virtual void runAfterPass()
   | |void registerAfterPassInvalidatedCallback(CallableT)  // CallableT = 
void(StringRef, const PreservedAnalyses &), called in llvm::PassInstrumentation 
| |
   | |  void registerBeforeAnalysisCallback(CallableT)   // CallableT = 
void(StringRef, Any), called in llvm::PassInstrumentation | virtual void 
runBeforeAnalysis() |
   | |  void registerAfterAnalysisCallback(CallableT)  // CallableT = 
void(StringRef, Any), called in llvm::PassInstrumentation | virtual void 
runAfterAnalysis() |
   | |  void registerAnalysisInvalidatedCallback(CallableT)   // CallableT 
= void(StringRef, Any), called in llvm::PassInstrumentation | |
   | |  void registerAnalysesClearedCallback(CallableT)  // CallableT = 
void(StringRef), called in llvm::PassInstrumentation | |
   | virtual void EnterPassContext() | | virtual void runBeforePipeline() |
   | virtual void ExitPassContext() | | virtual void runAfterPipeline() |
   
   ### RunBeforePass API
   Following is the code snip from  ```bool 
llvm::PassInstrumentation::runBeforePass()```.
   It splits the ```ShouldRun()``` and ```RunBefore()``` logic from a pass 
instrumentation, like @areusch recommended.

   ```c++
 /// BeforePass instrumentation point - takes \p Pass instance to be 
executed
 /// and constant reference to IR it operates on. \Returns true if pass is
 /// allowed to be executed. These are only run on optional pass since 
required
 /// passes must always be run. This allows these callbacks to print info 
when
 /// they want to skip a pass.
 template 
 bool runBeforePass(const PassT , const IRUnitT ) const 

[GitHub] [tvm] xqdan commented on pull request #8079: Complete register op from python

2021-05-23 Thread GitBox


xqdan commented on pull request #8079:
URL: https://github.com/apache/tvm/pull/8079#issuecomment-846551249


   @zackcquic please take another look


-- 
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] PhilippvK commented on pull request #7401: WIP/RFC: initial stab at TorchScript fallback

2021-05-23 Thread GitBox


PhilippvK commented on pull request #7401:
URL: https://github.com/apache/tvm/pull/7401#issuecomment-846545791


   Hey @t-vi, the idea of a fallback for unsupported TorchScript Ops is great. 
I am currently pursuing a similar approach for unsupported (and custom) TFLite 
Ops.
   
   I also stumbled over the issue that  `num_inpust == -1` leads to problems in 
the `type_infer` step and "solved" it in a quite bad way by just packing all my 
inputs in a single Tuple and using `num_inputs = 1`. I would really appreciate 
getting at least your fix to solve this issue merged into upstream. Maybe in a 
separate PR at this is not really related to the TorchScript use case.


-- 
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] zackcquic commented on pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic commented on pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#issuecomment-846538241


   > > For the reason not to overload superclass, I don't know, but I have a 
guess, maybe need someone to confirm:
   > > Currently, only FunctionPass, ModulePass, SequentialPass and 
PrimFuncPass extends Pass.
   > > FunctionPass calls InferType at the end (see the comments for detail) 
while SequentialPass is just like a wrapper.
   > > In these cases, InterType and SequentialPass maybe not want to be 
traced/instrumented.
   > 
   > I disagree with this, mainly because some (perhaps not so) surprising 
results I got with my initial pass profiling implementation is that `InferType` 
is called **a lot** and often due to the `FunctionPass`/`SequentialPass` 
design, leading to quite a bit of compilation slowdown. This information would 
not have been obtained if we didn't include all passes that are run in the 
profiling.
   > 
   > Now, I wonder if we should introduce a "universal" pass context which 
_every_ PassNode must respect and has access to (perhaps a thread local one)? I 
definitely want to make sure the timing instrument can time all passes that are 
run.
   
   @altanh Thanks.
   Just checked, YES, ```InferType``` is called **a lot** than I expected.
   So I updated like you suggested, instrument in Pass::operator() instead of 
subclasses' operator().


-- 
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] zackcquic commented on a change in pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic commented on a change in pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#discussion_r637521860



##
File path: include/tvm/ir/instrument.h
##
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ir/instrument.h
+ *
+ * This file introduces a pass instrument infrastructure, inspired by LLVM and 
MLIR.
+ * It inserts instrumentation points around passes.
+ */
+#ifndef TVM_IR_INSTRUMENT_H_
+#define TVM_IR_INSTRUMENT_H_
+
+#include 
+#include 
+
+#include 
+#include 
+
+namespace tvm {
+
+class IRModule;
+
+// Forward class for PassInstrumentNode methods
+namespace transform {
+class PassInfo;
+}  // namespace transform
+
+namespace instrument {
+
+/*!
+ * \brief PassInstrumentNode forms an instrument implementation.
+ * It provides API for users to register callbacks at different 
instrumentation points.
+ *
+ * Within a pass context (tvm::transfom::PassContext), the instrumentation 
call sequence will like:
+ *
+ *   Instrument SetUp()
+ *
+ * if (Instrument Before Pass())

Review comment:
   Just checked LLVM and its logic is pretty like you commented.
   
   Logic updated and test cases added.
   
   For   _**providing the after_pass with a "result" notion**_, could you give 
more elaboration?




-- 
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] zackcquic commented on a change in pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic commented on a change in pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#discussion_r637521526



##
File path: src/ir/instrument.cc
##
@@ -0,0 +1,314 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file src/ir/instrument.cc
+ * \brief Infrastructure for instrumentation.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+namespace tvm {
+namespace instrument {
+
+/*!
+ * \brief A named PassInstrument implementation

Review comment:
   Changed:
   - Made "name" a member in abstract class ```PassInstrumentNode```.
   - Implement``BasePassInstrument``` for FFI here. 




-- 
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] zackcquic removed a comment on pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic removed a comment on pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#issuecomment-845250654


   Hi @areusch:
   
   Could you elaborate more on "after_pass with a result notation"?
   What kind of result and use scenario you expected?  
   That will be very helpful for me. 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] zackcquic commented on a change in pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic commented on a change in pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#discussion_r637521199



##
File path: python/tvm/ir/instrument.py
##
@@ -0,0 +1,154 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name,unused-argument
+"""Common pass instrumentation across IR variants."""
+import inspect
+import functools
+
+import tvm._ffi
+import tvm.runtime
+
+from . import _ffi_instrument_api
+
+
+@tvm._ffi.register_object("instrument.PassInstrument")
+class PassInstrument(tvm.runtime.Object):
+"""A pass instrument implementation.
+
+Users don't need to interact with this class directly.
+Instead, a `PassInstrument` instance should be created through 
`pass_instrument`.
+
+See Also
+
+`pass_instrument`
+"""
+
+
+def _wrap_class_pass_instrument(pi_cls):
+"""Wrap a python class as pass instrument"""
+
+class PyPassInstrument(PassInstrument):
+"""Internal wrapper class to create a class instance."""
+
+def __init__(self, *args, **kwargs):
+# initialize handle in cass pi_cls creation failed.fg
+self.handle = None
+inst = pi_cls(*args, **kwargs)
+
+# check method declartion within class, if found, wrap it.
+def create_method(method):
+if hasattr(inst, method) and inspect.ismethod(getattr(inst, 
method)):
+
+def func(*args):
+return getattr(inst, method)(*args)
+
+func.__name__ = "_" + method
+return func
+return None
+
+# create runtime pass instrument object
+# reister instance's run_before_pass, run_after_pass, set_up and 
tear_down method
+# to it if present.
+self.__init_handle_by_constructor__(
+_ffi_instrument_api.NamedPassInstrument,
+pi_cls.__name__,
+create_method("run_before_pass"),
+create_method("run_after_pass"),
+create_method("set_up"),
+create_method("tear_down"),
+)
+
+self._inst = inst
+
+def __getattr__(self, name):
+# fall back to instance attribute if there is not any
+return self._inst.__getattribute__(name)
+
+functools.update_wrapper(PyPassInstrument.__init__, pi_cls.__init__)

Review comment:
   Just keep consistent with 
```_wrap_class_module_pass```[transform.py:206](https://github.com/apache/tvm/blob/8934d6b5af1defc6ccd9e18fc26dac1a27f4546b/python/tvm/ir/transform.py#L206)
 




-- 
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] zackcquic commented on a change in pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic commented on a change in pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#discussion_r637521086



##
File path: python/tvm/ir/instrument.py
##
@@ -0,0 +1,154 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# pylint: disable=invalid-name,unused-argument
+"""Common pass instrumentation across IR variants."""
+import inspect
+import functools
+
+import tvm._ffi
+import tvm.runtime
+
+from . import _ffi_instrument_api
+
+
+@tvm._ffi.register_object("instrument.PassInstrument")
+class PassInstrument(tvm.runtime.Object):
+"""A pass instrument implementation.
+
+Users don't need to interact with this class directly.
+Instead, a `PassInstrument` instance should be created through 
`pass_instrument`.
+
+See Also
+
+`pass_instrument`
+"""
+
+
+def _wrap_class_pass_instrument(pi_cls):
+"""Wrap a python class as pass instrument"""
+
+class PyPassInstrument(PassInstrument):
+"""Internal wrapper class to create a class instance."""
+
+def __init__(self, *args, **kwargs):
+# initialize handle in cass pi_cls creation failed.fg
+self.handle = None
+inst = pi_cls(*args, **kwargs)
+
+# check method declartion within class, if found, wrap it.
+def create_method(method):
+if hasattr(inst, method) and inspect.ismethod(getattr(inst, 
method)):
+
+def func(*args):
+return getattr(inst, method)(*args)
+
+func.__name__ = "_" + method
+return func
+return None
+
+# create runtime pass instrument object
+# reister instance's run_before_pass, run_after_pass, set_up and 
tear_down method
+# to it if present.
+self.__init_handle_by_constructor__(

Review comment:
   Yes, self.handle is initialized at line 49.
   This wrapper function mimics ```_wrap_class_module_pass```  
[transform.py:188](https://github.com/apache/tvm/blob/8934d6b5af1defc6ccd9e18fc26dac1a27f4546b/python/tvm/ir/transform.py#L188)




-- 
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] zackcquic commented on a change in pull request #7952: [IR][Pass][Instrument] Pass instrument framework

2021-05-23 Thread GitBox


zackcquic commented on a change in pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#discussion_r637520285



##
File path: include/tvm/ir/instrument.h
##
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ir/instrument.h
+ *
+ * This file introduces a pass instrument infrastructure, inspired by LLVM and 
MLIR.
+ * It inserts instrumentation points around passes.
+ */
+#ifndef TVM_IR_INSTRUMENT_H_
+#define TVM_IR_INSTRUMENT_H_
+
+#include 
+#include 
+
+#include 
+#include 
+
+namespace tvm {
+
+class IRModule;
+
+// Forward class for PassInstrumentNode methods
+namespace transform {
+class PassInfo;
+}  // namespace transform
+
+namespace instrument {
+
+/*!
+ * \brief PassInstrumentNode forms an instrument implementation.
+ * It provides API for users to register callbacks at different 
instrumentation points.
+ *
+ * Within a pass context (tvm::transfom::PassContext), the instrumentation 
call sequence will like:
+ *
+ *   Instrument SetUp()
+ *
+ * if (Instrument Before Pass())
+ *   Pass1()
+ *   Instrument After Pass()
+ *
+ * if (Instrument Before Pass())
+ *   Pass2()
+ *   Instrument After Pass()
+ *
+ *   Instrument TearDown()
+ *
+ *
+ * The `Before Pass` instrumentation point can selectively disable passes by 
returning true (to
+ * enable) or false (to disable).
+ *
+ * If there are multiple pass instrumentations provided, `Before Pass` 
callbacks are applied in
+ * order. If one return false, then the pass will be skipped:
+ *
+ *for (auto pi : PassInstruments)
+ *   if (pi->BeforePass())
+ *  return False  // Disable pass
+ *
+ *return True  // All ok, enable pass
+ *
+ *
+ * \sa PassInstrument
+ * \sa PassContextNode::InstrumentBeforePass()
+ * \sa src/ir/transform.cc
+ */
+class PassInstrumentNode : public Object {
+ public:
+  virtual ~PassInstrumentNode() {}
+
+  /*! \brief Set up environment for instrumentation. */

Review comment:
   Done

##
File path: include/tvm/ir/instrument.h
##
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/ir/instrument.h
+ *
+ * This file introduces a pass instrument infrastructure, inspired by LLVM and 
MLIR.
+ * It inserts instrumentation points around passes.
+ */
+#ifndef TVM_IR_INSTRUMENT_H_
+#define TVM_IR_INSTRUMENT_H_
+
+#include 
+#include 
+
+#include 
+#include 
+
+namespace tvm {
+
+class IRModule;
+
+// Forward class for PassInstrumentNode methods
+namespace transform {
+class PassInfo;
+}  // namespace transform
+
+namespace instrument {
+
+/*!
+ * \brief PassInstrumentNode forms an instrument implementation.
+ * It provides API for users to register callbacks at different 
instrumentation points.
+ *
+ * Within a pass context (tvm::transfom::PassContext), the instrumentation 
call sequence will like:
+ *
+ *   Instrument SetUp()
+ *
+ * if (Instrument Before Pass())
+ *   Pass1()
+ *   Instrument After Pass()
+ *
+ * if (Instrument Before Pass())
+ *   Pass2()
+ *   Instrument After Pass()
+ *
+ *   Instrument TearDown()
+ *
+ *
+ * The `Before Pass` instrumentation point can selectively disable passes by 
returning true (to
+ * enable) or false (to disable).
+ *
+ * If there are multiple pass instrumentations provided, `Before Pass` 
callbacks are applied in
+ * order. If one return false, then the pass will be skipped:
+ *
+