Skip to content

Conversation

Scheremo
Copy link
Contributor

This PR adds a wrapper for llvm::dbgs for easier adding of debug prints in critical code sections.

This PR introduces the ImportVerilogDebugStream structure which implements overloaded versions of dbgs to conveniently add compiler source and verilog input source location information from either explicit mlirLocations, or, if context is available, from slang::SourceLocation objects.

I also added a bunch of debug prints as I am debugging quiet crashes in circt-verilog.

If there is a more standard way of doing this let me know, I am happy to adapt.

Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally in favor of adding better debug printing to make hunting down issues easier! I'm wondering though if it wouldn't suffice to just have that dbgs method as a small helper that gives you back a raw_ostream& with the location info already populated. Someting like:

// ImportVerilogInternals.h
llvm::raw_ostream &dbgsLoc(
  std::variant<Location, slang::SourceLocation> srcLoc,
  std::source_location codeLoc = std::source_location::current()
);

// ImportVerilog.cpp
llvm::raw_ostream &dbgsLoc(
  std::variant<Location, slang::SourceLocation> srcLoc,
  std::source_location codeLoc
) {
  auto &os = llvm::dbgs();
  os << codeLoc.file_name() << ":";
  os << codeLoc.line() << ":";
  os << codeLoc.column() << ": ";
  os << codeLoc.function_name() << ": "
  if (auto *loc = std::get_if<Location>(srcLoc))
    os << *srcLoc << ": ";
  else if (auto *loc = std::get_if<slang::SourceLocation>(srcLoc))
    os << slang::toString(*loc) << ": ";  // or something like that
  return os;
}

We still have to wrap the call to this in LLVM_DEBUG(...), since we want the calls to the debug print function to be stripped out completely in non-debug builds. Something like:

if (!value) {
  LLVM_DEBUG(dbgsLoc(loc) << "yikes\n");
  return {};
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants