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 2020/12/22 23:32:30 UTC
[GitHub] [tvm] tkonolige opened a new pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
tkonolige opened a new pull request #7153:
URL: https://github.com/apache/tvm/pull/7153
- Added libbacktrace to 3rdparty
- Changed build settings to give absolute paths in debug symbols
- Move CHECK and LOG to tvm/support/logging.h
- Rename tvm::Error to tvm::CompileError
- Create new tvm::Error that contains structured information including a backtrace.
- Replace dmlc::Error with new tvm::Error.
- Unify dmlc headers to `include/tvm/support/dmlc.h` and hide LOG and CHECK macros from dmlc.
- Rename ICHECK, CHECK, and LOG to TVM_ICHECK, TVM_CHECK, TVM_LOG.
This branch is not up to date with main because the changes to check hit a lot of places in the codebase. I'll update it when we are in agreement to merge it.
RFC: https://discuss.tvm.apache.org/t/rfc-line-numbers-in-backtraces/8711
@areusch @junrushao1994 @u99127
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-766973012
Right now I've put the Error types in the tvm::runtime namespace and then re-exported them from the tvm namespace. Should I just put them in the tvm namespace instead?
----------------------------------------------------------------
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
[GitHub] [tvm] antinucleon commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
antinucleon commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r548161528
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace/configure" "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
Review comment:
This is unacceptable CMakefile.
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-766973012
Right now I've put the Error types in the tvm::runtime namespace and then re-exported them from the tvm namespace. Should I just put them in the tvm namespace instead?
----------------------------------------------------------------
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
[GitHub] [tvm] tqchen commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r552999228
##########
File path: src/tir/transforms/lower_thread_allreduce.cc
##########
@@ -509,7 +509,6 @@ class ThreadAllreduceBuilder final : public StmtExprMutator {
}
const AttrStmtNode* op = thread_extents_.back();
- DCHECK_EQ(op->attr_key, attr::thread_extent);
Review comment:
This seems to be un-intentional
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-784458728
@areusch @junrushao1994 @antinucleon @tqchen I think this PR is ready for a re-review. All previous issues should be fixed.
CI is failing because I'm hitting a segfault in the frontend tests. I think its a stack overflow, but I'm not sure. The segfault occurs within tvm::relay::ExprFunctor.
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r573256281
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,54 @@
+# 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.
+include(ExternalProject)
+
+# Git tag for libbacktrace
+set(TAG dedbe13fda00253fe5d4f2fb812c909729ed5937)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ GIT_REPOSITORY https://github.com/ianlancetaylor/libbacktrace.git
+ GIT_TAG "${TAG}"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/src/project_libbacktrace/configure"
+ "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace" --with-pic
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
+ BUILD_BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/lib/libbacktrace.a"
+ "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include/backtrace.h"
+ # disable the builtin update because it rebuilds on every build
+ UPDATE_COMMAND ""
+ )
+
+# Only rebuild libbacktrace if this file changes
+# libbacktrace has a bug on macOS with shared libraries, so we patch it here
+ExternalProject_Add_Step(project_libbacktrace checkout
Review comment:
I can't use the building patch command because I've disabled the default update rule.
----------------------------------------------------------------
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
[GitHub] [tvm] tqchen commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r563164652
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,380 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
+
+#ifndef TVM_BACKTRACE_DISABLED
+/* \brief Generate a backtrace when called. */
+std::string Backtrace();
Review comment:
document the return value
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,380 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
Review comment:
the function needs to be under runtime namespace
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -485,7 +486,7 @@ int TVMFuncCreateFromCFunc(TVMPackedCFunc func, void* resource_handle, TVMPacked
int ret = func(const_cast<TVMValue*>(args.values), const_cast<int*>(args.type_codes),
args.num_args, rv, rpack.get());
if (ret != 0) {
- throw dmlc::Error(TVMGetLastError() + ::dmlc::StackTrace());
+ throw tvm::Error(TVMGetLastError() + tvm::Backtrace());
Review comment:
if it is in runtime namespace, the file should be in runtime namespace, then exposed to the tvm namespace
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,380 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
+
+#ifndef TVM_BACKTRACE_DISABLED
+/* \brief Generate a backtrace when called. */
+std::string Backtrace();
+#else
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+inline std::string Backtrace() { return ""; }
+#endif
+
+/*! \brief Base error type for TVM. Wraps a string message. */
+class Error : public std::runtime_error {
+ public:
+ explicit Error(const std::string& s) : std::runtime_error(s) {}
+};
+
+/*! \brief Error type for errors from CHECK, ICHECK, and LOG(FATAL). This error
+ * contains a backtrace of where it occured.
+ */
+class InternalError : public Error {
+ public:
+ /*! \brief Construct an error. Not recommended to use directly. Instead use LOG(FATAL).
+ *
+ * \param file The file where the error occurred.
+ * \param lineno The line number where the error occurred.
+ * \param message The error message to display.
+ * \param time The time at which the error occurred.
+ * \param backtrace Backtrace from when the error occurred.
+ */
+ InternalError(const std::string& file, int lineno, const std::string& message,
+ const std::time_t& time = std::time(nullptr), std::string backtrace = Backtrace())
+ : Error(""),
+ file_(file),
+ lineno_(lineno),
+ message_(message),
+ time_(time),
+ backtrace_(backtrace) {
+ std::ostringstream s;
+ s << "[" << std::put_time(std::localtime(&time), "%H:%M:%S") << "] " << file << ":" << lineno
+ << ": " << std::endl
+ << backtrace << std::endl
+ << message << std::endl;
+ full_message_ = s.str();
+ }
+ /*! \brief The file in which the error occurred. */
Review comment:
brief -> return
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,380 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
+
+#ifndef TVM_BACKTRACE_DISABLED
+/* \brief Generate a backtrace when called. */
+std::string Backtrace();
+#else
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+inline std::string Backtrace() { return ""; }
+#endif
+
+/*! \brief Base error type for TVM. Wraps a string message. */
+class Error : public std::runtime_error {
+ public:
+ explicit Error(const std::string& s) : std::runtime_error(s) {}
+};
+
+/*! \brief Error type for errors from CHECK, ICHECK, and LOG(FATAL). This error
+ * contains a backtrace of where it occured.
+ */
+class InternalError : public Error {
+ public:
+ /*! \brief Construct an error. Not recommended to use directly. Instead use LOG(FATAL).
+ *
+ * \param file The file where the error occurred.
+ * \param lineno The line number where the error occurred.
+ * \param message The error message to display.
+ * \param time The time at which the error occurred.
+ * \param backtrace Backtrace from when the error occurred.
+ */
+ InternalError(const std::string& file, int lineno, const std::string& message,
+ const std::time_t& time = std::time(nullptr), std::string backtrace = Backtrace())
+ : Error(""),
+ file_(file),
+ lineno_(lineno),
+ message_(message),
+ time_(time),
+ backtrace_(backtrace) {
+ std::ostringstream s;
+ s << "[" << std::put_time(std::localtime(&time), "%H:%M:%S") << "] " << file << ":" << lineno
+ << ": " << std::endl
+ << backtrace << std::endl
+ << message << std::endl;
+ full_message_ = s.str();
+ }
+ /*! \brief The file in which the error occurred. */
+ const std::string& file() const { return file_; }
+ /*! \brief The message associated with this error. */
+ const std::string& message() const { return message_; }
+ /*! \brief Formatted error message including file, linenumber, backtrace, and message. */
+ const std::string& full_message() const { return full_message_; }
+ /*! \brief The backtrace from where this error occurred. */
+ const std::string& backtrace() const { return backtrace_; }
+ /*! \brief The time at which this error occurred. */
+ const std::time_t& time() const { return time_; }
+ /*! \brief The line number at which this error occurred. */
+ int lineno() const { return lineno_; }
+ virtual const char* what() const noexcept { return full_message_.c_str(); }
+
+ private:
+ std::string file_;
+ int lineno_;
+ std::string message_;
+ std::time_t time_;
+ std::string backtrace_;
+ std::string full_message_; // holds the full error string
+};
+
+/*! \brief Class to accumulate an error message and throw it. Do not use
+ * directly, instead use LOG(FATAL).
+ */
+class LogFatal {
Review comment:
consider move these classes to runtime::detail
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,380 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
+
+#ifndef TVM_BACKTRACE_DISABLED
+/* \brief Generate a backtrace when called. */
+std::string Backtrace();
+#else
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+inline std::string Backtrace() { return ""; }
+#endif
+
+/*! \brief Base error type for TVM. Wraps a string message. */
+class Error : public std::runtime_error {
+ public:
+ explicit Error(const std::string& s) : std::runtime_error(s) {}
Review comment:
document the constructor
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,380 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
+
+#ifndef TVM_BACKTRACE_DISABLED
+/* \brief Generate a backtrace when called. */
+std::string Backtrace();
+#else
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+inline std::string Backtrace() { return ""; }
+#endif
+
+/*! \brief Base error type for TVM. Wraps a string message. */
+class Error : public std::runtime_error {
+ public:
+ explicit Error(const std::string& s) : std::runtime_error(s) {}
+};
+
+/*! \brief Error type for errors from CHECK, ICHECK, and LOG(FATAL). This error
+ * contains a backtrace of where it occured.
+ */
+class InternalError : public Error {
+ public:
+ /*! \brief Construct an error. Not recommended to use directly. Instead use LOG(FATAL).
+ *
+ * \param file The file where the error occurred.
+ * \param lineno The line number where the error occurred.
+ * \param message The error message to display.
+ * \param time The time at which the error occurred.
+ * \param backtrace Backtrace from when the error occurred.
+ */
+ InternalError(const std::string& file, int lineno, const std::string& message,
Review comment:
pass message and file by value(instead of by const ref) to enable possible move optimization.
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-749842135
Thank you Tristan for the hard work! Would love to ask some questions just for clarification on the forum first :-)
----------------------------------------------------------------
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
[GitHub] [tvm] antinucleon removed a comment on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
antinucleon removed a comment on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-750442179
I request change this PR until it is able to officially supported by Window. libbacktrace is not supported on Windows, and this building rule will break Windows building. Please specialize for Windows.
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r595371691
##########
File path: src/runtime/logging.cc
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.
+ */
+
+#ifdef TVM_BACKTRACE_DISABLED
+#include <string>
+
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+namespace tvm {
+namespace runtime {
+__declspec(dllexport) std::string Backtrace() { return ""; }
Review comment:
got it. Thanks for the clarification!
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572264278
##########
File path: apps/android_camera/app/src/main/jni/tvm_runtime.h
##########
@@ -25,17 +25,11 @@
#include <fstream>
-/* Enable custom logging - this will cause TVM to pass every log message
- * through CustomLogMessage instead of LogMessage. By enabling this, we must
- * implement dmlc::CustomLogMessage::Log. We use this to pass TVM log
- * messages to Android logcat.
+/* Enable custom logging - this will cause TVM to use a custom implementation
+ * of tvm::runtime::detail::LogMessage. We use this to pass TVM log messages to
+ * Android logcat.
*/
-#define DMLC_LOG_CUSTOMIZE 1
-
-/* Ensure that fatal errors are passed to the logger before throwing
- * in LogMessageFatal
- */
-#define DMLC_LOG_BEFORE_THROW 1
+#define TVM_LOG_CUSTOMIZE 1
Review comment:
CC @yzhliu for the android part
##########
File path: CMakeLists.txt
##########
@@ -87,6 +92,7 @@ include_directories(${CMAKE_INCLUDE_PATH})
include_directories("include")
include_directories(SYSTEM ${DLPACK_PATH})
include_directories(SYSTEM ${DMLC_PATH})
+add_definitions(-DDMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
Review comment:
generally should we avoid global flags in cmake?
##########
File path: CMakeLists.txt
##########
@@ -379,6 +387,22 @@ set_property(TARGET tvm APPEND PROPERTY LINK_OPTIONS "${TVM_VISIBILITY_FLAG}")
add_library(tvm_runtime SHARED $<TARGET_OBJECTS:tvm_runtime_objs>)
set_property(TARGET tvm_runtime APPEND PROPERTY LINK_OPTIONS "${TVM_VISIBILITY_FLAG}")
+if(USE_LIBBACKTRACE)
+ message(STATUS "Building with libbacktrace...")
+ include(cmake/modules/Libbacktrace.cmake)
+ target_link_libraries(tvm PRIVATE libbacktrace)
+ target_link_libraries(tvm_runtime PRIVATE libbacktrace)
+ add_dependencies(tvm_objs libbacktrace)
+ add_dependencies(tvm_runtime_objs libbacktrace)
+ # pre 3.12 versions of cmake cannot propagate include directories from imported targets so we set them manually
+ target_include_directories(tvm PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include")
+ target_include_directories(tvm_objs PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include")
+ target_include_directories(tvm_runtime PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include")
+ target_include_directories(tvm_runtime_objs PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include")
+else()
+ add_definitions(-DTVM_BACKTRACE_DISABLED)
Review comment:
ditto
##########
File path: include/tvm/ir/diagnostic.h
##########
@@ -37,6 +37,15 @@ namespace tvm {
using tvm::parser::SourceMap;
using tvm::runtime::TypedPackedFunc;
+/*! \brief The diagnostic level, controls the printing of the message. */
+enum class DiagnosticLevel : int {
+ kBug = 10,
Review comment:
+1
##########
File path: CMakeLists.txt
##########
@@ -526,3 +550,25 @@ if(MSVC)
target_compile_definitions(tvm_objs PRIVATE -DTVM_EXPORTS)
target_compile_definitions(tvm_runtime_objs PRIVATE -DTVM_EXPORTS)
endif()
+
+set(TVM_IS_DEBUG_BUILD OFF)
+if(CMAKE_BUILD_TYPE STREQUAL "Debug" OR CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo" OR CMAKE_CXX_FLAGS MATCHES "-g")
+ set(TVM_IS_DEBUG_BUILD ON)
+endif()
+
+# Change relative paths in backtrace to absolute ones
+if(TVM_IS_DEBUG_BUILD)
+ target_compile_options(tvm PRIVATE "-ffile-prefix-map=..=${CMAKE_CURRENT_SOURCE_DIR}")
Review comment:
Is this GCC specific flag? If so, we probably should check compiler id
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,54 @@
+# 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.
+include(ExternalProject)
+
+# Git tag for libbacktrace
+set(TAG dedbe13fda00253fe5d4f2fb812c909729ed5937)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ GIT_REPOSITORY https://github.com/ianlancetaylor/libbacktrace.git
+ GIT_TAG "${TAG}"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/src/project_libbacktrace/configure"
+ "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace" --with-pic
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
+ BUILD_BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/lib/libbacktrace.a"
+ "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include/backtrace.h"
+ # disable the builtin update because it rebuilds on every build
+ UPDATE_COMMAND ""
+ )
+
+# Only rebuild libbacktrace if this file changes
+# libbacktrace has a bug on macOS with shared libraries, so we patch it here
+ExternalProject_Add_Step(project_libbacktrace checkout
Review comment:
An alternative to ExternalProject is that we can actually add libbacktrace as a submodule in `3rdparty/`, and we patch on our own fork
##########
File path: tests/scripts/task_build.sh
##########
@@ -16,4 +16,4 @@
# specific language governing permissions and limitations
# under the License.
export VTA_HW_PATH=`pwd`/3rdparty/vta-hw
-cd $1 && cmake .. && make $2 && cd ..
+cd $1 && cmake .. -DCMAKE_BUILD_TYPE=Debug && make $2 && cd ..
Review comment:
Is this change intentional? Perhaps it makes sense to test with `RelWithDebInfo`?
##########
File path: tutorials/auto_scheduler/tune_network_mali.py
##########
@@ -329,7 +329,7 @@ def tune_and_evaluate():
# The last line also prints the total number of measurement trials,
# total time spent on auto-tuning and the id of the next task to tune.
#
-# There will also be some "dmlc::Error"s errors, because the
+# There will also be some "tvn::Error"s errors, because the
Review comment:
oops
```suggestion
# There will also be some "tvm::Error"s errors, because the
```
##########
File path: tutorials/auto_scheduler/tune_network_x86.py
##########
@@ -251,7 +251,7 @@ def run_tuning():
# The last line also prints the total number of measurement trials,
# total time spent on auto-tuning and the id of the next task to tune.
#
-# There will also be some "dmlc::Error"s errors, because the
+# There will also be some "tvn::Error"s errors, because the
Review comment:
ditto
```suggestion
# There will also be some "tvm::Error"s errors, because the
```
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572346090
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,54 @@
+# 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.
+include(ExternalProject)
+
+# Git tag for libbacktrace
+set(TAG dedbe13fda00253fe5d4f2fb812c909729ed5937)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ GIT_REPOSITORY https://github.com/ianlancetaylor/libbacktrace.git
+ GIT_TAG "${TAG}"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/src/project_libbacktrace/configure"
+ "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace" --with-pic
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
+ BUILD_BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/lib/libbacktrace.a"
+ "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include/backtrace.h"
+ # disable the builtin update because it rebuilds on every build
+ UPDATE_COMMAND ""
+ )
+
+# Only rebuild libbacktrace if this file changes
+# libbacktrace has a bug on macOS with shared libraries, so we patch it here
+ExternalProject_Add_Step(project_libbacktrace checkout
Review comment:
I don’t have strong opinion. Would love to hear about options from Bob :-)
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r581354229
##########
File path: include/tvm/ir/diagnostic.h
##########
@@ -37,6 +37,15 @@ namespace tvm {
using tvm::parser::SourceMap;
using tvm::runtime::TypedPackedFunc;
+/*! \brief The diagnostic level, controls the printing of the message. */
+enum class DiagnosticLevel : int {
+ kBug = 10,
Review comment:
There isn't a translation to log messages here. `DiagnosticLevel` is used within relay and TIR to determine how to print the message (these messages are never hidden or filtered).
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572269081
##########
File path: tutorials/auto_scheduler/tune_network_x86.py
##########
@@ -251,7 +251,7 @@ def run_tuning():
# The last line also prints the total number of measurement trials,
# total time spent on auto-tuning and the id of the next task to tune.
#
-# There will also be some "dmlc::Error"s errors, because the
+# There will also be some "tvn::Error"s errors, because the
Review comment:
tvn here and in tune_network_mali
----------------------------------------------------------------
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
[GitHub] [tvm] antinucleon commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
antinucleon commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r548162253
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace/configure" "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
Review comment:
Question: How does cmake generate for Windows with this command?
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572298041
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,435 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <dmlc/common.h>
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
+namespace runtime {
+
+#ifndef TVM_BACKTRACE_DISABLED
+/* \brief Generate a backtrace when called.
+ * \return A multiline string of the backtrace. There will be either one or two lines per frame.
+ */
+std::string Backtrace();
+#else
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+inline std::string Backtrace() { return ""; }
+#endif
+
+/*! \brief Base error type for TVM. Wraps a string message. */
+class Error : public ::dmlc::Error { // for backwards compatibility
+ public:
+ /*! \brief Construct an error.
+ * \param s The message to be displayed with the error.
+ */
+ explicit Error(const std::string& s) : ::dmlc::Error(s) {}
+};
+
+/*! \brief Error type for errors from CHECK, ICHECK, and LOG(FATAL). This error
+ * contains a backtrace of where it occured.
+ */
+class InternalError : public Error {
+ public:
+ /*! \brief Construct an error. Not recommended to use directly. Instead use LOG(FATAL).
+ *
+ * \param file The file where the error occurred.
+ * \param lineno The line number where the error occurred.
+ * \param message The error message to display.
+ * \param time The time at which the error occurred.
+ * \param backtrace Backtrace from when the error occurred.
+ */
+ InternalError(std::string file, int lineno, std::string message,
+ std::time_t time = std::time(nullptr), std::string backtrace = Backtrace())
+ : Error(""),
+ file_(file),
+ lineno_(lineno),
+ message_(message),
+ time_(time),
+ backtrace_(backtrace) {
+ std::ostringstream s;
+ // XXX: Do not change this format, otherwise all error handling in python will break (because it
Review comment:
`XXX` = dangerous code
----------------------------------------------------------------
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
[GitHub] [tvm] tqchen commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r548206504
##########
File path: include/tvm/ir/attrs.h
##########
@@ -393,7 +392,7 @@ struct AttrInitEntry {
value_missing_ = false;
return *this;
}
- TSelf& describe(DMLC_ATTRIBUTE_UNUSED const char* str) { return *this; }
+ TSelf& describe(__attribute__((unused)) const char* str) { return *this; }
Review comment:
Thanks @tkonolige for the intitial POC. Given the amount of refactoring needed, which we did not anticipate in the beginning. Let us discuss a bit more about possible alternatives a good resolution before proceeding with this PR.
A generic marco is needed in most of these cases for cross platform reasons(MSVC support
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572322545
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,54 @@
+# 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.
+include(ExternalProject)
+
+# Git tag for libbacktrace
+set(TAG dedbe13fda00253fe5d4f2fb812c909729ed5937)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ GIT_REPOSITORY https://github.com/ianlancetaylor/libbacktrace.git
+ GIT_TAG "${TAG}"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/src/project_libbacktrace/configure"
+ "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace" --with-pic
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
+ BUILD_BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/lib/libbacktrace.a"
+ "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include/backtrace.h"
+ # disable the builtin update because it rebuilds on every build
+ UPDATE_COMMAND ""
+ )
+
+# Only rebuild libbacktrace if this file changes
+# libbacktrace has a bug on macOS with shared libraries, so we patch it here
+ExternalProject_Add_Step(project_libbacktrace checkout
Review comment:
I believe @rkimball has an opinion on this.
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-801672549
I built and tested locally the backtrace using some existing testcases, and here is the output:
```
â–¶ python tests/python/unittest/test_target_target.py
Traceback (most recent call last):
File "tests/python/unittest/test_target_target.py", line 229, in <module>
test_target_host_warning()
File "tests/python/unittest/test_target_target.py", line 218, in test_target_host_warning
tgt = tvm.target.Target("cuda --host nvidia/jetson-nano", "llvm")
File "/home/jrshao/Projects/tvm-dev/python/tvm/target/target.py", line 100, in __init__
self.__init_handle_by_constructor__(
File "/home/jrshao/Projects/tvm-dev/python/tvm/_ffi/_ctypes/object.py", line 136, in __init_handle_by_constructor__
handle = __init_by_constructor__(fconstructor, args)
File "/home/jrshao/Projects/tvm-dev/python/tvm/_ffi/_ctypes/packed_func.py", line 260, in __init_handle_by_constructor__
raise get_last_ffi_error()
ValueError: Traceback (most recent call last):
7: TVMFuncCall
at /home/jrshao/Projects/tvm-dev/src/runtime/c_runtime_api.cc:481
6: tvm::runtime::PackedFunc::CallPacked(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const
at /home/jrshao/Projects/tvm-dev/include/tvm/runtime/packed_func.h:1158
5: std::function<void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::operator()(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const
at /usr/include/c++/10/bits/std_function.h:622
4: std::_Function_handler<void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), void (*)(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)>::_M_invoke(std::_Any_data const&, tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&)
at /usr/include/c++/10/bits/std_function.h:291
3: std::enable_if<std::__and_<std::is_void<void>, std::__is_invocable<void (*&)(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*> >::value, void>::type std::__invoke_r<void, void (*&)(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*>(void (*&)(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&)
at /usr/include/c++/10/bits/invoke.h:153
2: void std::__invoke_impl<void, void (*&)(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*>(std::__invoke_other, void (*&)(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&)
at /usr/include/c++/10/bits/invoke.h:60
1: tvm::TargetInternal::ConstructorDispatcher(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)
at /home/jrshao/Projects/tvm-dev/src/target/target.cc:471
0: tvm::Target::Target(tvm::Target, tvm::Target)
at /home/jrshao/Projects/tvm-dev/src/target/target.cc:377
File "/home/jrshao/Projects/tvm-dev/src/target/target.cc", line 377
ValueError: Check failed: !n->host.defined() == false: Adding a host to a target whose host field has been defined
```
Have to say it is really neat! Thank you so much Tristan!
CC: @comaniac @zhiics @yzhliu @icemelon9 @kevinthesun
----------------------------------------------------------------
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
[GitHub] [tvm] apivovarov commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
apivovarov commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-810663046
@tkonolige Looks like libbacktrace build does not look at `CMAKE_CXX_COMPILER` and `CMAKE_C_COMPILER` flags.
Regardless of their values it builds libbacktrace for host platform (x86_64)
```
$ cmake .. \
-DCMAKE_C_COMPILER=mips-linux-uclibc-gnu-gcc \
-DCMAKE_CXX_COMPILER=mips-linux-uclibc-gnu-g++ \
-DCMAKE_C_FLAGS="-march=mips32r2 -DDMLC_LOG_STACK_TRACE=0" \
-DCMAKE_CXX_FLAGS="-march=mips32r2 -D_GLIBCXX_USE_C99 -DDMLC_LOG_STACK_TRACE=0"
-- The C compiler identification is GNU 5.4.0
-- The CXX compiler identification is GNU 5.4.0
-- Check for working C compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-gcc
-- Check for working C compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++
-- Check for working CXX compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++ -- works
...
[ 7%] Performing configure step for 'project_libbacktrace'
cd /home/alex/workspace/tvm/build-wyze/libbacktrace && /home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace/configure --prefix=/home/alex/workspace/tvm/build-wyze/libbacktrace --with-pic
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking target system type... x86_64-pc-linux-gnu
...
/bin/bash ./libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I/home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace -funwind-tables -frandom-seed=atomic.lo -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wcast-qual -g -O2 -c -o atomic.lo /home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace/atomic.c
...
[ 69%] Building CXX object CMakeFiles/tvm_runtime_objs.dir/src/runtime/graph/graph_runtime.cc.o
/home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++ -DDMLC_USE_FOPEN64=0 -DDMLC_USE_LOGGING_LIBRARY="<tvm/runtime/logging.h>" -DNDEBUG -DNDEBUG=1 -DTVM_INDEX_DEFAULT_I64=1 -DTVM_THREADPOOL_USE_OPENMP=0 -DTVM_USE_LIBBACKTRACE=1 -DUSE_FALLBACK_STL_MAP=0 -DTVM_GRAPH_RUNTIME_DEBUG -I/home/alex/workspace/tvm/include -isystem /home/alex/workspace/tvm/3rdparty/dlpack/include -isystem /home/alex/workspace/tvm/3rdparty/dmlc-core/include -isystem /home/alex/workspace/tvm/3rdparty/rang/include -isystem /home/alex/workspace/tvm/3rdparty/compiler-rt -isystem /home/alex/workspace/tvm/3rdparty/picojson -I/home/alex/workspace/tvm/build-wyze/libbacktrace/include -I/home/alex/workspace/tvm/3rdparty/libcrc/include -std=c++14 -O2 -Wall -fPIC -march=mips32r2 -D_GLIBCXX_USE_C99 -DDMLC_LOG_STACK_TRACE=0 -o CMakeFiles/tvm_runtime_objs.dir/src/runtime/graph/graph_runtime.cc.o -c /home/alex/workspace/tvm/src/runtime/graph/graph_runtime.cc
...
/home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/../lib/gcc/mips-linux-gnu/5.4.0/../../../../mips-linux-gnu/bin/ld: libbacktrace/lib/libbacktrace.a(fileline.o): Relocations in generic ELF (EM: 62)
libbacktrace/lib/libbacktrace.a: error adding symbols: File in wrong format
```
--
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
[GitHub] [tvm] junrushao1994 commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-810673710
@apivovarov I believe Tristan is working on this: https://github.com/apache/tvm/issues/7705#issuecomment-805048911
--
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
[GitHub] [tvm] tqchen commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-801912831
Thanks @tkonolige @rkimball @junrushao1994 @areusch @antinucleon . This PR is now merged
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-800440041
It is super cool work that has been missing for years. It looks really nice and I don't have other concerns besides the ExternalProject approach, where the internet is required on the first build, which I (being nitpicky) think could cause some issues if the build environment doesn't have network connection. A better approach might be creating a repo under tlc-pack/octoml/etc and submodule it, like what synr does.
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r581345367
##########
File path: include/tvm/ir/diagnostic.h
##########
@@ -37,6 +37,15 @@ namespace tvm {
using tvm::parser::SourceMap;
using tvm::runtime::TypedPackedFunc;
+/*! \brief The diagnostic level, controls the printing of the message. */
+enum class DiagnosticLevel : int {
+ kBug = 10,
Review comment:
So you want `LOG(BUG)`, `LOG(ERROR)`, `LOG(WARNING)`, `LOG(NOTE)`, `LOG(HELP)`? Our current levels are `INFO`, `WARNING`, `ERROR`, `FATAL`. We cause alias `INFO` to `NOTE` and then add `BUG` and `HELP`?
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572322697
##########
File path: CMakeLists.txt
##########
@@ -526,3 +550,25 @@ if(MSVC)
target_compile_definitions(tvm_objs PRIVATE -DTVM_EXPORTS)
target_compile_definitions(tvm_runtime_objs PRIVATE -DTVM_EXPORTS)
endif()
+
+set(TVM_IS_DEBUG_BUILD OFF)
+if(CMAKE_BUILD_TYPE STREQUAL "Debug" OR CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo" OR CMAKE_CXX_FLAGS MATCHES "-g")
+ set(TVM_IS_DEBUG_BUILD ON)
+endif()
+
+# Change relative paths in backtrace to absolute ones
+if(TVM_IS_DEBUG_BUILD)
+ target_compile_options(tvm PRIVATE "-ffile-prefix-map=..=${CMAKE_CURRENT_SOURCE_DIR}")
Review comment:
I've added a check to see if the flag is supported
----------------------------------------------------------------
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
[GitHub] [tvm] tqchen edited a comment on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-794190828
cc @zhiics @yzhliu @leandron @hzfan @jcf94 would be great if you can also help to review 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:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r595356485
##########
File path: src/runtime/logging.cc
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.
+ */
+
+#ifdef TVM_BACKTRACE_DISABLED
+#include <string>
+
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+namespace tvm {
+namespace runtime {
+__declspec(dllexport) std::string Backtrace() { return ""; }
Review comment:
Do we want to use `TVM_DLL` instead? I am not sure
##########
File path: src/runtime/logging.cc
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.
+ */
+
+#ifdef TVM_BACKTRACE_DISABLED
+#include <string>
+
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+namespace tvm {
+namespace runtime {
+__declspec(dllexport) std::string Backtrace() { return ""; }
+} // namespace runtime
+} // namespace tvm
+#else
+
+#include <backtrace.h>
+#include <cxxabi.h>
+#include <tvm/runtime/logging.h>
+
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+#include <vector>
+
+namespace tvm {
+namespace runtime {
+namespace {
+
+struct BacktraceInfo {
+ std::vector<std::string> lines;
+ size_t max_size;
+ std::string error_message;
+};
+
+void BacktraceCreateErrorCallback(void* data, const char* msg, int errnum) {
+ std::cerr << "Could not initialize backtrace state: " << msg << std::endl;
+}
+
+backtrace_state* BacktraceCreate() {
+ return backtrace_create_state(nullptr, 1, BacktraceCreateErrorCallback, nullptr);
+}
+
+static backtrace_state* _bt_state = BacktraceCreate();
+
+std::string DemangleName(std::string name) {
+ int status = 0;
+ size_t length = name.size();
+ std::unique_ptr<char, void (*)(void* __ptr)> demangled_name = {
+ abi::__cxa_demangle(name.c_str(), nullptr, &length, &status), &std::free};
+ if (demangled_name && status == 0 && length > 0) {
+ return demangled_name.get();
+ } else {
+ return name;
+ }
+}
+
+void BacktraceErrorCallback(void* data, const char* msg, int errnum) {
+ // do nothing
+}
+
+void BacktraceSyminfoCallback(void* data, uintptr_t pc, const char* symname, uintptr_t symval,
+ uintptr_t symsize) {
+ auto str = reinterpret_cast<std::string*>(data);
+
+ if (symname != nullptr) {
+ std::string tmp(symname, symsize);
+ *str = DemangleName(tmp.c_str());
+ } else {
+ std::ostringstream s;
+ s << "0x" << std::setfill('0') << std::setw(sizeof(uintptr_t) * 2) << std::hex << pc;
+ *str = s.str();
+ }
+}
+int BacktraceFullCallback(void* data, uintptr_t pc, const char* filename, int lineno,
Review comment:
format: add an empty line
##########
File path: CMakeLists.txt
##########
@@ -539,3 +567,33 @@ if(MSVC)
target_compile_definitions(tvm_objs PRIVATE -DTVM_EXPORTS)
target_compile_definitions(tvm_runtime_objs PRIVATE -DTVM_EXPORTS)
endif()
+
+set(TVM_IS_DEBUG_BUILD OFF)
+if(CMAKE_BUILD_TYPE STREQUAL "Debug" OR CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo" OR CMAKE_CXX_FLAGS MATCHES "-g")
+ set(TVM_IS_DEBUG_BUILD ON)
+endif()
+
+# Change relative paths in backtrace to absolute ones
+if(TVM_IS_DEBUG_BUILD)
+ set(FILE_PREFIX_MAP_FLAG "-ffile-prefix-map=..=${CMAKE_CURRENT_SOURCE_DIR}")
+ target_compile_options(tvm PRIVATE "${FILE_PREFIX_MAP_FLAG}")
+ CHECK_CXX_COMPILER_FLAG("${FILE_PREFIX_MAP_FLAG}" FILE_PREFIX_MAP_SUPPORTED)
+ if(FILE_PREFIX_MAP_SUPPORTED)
+ target_compile_options(tvm PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${FILE_PREFIX_MAP_FLAG}>)
+ target_compile_options(tvm_objs PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${FILE_PREFIX_MAP_FLAG}>)
+ target_compile_options(tvm_runtime PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${FILE_PREFIX_MAP_FLAG}>)
+ target_compile_options(tvm_runtime_objs PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${FILE_PREFIX_MAP_FLAG}>)
+ endif()
+endif()
+
+# Run dsymutil to generate debugging symbols for backtraces
+if(APPLE AND TVM_IS_DEBUG_BUILD)
+ find_program(DSYMUTIL dsymutil)
+ mark_as_advanced(DSYMUTIL)
+ add_custom_command(TARGET tvm
+ POST_BUILD
+ COMMAND ${DSYMUTIL} ARGS $<TARGET_FILE:tvm>
+ COMMENT "Running dsymutil"
+ VERBATIM
+ )
Review comment:
format: alignment
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r581345841
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -474,7 +474,8 @@ int TVMFuncCreateFromCFunc(TVMPackedCFunc func, void* resource_handle, TVMPacked
int ret = func(const_cast<TVMValue*>(args.values), const_cast<int*>(args.type_codes),
args.num_args, rv, resource_handle);
if (ret != 0) {
- throw dmlc::Error(TVMGetLastError() + ::dmlc::StackTrace());
+ throw tvm::Error(TVMGetLastError() +
+ tvm::runtime::Backtrace()); // TODO(tkonolige): this may double backtrace
Review comment:
can you explain more when and what is left todo?
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-801679436
I googled around and found some related cmake support for libbacktrace, like libgo: https://go.googlesource.com/gollvm/+/refs/heads/master/cmake/modules/LibbacktraceUtils.cmake
@tkonolige do you think we should switch libbacktrace to using cmake, as the tvm project is built with cmake?
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-802202635
Thanks everyone for putting the time in to review this. I know it was a big 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:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-750441687
I am not the best guy to review the cmake scripts and the cross-platform part (especially windows). I know a really expert @rkimball - would you like to take a second look if you have time? 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:
users@infra.apache.org
[GitHub] [tvm] antinucleon commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
antinucleon commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r548161528
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace/configure" "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
Review comment:
This is unacceptable.
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace/configure" "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
Review comment:
Question: How cmake generate for Windows with this command?
----------------------------------------------------------------
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
[GitHub] [tvm] tqchen commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r595365614
##########
File path: apps/android_deploy/app/src/main/jni/Application.mk
##########
@@ -27,7 +27,7 @@ include $(config)
APP_STL := c++_static
-APP_CPPFLAGS += -DDMLC_LOG_STACK_TRACE=0 -DTVM4J_ANDROID=1 -std=c++14 -Oz -frtti
+APP_CPPFLAGS += -DDMLC_USE_LOGGING_LIBRARY=\<tvm/runtime/logging.h\> -DTVM_BACKTRACE_DISABLED=1 -DTVM4J_ANDROID=1 -std=c++14 -Oz -frtti
Review comment:
consider move the def of -DDMLC_USE_LOGGING_LIBRARY=\<tvm/runtime/logging.h\> -DTVM_BACKTRACE_DISABLED=1 into the all in one file in tvm_runtime.h (very beginning)
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572269081
##########
File path: tutorials/auto_scheduler/tune_network_x86.py
##########
@@ -251,7 +251,7 @@ def run_tuning():
# The last line also prints the total number of measurement trials,
# total time spent on auto-tuning and the id of the next task to tune.
#
-# There will also be some "dmlc::Error"s errors, because the
+# There will also be some "tvn::Error"s errors, because the
Review comment:
tvn everywhere
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r596232572
##########
File path: apps/android_deploy/app/src/main/jni/Application.mk
##########
@@ -27,7 +27,7 @@ include $(config)
APP_STL := c++_static
-APP_CPPFLAGS += -DDMLC_LOG_STACK_TRACE=0 -DTVM4J_ANDROID=1 -std=c++14 -Oz -frtti
+APP_CPPFLAGS += -DDMLC_USE_LOGGING_LIBRARY=\<tvm/runtime/logging.h\> -DTVM_BACKTRACE_DISABLED=1 -DTVM4J_ANDROID=1 -std=c++14 -Oz -frtti
Review comment:
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:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r595375493
##########
File path: apps/android_deploy/app/src/main/jni/Application.mk
##########
@@ -27,7 +27,7 @@ include $(config)
APP_STL := c++_static
-APP_CPPFLAGS += -DDMLC_LOG_STACK_TRACE=0 -DTVM4J_ANDROID=1 -std=c++14 -Oz -frtti
+APP_CPPFLAGS += -DDMLC_USE_LOGGING_LIBRARY=\<tvm/runtime/logging.h\> -DTVM_BACKTRACE_DISABLED=1 -DTVM4J_ANDROID=1 -std=c++14 -Oz -frtti
Review comment:
so user build the all in one library does not have to worry too much about these flags
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572291038
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,54 @@
+# 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.
+include(ExternalProject)
+
+# Git tag for libbacktrace
+set(TAG dedbe13fda00253fe5d4f2fb812c909729ed5937)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ GIT_REPOSITORY https://github.com/ianlancetaylor/libbacktrace.git
+ GIT_TAG "${TAG}"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/src/project_libbacktrace/configure"
+ "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace" --with-pic
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
+ BUILD_BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/lib/libbacktrace.a"
+ "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include/backtrace.h"
+ # disable the builtin update because it rebuilds on every build
+ UPDATE_COMMAND ""
+ )
+
+# Only rebuild libbacktrace if this file changes
+# libbacktrace has a bug on macOS with shared libraries, so we patch it here
+ExternalProject_Add_Step(project_libbacktrace checkout
Review comment:
There are actually two sets of choices here: 1. How do we handle patching libbacktrace and 2. Where do we get the libbacktrace repo from.
For 1, I decided to include a local patching instead of a fork of the repo because I wasn't sure about how people would feel about TVM depending on a repo I control (and I wasn't sure about if apache would let us fork a repo to their GitHub account)
For 2, I think the fetching at configure time from an external repo to be easier than using a submodule. Switching branches with submodules is very awkward. Often people forget to run `git submodule update` and we get PRs that accidentally change submodule commits.
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572303952
##########
File path: tests/scripts/task_build.sh
##########
@@ -16,4 +16,4 @@
# specific language governing permissions and limitations
# under the License.
export VTA_HW_PATH=`pwd`/3rdparty/vta-hw
-cd $1 && cmake .. && make $2 && cd ..
+cd $1 && cmake .. -DCMAKE_BUILD_TYPE=Debug && make $2 && cd ..
Review comment:
Yeah i think it makes sense to have a clear backtrace in CI, but it also occurs to me that some bugs may not always appear in Debug mode, for example, mistakenly returning a reference to a stack variables. Given that we use Release mode in production, it is good that the CI aligns well.
----------------------------------------------------------------
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
[GitHub] [tvm] apivovarov edited a comment on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
apivovarov edited a comment on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-810663046
@tkonolige Looks like libbacktrace build does not look at `CMAKE_CXX_COMPILER` and `CMAKE_C_COMPILER` flags.
Regardless of their values it builds libbacktrace for host platform (x86_64)
```
$ cmake .. \
-DCMAKE_C_COMPILER=mips-linux-uclibc-gnu-gcc \
-DCMAKE_CXX_COMPILER=mips-linux-uclibc-gnu-g++ \
-DCMAKE_C_FLAGS="-march=mips32r2 -DDMLC_LOG_STACK_TRACE=0" \
-DCMAKE_CXX_FLAGS="-march=mips32r2 -D_GLIBCXX_USE_C99 -DDMLC_LOG_STACK_TRACE=0"
-- The C compiler identification is GNU 5.4.0
-- The CXX compiler identification is GNU 5.4.0
-- Check for working C compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-gcc
-- Check for working C compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++
-- Check for working CXX compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++ -- works
...
[ 7%] Performing configure step for 'project_libbacktrace'
cd /home/alex/workspace/tvm/build-wyze/libbacktrace && /home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace/configure --prefix=/home/alex/workspace/tvm/build-wyze/libbacktrace --with-pic
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking target system type... x86_64-pc-linux-gnu
...
/bin/bash ./libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I/home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace -funwind-tables -frandom-seed=atomic.lo -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wcast-qual -g -O2 -c -o atomic.lo /home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace/atomic.c
...
[ 69%] Building CXX object CMakeFiles/tvm_runtime_objs.dir/src/runtime/graph/graph_runtime.cc.o
/home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++ -DDMLC_USE_FOPEN64=0 -DDMLC_USE_LOGGING_LIBRARY="<tvm/runtime/logging.h>" -DNDEBUG -DNDEBUG=1 -DTVM_INDEX_DEFAULT_I64=1 -DTVM_THREADPOOL_USE_OPENMP=0 -DTVM_USE_LIBBACKTRACE=1 -DUSE_FALLBACK_STL_MAP=0 -DTVM_GRAPH_RUNTIME_DEBUG -I/home/alex/workspace/tvm/include -isystem /home/alex/workspace/tvm/3rdparty/dlpack/include -isystem /home/alex/workspace/tvm/3rdparty/dmlc-core/include -isystem /home/alex/workspace/tvm/3rdparty/rang/include -isystem /home/alex/workspace/tvm/3rdparty/compiler-rt -isystem /home/alex/workspace/tvm/3rdparty/picojson -I/home/alex/workspace/tvm/build-wyze/libbacktrace/include -I/home/alex/workspace/tvm/3rdparty/libcrc/include -std=c++14 -O2 -Wall -fPIC -march=mips32r2 -D_GLIBCXX_USE_C99 -DDMLC_LOG_STACK_TRACE=0 -o CMakeFiles/tvm_runtime_objs.dir/src/runtime/graph/graph_runtime.cc.o -c /home/alex/workspace/tvm/src/runtime/graph/graph_runtime.cc
...
/home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/../lib/gcc/mips-linux-gnu/5.4.0/../../../../mips-linux-gnu/bin/ld: libbacktrace/lib/libbacktrace.a(fileline.o): Relocations in generic ELF (EM: 62)
libbacktrace/lib/libbacktrace.a: error adding symbols: File in wrong format
```
At least it can be disabled `-DUSE_LIBBACKTRACE=0`...
--
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
[GitHub] [tvm] apivovarov edited a comment on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
apivovarov edited a comment on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-810663046
@tkonolige Looks like libbacktrace build does not look at `CMAKE_CXX_COMPILER` and `CMAKE_C_COMPILER` flags.
Regardless of their values it builds libbacktrace for host platform (x86_64)
```
$ cmake .. \
-DCMAKE_C_COMPILER=mips-linux-uclibc-gnu-gcc \
-DCMAKE_CXX_COMPILER=mips-linux-uclibc-gnu-g++ \
-DCMAKE_C_FLAGS="-march=mips32r2 -DDMLC_LOG_STACK_TRACE=0" \
-DCMAKE_CXX_FLAGS="-march=mips32r2 -D_GLIBCXX_USE_C99 -DDMLC_LOG_STACK_TRACE=0"
-- The C compiler identification is GNU 5.4.0
-- The CXX compiler identification is GNU 5.4.0
-- Check for working C compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-gcc
-- Check for working C compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++
-- Check for working CXX compiler: /home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++ -- works
...
[ 7%] Performing configure step for 'project_libbacktrace'
cd /home/alex/workspace/tvm/build-wyze/libbacktrace && /home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace/configure --prefix=/home/alex/workspace/tvm/build-wyze/libbacktrace --with-pic
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking target system type... x86_64-pc-linux-gnu
...
/bin/bash ./libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I/home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace -funwind-tables -frandom-seed=atomic.lo -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wcast-qual -g -O2 -c -o atomic.lo /home/alex/workspace/tvm/cmake/libs/../../3rdparty/libbacktrace/atomic.c
...
[ 69%] Building CXX object CMakeFiles/tvm_runtime_objs.dir/src/runtime/graph/graph_runtime.cc.o
/home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/mips-linux-uclibc-gnu-g++ -DDMLC_USE_FOPEN64=0 -DDMLC_USE_LOGGING_LIBRARY="<tvm/runtime/logging.h>" -DNDEBUG -DNDEBUG=1 -DTVM_INDEX_DEFAULT_I64=1 -DTVM_THREADPOOL_USE_OPENMP=0 -DTVM_USE_LIBBACKTRACE=1 -DUSE_FALLBACK_STL_MAP=0 -DTVM_GRAPH_RUNTIME_DEBUG -I/home/alex/workspace/tvm/include -isystem /home/alex/workspace/tvm/3rdparty/dlpack/include -isystem /home/alex/workspace/tvm/3rdparty/dmlc-core/include -isystem /home/alex/workspace/tvm/3rdparty/rang/include -isystem /home/alex/workspace/tvm/3rdparty/compiler-rt -isystem /home/alex/workspace/tvm/3rdparty/picojson -I/home/alex/workspace/tvm/build-wyze/libbacktrace/include -I/home/alex/workspace/tvm/3rdparty/libcrc/include -std=c++14 -O2 -Wall -fPIC -march=mips32r2 -D_GLIBCXX_USE_C99 -DDMLC_LOG_STACK_TRACE=0 -o CMakeFiles/tvm_runtime_objs.dir/src/runtime/graph/graph_runtime.cc.o -c /home/alex/workspace/tvm/src/runtime/graph/graph_runtime.cc
...
/home/alex/workspace/mips-gcc540-glibc222-64bit-r3.3.0/bin/../lib/gcc/mips-linux-gnu/5.4.0/../../../../mips-linux-gnu/bin/ld: libbacktrace/lib/libbacktrace.a(fileline.o): Relocations in generic ELF (EM: 62)
libbacktrace/lib/libbacktrace.a: error adding symbols: File in wrong format
```
At least it can be disabled `-DDMLC_LOG_STACK_TRACE=0`...
--
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572287959
##########
File path: tests/scripts/task_build.sh
##########
@@ -16,4 +16,4 @@
# specific language governing permissions and limitations
# under the License.
export VTA_HW_PATH=`pwd`/3rdparty/vta-hw
-cd $1 && cmake .. && make $2 && cd ..
+cd $1 && cmake .. -DCMAKE_BUILD_TYPE=Debug && make $2 && cd ..
Review comment:
This change is intensional. With this change, we get clearer backtraces from CI.
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r581353108
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -474,7 +474,8 @@ int TVMFuncCreateFromCFunc(TVMPackedCFunc func, void* resource_handle, TVMPacked
int ret = func(const_cast<TVMValue*>(args.values), const_cast<int*>(args.type_codes),
args.num_args, rv, resource_handle);
if (ret != 0) {
- throw dmlc::Error(TVMGetLastError() + ::dmlc::StackTrace());
+ throw tvm::Error(TVMGetLastError() +
+ tvm::runtime::Backtrace()); // TODO(tkonolige): this may double backtrace
Review comment:
TVMGetLastError may already contain a backtrace, so this would double it. (This was a preexisting issue).
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r548204709
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace/configure" "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
Review comment:
Good question, I still need to look at windows support.
----------------------------------------------------------------
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
[GitHub] [tvm] tqchen commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-794190828
cc @zhiics @yzhliu @Laurawly @leandron @hzfan @jcf94 would be great if you can also help to review 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:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r581356381
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -474,7 +474,8 @@ int TVMFuncCreateFromCFunc(TVMPackedCFunc func, void* resource_handle, TVMPacked
int ret = func(const_cast<TVMValue*>(args.values), const_cast<int*>(args.type_codes),
args.num_args, rv, resource_handle);
if (ret != 0) {
- throw dmlc::Error(TVMGetLastError() + ::dmlc::StackTrace());
+ throw tvm::Error(TVMGetLastError() +
+ tvm::runtime::Backtrace()); // TODO(tkonolige): this may double backtrace
Review comment:
can you comment this in? why would it already contain a backtrace--because this function was previously called down-stack?
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 edited a comment on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 edited a comment on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-749842135
Thank you Tristan for the hard work! Would love to ask some questions just for clarification on the forum first :-)
Would you like to also copy your bullet points to the forum as well? 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:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r552998444
##########
File path: src/support/logging.cc
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.
+ */
+#include <backtrace.h>
+#include <cxxabi.h>
+#include <tvm/support/logging.h>
+
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+#include <vector>
+
+struct frame {};
+
+struct backtrace_info {
+ std::vector<std::string> lines;
+ size_t max_size;
+ std::string error_message;
+};
+
+static backtrace_state* _backtrace_state = nullptr;
+
+std::string demangle_name(const char* name) {
+ int status = 0;
+ size_t length = std::string::npos;
+ std::unique_ptr<char, void (*)(void* __ptr)> demangled_name = {
+ abi::__cxa_demangle(name, 0, &length, &status), &std::free};
+ if (demangled_name && status == 0 && length > 0) {
+ return demangled_name.get();
+ } else {
+ return name;
+ }
+}
+
+void tvm_backtrace_error(void* data, const char* msg, int errnum) {
+ auto stack_trace = reinterpret_cast<backtrace_info*>(data);
+ stack_trace->error_message = msg;
+}
+
+void tvm_backtrace_syminfo_callback(void* data, uintptr_t pc, const char* symname, uintptr_t symval,
+ uintptr_t symsize) {
+ auto str = reinterpret_cast<std::string*>(data);
+
+ if (symname != nullptr) {
+ *str = demangle_name(symname);
+ } else {
+ std::ostringstream s;
+ s << "0x" << std::setfill('0') << std::setw(sizeof(uintptr_t) * 2) << std::hex << pc;
+ *str = s.str();
+ }
+}
+int tvm_backtrace_full_callback(void* data, uintptr_t pc, const char* filename, int lineno,
Review comment:
A generic comment, use CamelCase as per Google C style. It is OK to wrap it in namespace if there is no strict need of a C ABI symbol
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572322269
##########
File path: tests/scripts/task_build.sh
##########
@@ -16,4 +16,4 @@
# specific language governing permissions and limitations
# under the License.
export VTA_HW_PATH=`pwd`/3rdparty/vta-hw
-cd $1 && cmake .. && make $2 && cd ..
+cd $1 && cmake .. -DCMAKE_BUILD_TYPE=Debug && make $2 && cd ..
Review comment:
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:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r553499647
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace/configure" "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
Review comment:
Thank you for the help @rkimball!
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r548204570
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace/configure" "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
Review comment:
Could you expand on this comment?
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572251329
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,435 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <dmlc/common.h>
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
+namespace runtime {
+
+#ifndef TVM_BACKTRACE_DISABLED
+/* \brief Generate a backtrace when called.
+ * \return A multiline string of the backtrace. There will be either one or two lines per frame.
+ */
+std::string Backtrace();
+#else
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+inline std::string Backtrace() { return ""; }
+#endif
+
+/*! \brief Base error type for TVM. Wraps a string message. */
+class Error : public ::dmlc::Error { // for backwards compatibility
+ public:
+ /*! \brief Construct an error.
+ * \param s The message to be displayed with the error.
+ */
+ explicit Error(const std::string& s) : ::dmlc::Error(s) {}
+};
+
+/*! \brief Error type for errors from CHECK, ICHECK, and LOG(FATAL). This error
+ * contains a backtrace of where it occured.
+ */
+class InternalError : public Error {
+ public:
+ /*! \brief Construct an error. Not recommended to use directly. Instead use LOG(FATAL).
+ *
+ * \param file The file where the error occurred.
+ * \param lineno The line number where the error occurred.
+ * \param message The error message to display.
+ * \param time The time at which the error occurred.
+ * \param backtrace Backtrace from when the error occurred.
+ */
+ InternalError(std::string file, int lineno, std::string message,
+ std::time_t time = std::time(nullptr), std::string backtrace = Backtrace())
+ : Error(""),
+ file_(file),
+ lineno_(lineno),
+ message_(message),
+ time_(time),
+ backtrace_(backtrace) {
+ std::ostringstream s;
+ // XXX: Do not change this format, otherwise all error handling in python will break (because it
Review comment:
XXX = do not submit?
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r552976482
##########
File path: include/tvm/support/logging.h
##########
@@ -113,35 +119,171 @@
namespace tvm {
+std::string backtrace();
+
+struct Error : public std::runtime_error {
+ explicit Error(const std::string &s) : std::runtime_error(s) {}
+};
+
+class InternalError : public std::exception {
+ public:
+ InternalError(const std::string& file, int lineno, const std::string& message,
+ const std::time_t& time = std::time(nullptr), std::string stacktrace = backtrace())
Review comment:
I think it makes more sense to keep this error specific to TVM. It prints a message about how to report internal errors to the TVM discuss.
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r581348514
##########
File path: include/tvm/ir/diagnostic.h
##########
@@ -37,6 +37,15 @@ namespace tvm {
using tvm::parser::SourceMap;
using tvm::runtime::TypedPackedFunc;
+/*! \brief The diagnostic level, controls the printing of the message. */
+enum class DiagnosticLevel : int {
+ kBug = 10,
Review comment:
I actually don't mind these labels for indicating what a help message contains, but it would be helpful to comment the translation to log levels here so I don't have to look that up. I imagine people will try to work with these labels but will be approaching this from "the build log is too spammy"
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r581342651
##########
File path: include/tvm/ir/diagnostic.h
##########
@@ -37,6 +37,15 @@ namespace tvm {
using tvm::parser::SourceMap;
using tvm::runtime::TypedPackedFunc;
+/*! \brief The diagnostic level, controls the printing of the message. */
+enum class DiagnosticLevel : int {
+ kBug = 10,
Review comment:
I think logging will be for users when it is possible to adjust the c log level at runtime
----------------------------------------------------------------
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
[GitHub] [tvm] rkimball commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
rkimball commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r573215686
##########
File path: CMakeLists.txt
##########
@@ -379,6 +386,26 @@ set_property(TARGET tvm APPEND PROPERTY LINK_OPTIONS "${TVM_VISIBILITY_FLAG}")
add_library(tvm_runtime SHARED $<TARGET_OBJECTS:tvm_runtime_objs>)
set_property(TARGET tvm_runtime APPEND PROPERTY LINK_OPTIONS "${TVM_VISIBILITY_FLAG}")
+target_compile_definitions(tvm_objs PUBLIC DMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
+target_compile_definitions(tvm_runtime_objs PUBLIC DMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
+target_compile_definitions(tvm PUBLIC DMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
+target_compile_definitions(tvm_runtime PUBLIC DMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
Review comment:
You should be able to define these on the external project for libbacktrace and then when you target_link_libraries to it you will get all of the PUBLIC compile definitions and include directories.
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,54 @@
+# 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.
+include(ExternalProject)
+
+# Git tag for libbacktrace
+set(TAG dedbe13fda00253fe5d4f2fb812c909729ed5937)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ GIT_REPOSITORY https://github.com/ianlancetaylor/libbacktrace.git
+ GIT_TAG "${TAG}"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/src/project_libbacktrace/configure"
+ "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace" --with-pic
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
+ BUILD_BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/lib/libbacktrace.a"
+ "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include/backtrace.h"
+ # disable the builtin update because it rebuilds on every build
+ UPDATE_COMMAND ""
+ )
+
+# Only rebuild libbacktrace if this file changes
+# libbacktrace has a bug on macOS with shared libraries, so we patch it here
+ExternalProject_Add_Step(project_libbacktrace checkout
Review comment:
ExternalProject has a PATCH command which can apply a patch file. I have used it often when using external code.
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572252014
##########
File path: include/tvm/runtime/logging.h
##########
@@ -0,0 +1,435 @@
+/*
+ * 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/runtime/logging.h
+ * \brief logging utilities
+ *
+ * We define our own CHECK and LOG macros to replace those from dmlc-core.
+ * These macros are then injected into dmlc-core via the
+ * DMLC_USE_LOGGING_LIBRARY define. dmlc-core will #include this file wherever
+ * it needs logging.
+ */
+#ifndef TVM_RUNTIME_LOGGING_H_
+#define TVM_RUNTIME_LOGGING_H_
+
+#include <dmlc/common.h>
+
+#include <ctime>
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "tvm/runtime/c_runtime_api.h"
+
+// a technique that enables overriding macro names on the number of parameters. This is used
+// to define other macros below
+#define GET_MACRO(_1, _2, _3, _4, _5, NAME, ...) NAME
+
+/*!
+ * \brief COND_X calls COND_X_N where N is the number of parameters passed to COND_X
+ * X can be any of CHECK_GE, CHECK_EQ, CHECK, or LOG COND_X (but not COND_X_N)
+ * are supposed to be used outside this file.
+ * The first parameter of COND_X (and therefore, COND_X_N), which we call 'quit_on_assert',
+ * is a boolean. The rest of the parameters of COND_X is the same as the parameters of X.
+ * quit_on_assert determines the overall behavior of COND_X. If it's true COND_X
+ * quits the program on assertion failure. If it's false, then it moves on and somehow reports
+ * the assertion failure back to the macro caller in an appropriate manner (e.g, 'return false'
+ * in a function, or 'continue' or 'break' in a loop)
+ * The default behavior when quit_on_assertion is false, is to 'return false'. If this is not
+ * desirable, the macro caller can pass one more last parameter to COND_X to tell COND_X what
+ * to do when when quit_on_assertion is false and the assertion fails.
+ *
+ * Rationale: These macros were designed to implement functions that have two behaviors
+ * in a concise way. Those behaviors are quitting on assertion failures, or trying to
+ * move on from assertion failures. Note that these macros hide lots of control flow in them,
+ * and therefore, makes the logic of the whole code slightly harder to understand. However,
+ * in pieces of code that use these macros frequently, it will significantly shorten the
+ * amount of code needed to be read, and we won't need to clutter the main logic of the
+ * function by repetitive control flow structure. The first problem
+ * mentioned will be improved over time as the developer gets used to the macro.
+ *
+ * Here is an example of how to use it
+ * \code
+ * bool f(..., bool quit_on_assertion) {
+ * int a = 0, b = 0;
+ * ...
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'return false' (default
+ * behaviour) COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"
+ * ...
+ * for (int i = 0; i < N; i++) {
+ * a = ...
+ * b = ...
+ * // if quit_on_assertion is true, if a==b, continue, otherwise quit.
+ * // if quit_on_assertion is false, if a==b, continue, otherwise 'break' (non-default
+ * // behaviour, therefore, has to be explicitly specified)
+ * COND_CHECK_EQ(quit_on_assertion, a, b, break) << "some error message when quiting"
+ * }
+ * }
+ * \endcode
+ */
+#define COND_CHECK_GE(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_GE_5, COND_CHECK_GE_4, COND_CHECK_GE_3)(__VA_ARGS__)
+#define COND_CHECK_EQ(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_EQ_5, COND_CHECK_EQ_4, COND_CHECK_EQ_3)(__VA_ARGS__)
+#define COND_CHECK(...) \
+ GET_MACRO(__VA_ARGS__, COND_CHECK_5, COND_CHECK_4, COND_CHECK_3, COND_CHECK_2)(__VA_ARGS__)
+#define COND_LOG(...) \
+ GET_MACRO(__VA_ARGS__, COND_LOG_5, COND_LOG_4, COND_LOG_3, COND_LOG_2)(__VA_ARGS__)
+
+// Not supposed to be used by users directly.
+#define COND_CHECK_OP(quit_on_assert, x, y, what, op) \
+ if (!quit_on_assert) { \
+ if (!((x)op(y))) what; \
+ } else /* NOLINT(*) */ \
+ CHECK_##op(x, y)
+
+#define COND_CHECK_EQ_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, ==)
+#define COND_CHECK_GE_4(quit_on_assert, x, y, what) COND_CHECK_OP(quit_on_assert, x, y, what, >=)
+
+#define COND_CHECK_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ if (!(x)) what; \
+ } else /* NOLINT(*) */ \
+ CHECK(x)
+
+#define COND_LOG_3(quit_on_assert, x, what) \
+ if (!quit_on_assert) { \
+ what; \
+ } else /* NOLINT(*) */ \
+ LOG(x)
+
+#define COND_CHECK_EQ_3(quit_on_assert, x, y) COND_CHECK_EQ_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_GE_3(quit_on_assert, x, y) COND_CHECK_GE_4(quit_on_assert, x, y, return false)
+#define COND_CHECK_2(quit_on_assert, x) COND_CHECK_3(quit_on_assert, x, return false)
+#define COND_LOG_2(quit_on_assert, x) COND_LOG_3(quit_on_assert, x, return false)
+
+#define TVM_THROW_EXCEPTION noexcept(false)
+
+namespace tvm {
+namespace runtime {
+
+#ifndef TVM_BACKTRACE_DISABLED
+/* \brief Generate a backtrace when called.
+ * \return A multiline string of the backtrace. There will be either one or two lines per frame.
+ */
+std::string Backtrace();
+#else
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+inline std::string Backtrace() { return ""; }
+#endif
+
+/*! \brief Base error type for TVM. Wraps a string message. */
+class Error : public ::dmlc::Error { // for backwards compatibility
+ public:
+ /*! \brief Construct an error.
+ * \param s The message to be displayed with the error.
+ */
+ explicit Error(const std::string& s) : ::dmlc::Error(s) {}
+};
+
+/*! \brief Error type for errors from CHECK, ICHECK, and LOG(FATAL). This error
+ * contains a backtrace of where it occured.
+ */
+class InternalError : public Error {
+ public:
+ /*! \brief Construct an error. Not recommended to use directly. Instead use LOG(FATAL).
+ *
+ * \param file The file where the error occurred.
+ * \param lineno The line number where the error occurred.
+ * \param message The error message to display.
+ * \param time The time at which the error occurred.
Review comment:
if supplying this argument, is there guidance as to which clock should be used?
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572276098
##########
File path: src/runtime/logging.cc
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.
+ */
+
+#ifndef TVM_BACKTRACE_DISABLED
+
+#include <backtrace.h>
+#include <cxxabi.h>
+#include <tvm/runtime/logging.h>
+
+#include <iomanip>
+#include <iostream>
+#include <sstream>
+#include <string>
+#include <vector>
+
+namespace tvm {
+namespace runtime {
+namespace {
+
+struct BacktraceInfo {
+ std::vector<std::string> lines;
+ size_t max_size;
+ std::string error_message;
+};
+
+static backtrace_state* _backtrace_state = nullptr;
Review comment:
Ideally, we probably don't want a global static variable. One way to do this is to wrap it with a function, like:
```C++
backtrace_state** GetBacktraceState() {
thread_local backtrace_state* state = nullptr;
return &state;
}
```
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-775306517
@junrushao1994 @tqchen @areusch CI is all green now. Could I get a re-review?
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572250851
##########
File path: cmake/config.cmake
##########
@@ -269,3 +269,8 @@ set(USE_HEXAGON_SDK /path/to/sdk)
# Whether to use ONNX codegen
set(USE_TARGET_ONNX OFF)
+
+# Whether to use libbacktrace
+# Libbacktrace provides line and column information on stack traces from errors. It is only
Review comment:
add an explanation of the exact bug so we know how to sync your patch in the future
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r547579013
##########
File path: include/tvm/target/tag.h
##########
@@ -139,7 +139,7 @@ inline TargetTagRegEntry& TargetTagRegEntry::set_name() {
}
#define TVM_TARGET_TAG_REGISTER_VAR_DEF \
- static DMLC_ATTRIBUTE_UNUSED ::tvm::TargetTagRegEntry& __make_##TargetTag
+ static __attribute__((unused)) ::tvm::TargetTagRegEntry& __make_##TargetTag
Review comment:
Let's define `TVM_ATTRIBUTE_UNUSED` instead
##########
File path: include/tvm/support/with.h
##########
@@ -65,7 +63,7 @@ class With {
ctx_.EnterWithScope();
}
/*! \brief destructor, leaves the scope of the context. */
- ~With() DMLC_THROW_EXCEPTION { ctx_.ExitWithScope(); }
+ ~With() noexcept(false) { ctx_.ExitWithScope(); }
Review comment:
Let's define `TVM_THROW_EXCEPTION ` instea
##########
File path: CMakeLists.txt
##########
@@ -526,3 +530,25 @@ if(MSVC)
target_compile_definitions(tvm_objs PRIVATE -DTVM_EXPORTS)
target_compile_definitions(tvm_runtime_objs PRIVATE -DTVM_EXPORTS)
endif()
+
+set(IS_DEBUG_BUILD OFF)
Review comment:
let's prefix it with TVM_, like `TVM_IS_DEBUG`
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r573255711
##########
File path: CMakeLists.txt
##########
@@ -379,6 +386,26 @@ set_property(TARGET tvm APPEND PROPERTY LINK_OPTIONS "${TVM_VISIBILITY_FLAG}")
add_library(tvm_runtime SHARED $<TARGET_OBJECTS:tvm_runtime_objs>)
set_property(TARGET tvm_runtime APPEND PROPERTY LINK_OPTIONS "${TVM_VISIBILITY_FLAG}")
+target_compile_definitions(tvm_objs PUBLIC DMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
+target_compile_definitions(tvm_runtime_objs PUBLIC DMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
+target_compile_definitions(tvm PUBLIC DMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
+target_compile_definitions(tvm_runtime PUBLIC DMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>)
Review comment:
The logging libraries are part of TVM, so I think the define should go there.
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572247322
##########
File path: include/tvm/ir/diagnostic.h
##########
@@ -37,6 +37,15 @@ namespace tvm {
using tvm::parser::SourceMap;
using tvm::runtime::TypedPackedFunc;
+/*! \brief The diagnostic level, controls the printing of the message. */
+enum class DiagnosticLevel : int {
+ kBug = 10,
Review comment:
should we use terminology that reflects logging levels 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:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572286763
##########
File path: include/tvm/ir/diagnostic.h
##########
@@ -37,6 +37,15 @@ namespace tvm {
using tvm::parser::SourceMap;
using tvm::runtime::TypedPackedFunc;
+/*! \brief The diagnostic level, controls the printing of the message. */
+enum class DiagnosticLevel : int {
+ kBug = 10,
Review comment:
These are levels for feedback to the user from compiling a relay expression. I'm not sure if it makes sense to merge them as these are user-facing while logging is more for developers.
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-765624123
@tqchen @junrushao1994 @antinucleon @areusch dmlc-core has merged the changes needed for the PR. I've update this PR to inject logging macros into dmlc-core, so we now use our new logging macros everywhere. I think this PR is ready for a new round of reviews when you get a chance.
----------------------------------------------------------------
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
[GitHub] [tvm] antinucleon commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
antinucleon commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-750442179
I request change this PR until it is able to officially supported by Window. libbacktrace is not supported on Windows, and this building rule will break Windows building. Please specialize for Windows.
----------------------------------------------------------------
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
[GitHub] [tvm] areusch commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r548029374
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
Review comment:
it would be good to double-check that the sources are properly depended on by libbacktrace.a
##########
File path: include/tvm/support/logging.h
##########
@@ -113,35 +119,171 @@
namespace tvm {
+std::string backtrace();
+
+struct Error : public std::runtime_error {
+ explicit Error(const std::string &s) : std::runtime_error(s) {}
+};
+
+class InternalError : public std::exception {
+ public:
+ InternalError(const std::string& file, int lineno, const std::string& message,
+ const std::time_t& time = std::time(nullptr), std::string stacktrace = backtrace())
Review comment:
what do we think about pushing this InternalError object into dmlc and defining a weak function backtrace() there that allows you to override/supply a backtrace?
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-775358117
Thanks @tkonolige for the contribution! I think the PR is in pretty good shape, besides I have some minor concern on the cmake structuring. Please find my comments above.
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r581358135
##########
File path: src/runtime/c_runtime_api.cc
##########
@@ -474,7 +474,8 @@ int TVMFuncCreateFromCFunc(TVMPackedCFunc func, void* resource_handle, TVMPacked
int ret = func(const_cast<TVMValue*>(args.values), const_cast<int*>(args.type_codes),
args.num_args, rv, resource_handle);
if (ret != 0) {
- throw dmlc::Error(TVMGetLastError() + ::dmlc::StackTrace());
+ throw tvm::Error(TVMGetLastError() +
+ tvm::runtime::Backtrace()); // TODO(tkonolige): this may double backtrace
Review comment:
I think my todo is wrong. This is just appending the stack trace to the existing one.
----------------------------------------------------------------
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
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r572296219
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,54 @@
+# 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.
+include(ExternalProject)
+
+# Git tag for libbacktrace
+set(TAG dedbe13fda00253fe5d4f2fb812c909729ed5937)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ GIT_REPOSITORY https://github.com/ianlancetaylor/libbacktrace.git
+ GIT_TAG "${TAG}"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/src/project_libbacktrace/configure"
+ "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace" --with-pic
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
+ BUILD_BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/lib/libbacktrace.a"
+ "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace/include/backtrace.h"
+ # disable the builtin update because it rebuilds on every build
+ UPDATE_COMMAND ""
+ )
+
+# Only rebuild libbacktrace if this file changes
+# libbacktrace has a bug on macOS with shared libraries, so we patch it here
+ExternalProject_Add_Step(project_libbacktrace checkout
Review comment:
Option 1 is actually acceptable if we fork it to a public place like tlc-pack/libbacktrace, where we host many tvm related repos
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r548204295
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
Review comment:
Thanks for the heads up. I've fixed it so that libbacktrace.a depends on its sources correctly. Note that there is a bug with cmake and ninja where dependencies aren't tracked correctly, so you'll have to build twice with ninja when updating libbacktrace. But with make everything works fine.
----------------------------------------------------------------
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
[GitHub] [tvm] rkimball commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
rkimball commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r553391128
##########
File path: cmake/modules/Libbacktrace.cmake
##########
@@ -0,0 +1,18 @@
+include(ExternalProject)
+
+ExternalProject_Add(project_libbacktrace
+ PREFIX libbacktrace
+ SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace"
+ CONFIGURE_COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/libbacktrace/configure" "--prefix=${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/libbacktrace"
+ BUILD_COMMAND make
+ INSTALL_COMMAND make install
Review comment:
This does not work on windows. libbacktrace only has support for building using configure which requires bash and bash is only available on windows via projects such as msys or cygwin which seems like too large a prerequisite for windows.
In the short term I have disabled support for libbacktrace on windows which should still give us support on other platforms. For windows we should make a cmake build script for libbacktrace which is fairly easy. We can patch that in until it can be upstreamed to libbacktrace.
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#issuecomment-801266206
I've switched this to using a submodule.
----------------------------------------------------------------
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
[GitHub] [tvm] tkonolige commented on a change in pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7153:
URL: https://github.com/apache/tvm/pull/7153#discussion_r595362262
##########
File path: src/runtime/logging.cc
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.
+ */
+
+#ifdef TVM_BACKTRACE_DISABLED
+#include <string>
+
+// TODO(bkimball,tkonolige) This inline function is to work around a linking error I am having when
+// using MSVC If the function definition is in logging.cc then the linker can't find it no matter
+// what kind of attributes (dllexport) I decorate it with. This is temporary and will be addressed
+// when we get backtrace working on Windows.
+namespace tvm {
+namespace runtime {
+__declspec(dllexport) std::string Backtrace() { return ""; }
Review comment:
TVM_DLL only conditionally adds __declspec(dllexport), but we need it all the time.
----------------------------------------------------------------
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
[GitHub] [tvm] tqchen merged pull request #7153: [RUNTIME] Add libbacktrace for backtraces with line numbers
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #7153:
URL: https://github.com/apache/tvm/pull/7153
----------------------------------------------------------------
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