You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/05/19 16:49:24 UTC
[GitHub] [tvm] areusch commented on a change in pull request #7952: [IR][Pass][Instrument] Pass instrument framework
areusch commented on a change in pull request #7952:
URL: https://github.com/apache/tvm/pull/7952#discussion_r635408096
##########
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 <tvm/node/reflection.h>
+#include <tvm/runtime/container.h>
+
+#include <utility>
+#include <vector>
+
+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. */
+ virtual void SetUp() const = 0;
+
+ /*! \brief Clean up instrumentation environment. */
Review comment:
also specify here this runs once after all passes run. also, specify what will happen in these cases:
1. when SetUp has been called but a following SetUp (e.g. on another instrument) raises an exception
2. when an exception is raised in the course of running passes
it would also be great to add unit tests for these cases
##########
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:
why do we need the metadata logic? can't we just pass a PackedFunc to the C++ FFI that binds to e.g. `self.run_before_pass`? Python captures `self` in that function pointer I believe.
##########
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
Review comment:
nit: remove fg
##########
File path: include/tvm/ir/transform.h
##########
@@ -95,8 +87,9 @@ class PassContextNode : public Object {
mutable Optional<DiagnosticContext> diag_ctx;
/*! \brief Pass specific configurations. */
Map<String, ObjectRef> config;
- /*! \brief Trace function to be invoked before and after each pass. */
- TraceFunc trace_func;
+
+ /*! \brief A list of pass instrument implementations. */
Review comment:
specify the order
##########
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 <tvm/node/reflection.h>
+#include <tvm/runtime/container.h>
+
+#include <utility>
+#include <vector>
+
+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:
i think it would be great to separate the before-pass setup from the decision of "should this pass run?" The reason is that this removes concerns about how the ordering of pass instrumentation may change the number of calls to a given instrument. For example, if you just want to log all of the passes executed, you need to make sure that it's the first pass instrument registered.
I'd suggest this:
```
should_run_pass = [pi.should_run(pass) for pi in pass_instruments]
if all(should_run_pass):
for pi in pass_instruments:
pi.before_pass(pass)
pass()
for pi in pass_instruments:
pi.after_pass(pass)
```
additionally, it would be great to document what happens if any of the non-pass logic raises an error here. the Python context manager framework should provide some good inspiration for this. it seems like it may be helpful to provide the after_pass with a "result" notion. also, can you add tests for these cases?
##########
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 <dmlc/thread_local.h>
+#include <tvm/ir/instrument.h>
+#include <tvm/ir/transform.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/registry.h>
+
+#include <stack>
+
+namespace tvm {
+namespace instrument {
+
+/*!
+ * \brief A named PassInstrument implementation
Review comment:
State the meaning of "named" here: "A PassInstrument implementation which also carries a human-readable name"
Why don't all PassInstruments have names? Seems impossible to refer to them in the output otherwise.
##########
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 <tvm/node/reflection.h>
+#include <tvm/runtime/container.h>
+
+#include <utility>
+#include <vector>
+
+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:
also specify here that this runs once before all passes run
##########
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:
due to an issue with how pytest interacts with Node classes, it's preferable that this is the first thing in `__init__`. if anything between line 47 and here raises an exception, pytest will cause an infinite recursion rather than printing the actual error. the constraint is that `self.handle` needs to be initialized.
--
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:
users@infra.apache.org