[Relay] VLOG for finer grained control of hyper-detailed logging #9012

mbs-octoml · 2021-09-14T21:18:02Z

I've been making very heavy use of DLOG and PrettyPrint to trace, understand and
debug Relay transforms. I've found myself deleting log statements to reduce the
output verbosity, only to have to re-add them a few days later. Better would be to
support leaving all those statements in place but have finer control over when they
are enabled.

This PR introduces a 'VLOG(level)' macro to that end. The log is ignored
unless TVM_LOG_DEBUG is enabled (both as #define and an environment var), and the
the current verbosity level is >= level. The current verbosity level can be set
globally and/or overridden per source file (see 'VerboseLoggingEnabled').

(Those familiar with the origin of the LOG and DLOG family will also recognize VLOG.)

I also introduce a 'VLOG_CONTEXT' macro which pushes a string onto an internal
per-thread stack. Each VLOG message includes that stack as its prefix, which is a
very handy way to keep track of the (often recursive) program context in which each
VLOG is executed.

I've rejigged some existing DLOGs to VLOGs to illustrate, but left most of them
alone for now. See the draft #8788 for use in the wild.

I noticed the DCHECK macros were disabled instead of enabled with TVM_LOG_DEBUG
defined, so fixed that.

I've also made changes to the Relay text printer to dump attributes in a human
readable format rather than the indirect but machine readable 'meta' representation.
This is gated by the show_meta_data_ flag, and I think this use is consistent with it's
original purpose.

huajsj · 2021-09-14T22:31:30Z

include/tvm/runtime/logging.h

+    if (const char* var = std::getenv("TVM_LOG_DEBUG")) {
+      if (var[0] == '1') {


According this logic there is no difference for TVM_LOG_DEBUG = 11 and TVM_LOG_DEBUG = 1, this seems like not make sense, could you help to give more information about why just check first digit here?

This was a lazy way to allow "1" or "1;...." to enable. I made it more specific.

huajsj · 2021-09-14T22:41:41Z

include/tvm/runtime/logging.h

@@ -409,6 +413,68 @@ inline bool DebugLoggingEnabled() {
  return state == 1;
 }

+/*! \brief Helpers for \p VerboseLoggingEnabled. Exposed for unit testing only. */
+std::unordered_map<std::string, int> ParseTvmLogDebugSpec(const char* opt_spec);
+bool VerboseEnabledInMap(const char* filename, int level,


here mixed used "std::string" and "char *", recommend to use std::string to match existing global function style that used string.

Done. Note the original filename comes via a FILE so I left const char* for the main func.

huajsj · 2021-09-14T22:43:12Z

include/tvm/runtime/logging.h

+                         const std::unordered_map<std::string, int>& map);
+
+/*!
+ * \brief Returns true if a VLOG statement in \p filename is enabled by the \p TVM_LOG_DEBUG


the "\p" seems like not necessary.

I'm using that as a convention to quote parameters or code. Looks nicer in the formatted comments.

huajsj · 2021-09-14T22:43:56Z

include/tvm/runtime/logging.h

+  std::string str() const;
+
+ private:
+  std::vector<std::stringstream*> context_stack;


context_stack_

Done , thx.

huajsj · 2021-09-14T22:45:21Z

include/tvm/runtime/logging.h

 // Re-export error types
 using runtime::Error;
 using runtime::InternalError;
+


no need this line.

huajsj · 2021-09-14T22:57:28Z

src/runtime/logging.cc

+  size_t last_src = key.rfind("/src/", std::string::npos, 5);
+  if (last_src != std::string::npos) {
+    key = key.substr(last_src + 5);


key can be filename or the string after last "/src/", why for some file the key can be file name and other can not ?

for the file /abc/src/foo.cc and /def/src/foo.cc, the key will be same , is it intend logic?

should avoid hardcode logic that use 5 directly

const char * src = "/src/"; size_t src_len = strlen(src); size_t last_src = key.rfind(src, std::string::npos, src_len); .... key = key.substr(last_src + src_len);

Done for code cleanup. Just wanted to gracefully fallback if the filename for some reason is outside of the /src/ root. Could LOG(FATAL) since it signals the user is trying to use VLOG in, say, include. wdyt?

huajsj · 2021-09-14T23:02:30Z

src/runtime/logging.cc

+  auto itr = map.find("*");
+  if (itr != map.end()) {
+    limit = itr->second;
+  }
+  // Apply specific filename if any.
+  itr = map.find(key);
+  if (itr != map.end()) {
+    limit = itr->second;
+  }


the duplicate map search seems like not efficient, why not do "*" search only when key not found?

Yes that was silly of me!

huajsj · 2021-09-14T23:04:34Z

thanks @mbs-octoml.

mbs-octoml · 2021-09-14T23:58:24Z

@huajsj thanks for the quick review, PTAL.

huajsj · 2021-09-15T17:58:01Z

CMakeLists.txt

-  file(GLOB TEST_SRCS tests/cpp/*.cc)
+  file(GLOB_RECURSE TEST_SRCS tests/cpp/*.cc)


why changed from GLOB into GLOB_RECURSE?

Because i'm adding a unit test under runtime/, paving the way for adding more c++ unit tests in a directory structure that mirrors src/.

huajsj · 2021-09-15T18:00:06Z

include/tvm/runtime/logging.h

+ *   // (default behaviour)
+ *   COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when  quiting"


136 and 137 should be same line.

ok, but that will have to be for another cl unless ci fails :-|

huajsj · 2021-09-15T18:02:22Z

include/tvm/runtime/logging.h

@@ -395,10 +399,13 @@ class LogMessageVoidify {
 inline bool DebugLoggingEnabled() {
  static int state = 0;
  if (state == 0) {
-    if (auto var = std::getenv("TVM_LOG_DEBUG")) {
-      if (std::string(var) == "1") {
+    if (const char* var = std::getenv("TVM_LOG_DEBUG")) {


seems like change not necessary, should keep no change "auto var = std::getenv("TVM_LOG_DEBUG")"

We don't have a consistent style on auto, but generally I spell out the types except for auto x = make_object(...), auto f = ... {...}, and iterators.

huajsj · 2021-09-15T18:06:43Z

include/tvm/runtime/logging.h

-      if (std::string(var) == "1") {
+    if (const char* var = std::getenv("TVM_LOG_DEBUG")) {
+      std::string var_str(var);
+      if (var_str == "1" || var_str.rfind("1;", 0) == 0) {


seems like there are multiple type information get put into TVM_LOG_DEBUG, this would make the use of such environment variable become trivial and complicated, please avoid such usage.

Will address in your comment below.

var_str.find ?

huajsj · 2021-09-15T18:24:10Z

include/tvm/runtime/logging.h

+ * TVM_LOG_DEBUG="1;relay/foo.cc=2;ir/bar.cc=0;"
+ * \endcode
+ *
+ * To enable all files up to level 3 but disable \p ir/bar.cc set:
+ * \code
+ * TVM_LOG_DEBUG="1;*=2;ir/bar.cc=-1;"


This usage looks like not user friendly, is there any other better way to do this? for example
TVM_LOG_DEBUG_LEVEL_2="relay/foo.cc;"
TVM_LOG_DEBUG_LEVEL_0="ir/bar.cc;"

There are many possible designs. I settled on re-using the existing TWM_LOG_DEBUG since

it is one control surface

I often modules on and off during debugging and having one var to redefine makes that easy

According the logic, TVM_LOG_DEBUG now have a hiding strict format assumptions, these setting like "=2;1;" or "=2;1", "*=1;" will be illegal, I don't see any necessary here to introduce such complexity for environment variable content.

I definitely agree with you the ergonomics are horrible! Using an env variable at all is already unfortunate but forced on us by the python-is-the-driver convention for TVM. But I think this is all very temporary and 'for developers only' while we figure out a more structured approach to controlling, capturing and redirecting logging, including both 'for debug' logging as well as logging which may be relevant to downstream customers running tvm in a production environment.
(+ @jroesch since we were chatting about this today).

At this point the CI is green -- do you feel strongly enough we should not proceed as is?

If we agree that is a problem , that sound not make sense not to fix it, and better to avoid a temporary solution that in some case will become a permanent solution, as a user for this VLOG feature, I like the idea filtering in file level, but I think such configuration method is unnecessary complicated and error-prone, if we can better to find a more easy to use method.
about CI is green, I can understand the CI consume time, to get it become green not very easy, but from my point view we should better to more focus on code logic instead of CI.

thanks @areusch, @mbs-octoml for the follow up, that make sense.

Along the same line as 'we have many frontends' I'm also trying to support writing C++ unit tests, and obviously being able to control logging for any test is vital. So any python-only control surface would not work for that case.

As far as syntax is concerned:

obviously the legacy TVM_LOG_DEBUG=1 behavior must be kept. Tat will turn on DLOG but leave the implied default VLOG levels for all files as -1.

but for finer control the '1' start seems to be throwing folks off, so I'll drop that. Eg
TVM_LOG_DEBUG="foo/bar.cc=3" means enable DLOG and VLOG for that file at levels 0 to 3.

@mbs-octoml does that mean there's no way to do global + per-file at once?

ok i see. i'm okay with this change if you also print a warning or just FATAL when someone tries to do src/*.cc=3

huajsj · 2021-09-15T18:27:13Z

src/runtime/logging.cc

+  static const std::unordered_map<std::string, int>* map =
+      new std::unordered_map<std::string, int>(ParseTvmLogDebugSpec(std::getenv("TVM_LOG_DEBUG")));


recommend to use smart pointer.

This is the idiom for statically initialized aggregate structures. Since they can never be freed they go in 'raw' pointers.

areusch · 2021-09-16T15:44:35Z

include/tvm/runtime/logging.h

+ * Filenames are canonicalized to be w.r.t. the src/ dir of the TVM tree. (VLOG's should not
+ * appear under include/).
+ *
+ * To enable file \p relay/foo.cc up to level 2 and \p ir/bar.cc for level 0 only set:


maybe a little confusing to say 2 here but then have TVM_LOG_DEBUG="1 below

Ok, looks like I'm going to go through CI again so let's get this at least better if not perfect. See above mega-comment.

areusch · 2021-09-16T17:29:17Z

include/tvm/runtime/logging.h

+ * TVM_LOG_DEBUG="1;relay/foo.cc=2;ir/bar.cc=0;"
+ * \endcode
+ *
+ * To enable all files up to level 3 but disable \p ir/bar.cc set:
+ * \code
+ * TVM_LOG_DEBUG="1;*=2;ir/bar.cc=-1;"


@huajsj i agree this is a bit more complex, but it is backwards-compatible. i also don't think it's especially complex given what it does.

it seems like a more structured, frontend-agnostic interface might be a PackedFunc like:

TVM_REGISTER_GLOBAL("set_vlog_level").set_body_typed([](const std::string& file_name, int level) { my_global_map[file_name] = level }

I'm totally open to adding this. However, I'd like to point out a couple of advantages of the environment variable route @mbs-octoml proposes here.

use of a single variable or config string: it's very easy to determine exactly the VLOG configuration of TVM. you just echo ${TVM_LOG_DEBUG}. if multiple variables are used, the command becomes more complex: export | grep TVM_LOG_DEBUG_LEVEL_

use of environment variable: We make heavy use of multiprocessing or PopenPool, and have many different frontends. i'd argue that VLOG level configuration is a TVM developer-facing interface. If we use a command-line flag or script to configure logging, then every time a program is launched with a different entry point, a parser would need to be written to convert some command-line flag into a set of get_function("set_vlog_level")(file_name, level) calls. Each script may have a different way to parse out the various vlog levels, so if a TVM C++ developer is trying to help a user debug a problem in libtvm.so, they first would need to determine how to configure TVM VLOG levels. TVM_LOG_DEBUG is a more foolproof route. Additionally, we would need to find a way to thread this down through multiprocessing/PopenPool so that this configuration is propagated to e.g. autotuning Builder/Runner.

I agree with your point that temporary solutions can tend to become permanent. In this case I'd argue that the complex part of this PR is in parsing the environment variable and not in the change to the logging system. Given the advantages listed above, I think I'd still want to keep this functionality even if we overhaul the fundamental way logging works here. I also don't think this mechanism has to be kept particularly backwards-compatible--we can continue to improve it as we consider bigger changes to logging. What do you think?

huajsj · 2021-09-16T20:37:07Z

include/tvm/runtime/logging.h

-      if (std::string(var) == "1") {
+    if (const char* var = std::getenv("TVM_LOG_DEBUG")) {
+      std::string var_str(var);
+      if (var_str == "1" || var_str.rfind("1;", 0) == 0) {


var_str.find ?

huajsj · 2021-09-16T20:38:39Z

src/runtime/logging.cc

+  }
+  std::string spec(opt_spec);
+  // Check we are enabled overall with at least one VLOG option.
+  if (spec.rfind("1;", 0) != 0) {


spec.find ? is there any reason here use rfind instead of find ?

if (spec == "1" || spec.find("1;") != 0)

That's the idiom for checking a string begins with a prefix. It won't waste time looking though the whole string, just check the first 3 chars.

huajsj · 2021-09-16T20:43:52Z

src/runtime/logging.cc

+    LOG(INFO) << "VLOG disabled, TVM_LOG_DEBUG does not start with '1;'";
+    return map;
+  }
+  size_t start = 2UL;


the hardcode 2UL not readable, how about use

const char *str_enable = "1;";
//use str_enable for whether debug enable check.
size_t start = strlen(str_enable);

done - this is going to be rejigged anyway.

huajsj · 2021-09-16T21:00:37Z

src/runtime/logging.cc

+  while (start < spec.size()) {
+    // We are looking for "name=level;" or "*=level;"
+    size_t end = start;
+    // Scan up to '='.
+    while (spec[end] != '=') {
+      ++end;
+      if (end >= spec.size()) {
+        LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, missing '='";
+        return map;
+      }
+    }
+    if (end == start) {
+      LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, empty name";
+      return map;
+    }
+    std::string name(spec.substr(start, end - start));
+    // Skip '='
+    ++end;
+    if (end >= spec.size()) {
+      LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, missing level";
+      return map;
+    }
+    // Scan up to ';'.
+    start = end;
+    while (spec[end] != ';') {
+      ++end;
+      if (end >= spec.size()) {
+        LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, missing ';'";
+        return map;
+      }
+    }
+    if (end == start) {
+      LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, empty level";
+      return map;
+    }
+    std::string level_str(spec.substr(start, end - start));
+    // Skip ';'.
+    ++end;
+    // Parse level, default to 0 if ill-formed which we don't detect.
+    char* end_of_level = nullptr;
+    int level = static_cast<int>(strtol(level_str.c_str(), &end_of_level, 10));
+    if (end_of_level != level_str.c_str() + level_str.size()) {
+      LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, invalid level";
+    }
+    LOG(INFO) << "adding VLOG entry for '" << name << "' at level " << level;
+    map.emplace(name, level);
+    start = end;
+  }


recommend to use istringstream, just a example.

std::istringstream istr(spec.substr(strlen(str_enable))); std::string str; while(getline(istr, str, ';')){ istringstream istr_file(str); string file, level; if (!getline(istr_file, file, '=')){ break; } if (!getline(istr_file, level, '=')) { break; } map.emplace(file, level); }

Thanks, that is nicer. Not quite the same behavior but better than index hacking. I'm a little worried this might be too onerous for the runtimes but let's see what CI says.

mbs-octoml · 2021-09-16T22:25:01Z

PTAL.

mbs-octoml · 2021-09-17T13:37:18Z

@huajsj @areusch PTAL, and CI is green :-)

huajsj · 2021-09-17T01:59:47Z

src/runtime/logging.cc

+  if (opt_spec == nullptr) {
+    // DLOG and VLOG disabled.
+    return map;
+  }
+  std::string spec(opt_spec);
+  if (spec.empty() || spec == "0") {
+    // DLOG and VLOG disabled.
+    return map;
+  }


std::string spec(opt_spec);
if (opt_spec == nullptr || spec.emptry() || spec == "0"){
// DLOG and VLOG disabled.
return map;
}

avoiding undefined string over nullptr

huajsj · 2021-09-18T03:04:32Z

src/runtime/logging.cc

@@ -166,10 +167,118 @@ namespace tvm {
 namespace runtime {
 namespace detail {

+namespace {
+constexpr const char* kSrcPrefix = "/src/";
+constexpr const size_t kSrcPrefixLength = 5;


constexpr size_t kSrcPrefixLength = std::char_traits<const char>::length(kSrcPrefix);

That's a good trick, thx.

Alas it fails on the i386 compilation CI flow, reverted.

huajsj · 2021-09-18T03:41:19Z

src/runtime/logging.cc

+    char* end_of_level = nullptr;
+    int level_val = static_cast<int>(strtol(level.c_str(), &end_of_level, 10));
+    if (end_of_level != level.c_str() + level.size()) {
+      LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, invalid level";
+      return map;
+    }


recommend to use c++ here to make code style consistent and more readable.

int level_val = std::stoi(level); if (to_string(level_val) != level) { LOG(FATAL) << "TVM_LOG_DEBUG ill-formed, invalid level"; return map; }

I think it's good enough.

huajsj · 2021-09-18T03:50:39Z

src/runtime/logging.cc

+
+bool VerboseEnabledInMap(const std::string& filename, int level,
+                         const std::unordered_map<std::string, int>& map) {
+  if (level < 0) {


seems like correct level start from 0 and negative level will no need check, could you add some comments to explain this logic?

Added to VerboseLoggingEnabled comment.

huajsj · 2021-09-18T04:06:01Z

src/runtime/logging.cc

+    // Legacy specification for enabling just DLOG.
+    // A wildcard entry in the map will signal DLOG is on, but all VLOG levels are disabled.
+    LOG(INFO) << "TVM_LOG_DEBUG enables DLOG statements only";
+    map.emplace(kDefaultKeyword, -1);


this logic seems like not backward compatible in performance. when spec == "1", VerboseEnabledInMap still need to do string parse and map search, that seems like not necessary, is there any specific concern here not just leave map be empty and return false in VerboseEnabledInMap when map.empty() is true?

Everything is gated on the TVM_LOG_DEBUG #define, signalling this is a developer build for the purposes of debugging. If this were on prod code paths then we'd need to do some very special tricks to cache the lookup per file.

TVM_LOG_DEBUG gate only can help for TVM_LOG_DEBUG not define or be 0 case, when #define TVM_LOG_DEBUG 1, and TVM_LOG_DEBUG="1", VerboseLoggingEnabled should directly return false instead of still doing filename parse and map search, I think that a normal check instead of special trick and doing correct check should also apply for debug build.

huajsj

LGTM, Thanks @mbs-octoml.

I've been making very heavy use of DLOG and PrettyPrint to trace, understand and debug Relay transforms. I've found myself deleting log statements to reduce the output verbosity, only to have to re-add them a few days later. Better would be to support leaving all those statements in place but have finer control over when they are enabled. This PR introduces a 'VLOG(level)' macro to that end. The log is ignored unless TVM_LOG_DEBUG is enabled (both as #define and an environment var), and the the current verbosity level is >= level. The current verbosity level can be set globally and/or overridden per source file (see 'VerboseLoggingEnabled'). (Those familiar with the origin of the LOG and DLOG family will also recognize VLOG.) I also introduce a 'VLOG_CONTEXT' macro which pushes a string onto an internal per-thread stack. Each VLOG message includes that stack as its prefix, which is a very handy way to keep track of the (often recursive) program context in which each VLOG is executed. I've rejigged some existing DLOGs to VLOGs to illustrate, but left most of them alone for now. See the draft apache#8788 for use in the wild. I noticed the DCHECK macros *disabled* instead enabled with TVM_LOG_DEBUG defined, so fixed that. I've also made changes to the Relay text printer to dump attributes in a human readable format rather than the indirect but machine readable 'meta' representation. This is gated by the show_meta_data_ flag, and I think this use is consistent with it's original purpose.

Gotta get my docker setup going

Andrew pointed out *=9 suggests foo/*.cc=9 would work but it is not supported.

areusch · 2021-09-22T00:08:08Z

thanks @mbs-octoml @huajsj the PR is now merged!

…che#9012) * [Relay] VLOG for finer grained control of hyper-detailed logging I've been making very heavy use of DLOG and PrettyPrint to trace, understand and debug Relay transforms. I've found myself deleting log statements to reduce the output verbosity, only to have to re-add them a few days later. Better would be to support leaving all those statements in place but have finer control over when they are enabled. This PR introduces a 'VLOG(level)' macro to that end. The log is ignored unless TVM_LOG_DEBUG is enabled (both as #define and an environment var), and the the current verbosity level is >= level. The current verbosity level can be set globally and/or overridden per source file (see 'VerboseLoggingEnabled'). (Those familiar with the origin of the LOG and DLOG family will also recognize VLOG.) I also introduce a 'VLOG_CONTEXT' macro which pushes a string onto an internal per-thread stack. Each VLOG message includes that stack as its prefix, which is a very handy way to keep track of the (often recursive) program context in which each VLOG is executed. I've rejigged some existing DLOGs to VLOGs to illustrate, but left most of them alone for now. See the draft apache#8788 for use in the wild. I noticed the DCHECK macros *disabled* instead enabled with TVM_LOG_DEBUG defined, so fixed that. I've also made changes to the Relay text printer to dump attributes in a human readable format rather than the indirect but machine readable 'meta' representation. This is gated by the show_meta_data_ flag, and I think this use is consistent with it's original purpose. * [checkpoint] lints * [checkpoint] missing \n lint Gotta get my docker setup going * [checkpoint] woops, we don't support gmock.h * [checkpoint] Address Hua Jiang's comments. * [checkpoint] strlen not avail on all toolchains? * [checkpoint] Rework TVM_LOG_DEBUG spec and parser * [checkpoint] woops, forgot the static modifier on map * [checkpoint] * -> DEFAULT for wildcard. Andrew pointed out *=9 suggests foo/*.cc=9 would work but it is not supported. * [checkpoint] constexpr length * [checkpoint] length is not constexpr on all targets, reverting * [checkpoint] minimize VLOG overhead when in legacy DLOG-only mode

linlifan · 2021-10-12T09:28:28Z

it seems like after this commit , IR print under TVM_LOG_DEBUG=1 control is gone again...... i think many people are relying on it to debug performance variations, better there are some DOC to explain , how to enable it back

mbs-octoml · 2021-10-13T22:41:34Z

@linlifan you're quite right, sorry. #9278.

…che#9012) * [Relay] VLOG for finer grained control of hyper-detailed logging I've been making very heavy use of DLOG and PrettyPrint to trace, understand and debug Relay transforms. I've found myself deleting log statements to reduce the output verbosity, only to have to re-add them a few days later. Better would be to support leaving all those statements in place but have finer control over when they are enabled. This PR introduces a 'VLOG(level)' macro to that end. The log is ignored unless TVM_LOG_DEBUG is enabled (both as #define and an environment var), and the the current verbosity level is >= level. The current verbosity level can be set globally and/or overridden per source file (see 'VerboseLoggingEnabled'). (Those familiar with the origin of the LOG and DLOG family will also recognize VLOG.) I also introduce a 'VLOG_CONTEXT' macro which pushes a string onto an internal per-thread stack. Each VLOG message includes that stack as its prefix, which is a very handy way to keep track of the (often recursive) program context in which each VLOG is executed. I've rejigged some existing DLOGs to VLOGs to illustrate, but left most of them alone for now. See the draft apache#8788 for use in the wild. I noticed the DCHECK macros *disabled* instead enabled with TVM_LOG_DEBUG defined, so fixed that. I've also made changes to the Relay text printer to dump attributes in a human readable format rather than the indirect but machine readable 'meta' representation. This is gated by the show_meta_data_ flag, and I think this use is consistent with it's original purpose. * [checkpoint] lints * [checkpoint] missing \n lint Gotta get my docker setup going * [checkpoint] woops, we don't support gmock.h * [checkpoint] Address Hua Jiang's comments. * [checkpoint] strlen not avail on all toolchains? * [checkpoint] Rework TVM_LOG_DEBUG spec and parser * [checkpoint] woops, forgot the static modifier on map * [checkpoint] * -> DEFAULT for wildcard. Andrew pointed out *=9 suggests foo/*.cc=9 would work but it is not supported. * [checkpoint] constexpr length * [checkpoint] length is not constexpr on all targets, reverting * [checkpoint] minimize VLOG overhead when in legacy DLOG-only mode

mbs-octoml requested review from anijain2305, areusch, comaniac, jroesch, junrushao, jwfromm, kazum, liangfu, MarisaKirisame, masahi, mbrookhart, merrymercy, slyubomirsky, tmoreau89, tqchen, vinx13, wweic, yzhliu, zhiics, ZihengJiang and a team as code owners September 14, 2021 21:18

huajsj reviewed Sep 14, 2021

View reviewed changes

mbs-octoml force-pushed the mbs-vlog branch from 43d1584 to 7507e71 Compare September 14, 2021 23:19

huajsj reviewed Sep 15, 2021

View reviewed changes

areusch reviewed Sep 16, 2021

View reviewed changes

huajsj reviewed Sep 16, 2021

View reviewed changes

mbs-octoml mentioned this pull request Sep 17, 2021

[Relay] Merge analysis/context_analysis.cc and transforms/device_annotation.cc #9038

Merged

mbs-octoml force-pushed the mbs-vlog branch from 0771005 to 3482fc2 Compare September 17, 2021 23:06

huajsj reviewed Sep 18, 2021

View reviewed changes

mbs-octoml force-pushed the mbs-vlog branch from 3482fc2 to c401079 Compare September 18, 2021 16:16

areusch self-assigned this Sep 20, 2021

mbs-octoml force-pushed the mbs-vlog branch from 48dfa1c to 3b71f7c Compare September 20, 2021 23:08

huajsj approved these changes Sep 21, 2021

View reviewed changes

mbs-octoml added 12 commits September 21, 2021 08:27

[checkpoint] lints

51dfc56

[checkpoint] missing \n lint

5751337

Gotta get my docker setup going

[checkpoint] woops, we don't support gmock.h

b7b8176

[checkpoint] Address Hua Jiang's comments.

e75d7a9

[checkpoint] strlen not avail on all toolchains?

69ea23f

[checkpoint] Rework TVM_LOG_DEBUG spec and parser

214ab82

[checkpoint] woops, forgot the static modifier on map

fdf61c0

[checkpoint] * -> DEFAULT for wildcard.

09a1722

Andrew pointed out *=9 suggests foo/*.cc=9 would work but it is not supported.

[checkpoint] constexpr length

0a90d40

[checkpoint] length is not constexpr on all targets, reverting

e59321e

[checkpoint] minimize VLOG overhead when in legacy DLOG-only mode

bf088b9

mbs-octoml force-pushed the mbs-vlog branch from 3b71f7c to bf088b9 Compare September 21, 2021 15:28

areusch mentioned this pull request Sep 21, 2021

Support verisilicon's NPU with BYOC framework #9046

Closed

areusch approved these changes Sep 22, 2021

View reviewed changes

areusch merged commit 3fbce70 into apache:main Sep 22, 2021

mbs-octoml deleted the mbs-vlog branch September 22, 2021 19:13

		if (const char* var = std::getenv("TVM_LOG_DEBUG")) {
		if (var[0] == '1') {

		file(GLOB TEST_SRCS tests/cpp/*.cc)
		file(GLOB_RECURSE TEST_SRCS tests/cpp/*.cc)

		* // (default behaviour)
		* COND_CHECK_EQ(quit_on_assertion, a, b) << "some error message when quiting"

		static const std::unordered_map<std::string, int>* map =
		new std::unordered_map<std::string, int>(ParseTvmLogDebugSpec(std::getenv("TVM_LOG_DEBUG")));

[Relay] VLOG for finer grained control of hyper-detailed logging #9012

[Relay] VLOG for finer grained control of hyper-detailed logging #9012

Conversation

mbs-octoml commented Sep 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huajsj commented Sep 14, 2021

mbs-octoml commented Sep 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huajsj Sep 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

areusch Sep 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbs-octoml commented Sep 16, 2021

mbs-octoml commented Sep 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbs-octoml Sep 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huajsj left a comment

Choose a reason for hiding this comment

areusch commented Sep 22, 2021

linlifan commented Oct 12, 2021

mbs-octoml commented Oct 13, 2021

mbs-octoml commented Sep 14, 2021 •

edited

Loading

huajsj Sep 16, 2021 •

edited

Loading

areusch Sep 17, 2021 •

edited

Loading

mbs-octoml Sep 18, 2021 •

edited

Loading