From 83fb49d23185c6e18e7114470e5630a2a68f06ff Mon Sep 17 00:00:00 2001 From: Moritz Scherer Date: Fri, 19 Sep 2025 12:01:52 +0200 Subject: [PATCH 1/2] Add ImportVerilogDebugStream --- lib/Conversion/ImportVerilog/CMakeLists.txt | 1 + .../ImportVerilog/ImportVerilog.cpp | 1 + .../ImportVerilogDebugStream.cpp | 28 ++++++ .../ImportVerilog/ImportVerilogDebugStream.h | 93 +++++++++++++++++++ .../ImportVerilog/ImportVerilogInternals.h | 7 ++ 5 files changed, 130 insertions(+) create mode 100644 lib/Conversion/ImportVerilog/ImportVerilogDebugStream.cpp create mode 100644 lib/Conversion/ImportVerilog/ImportVerilogDebugStream.h diff --git a/lib/Conversion/ImportVerilog/CMakeLists.txt b/lib/Conversion/ImportVerilog/CMakeLists.txt index dbb17a18b2fd..ac811a4e30d0 100644 --- a/lib/Conversion/ImportVerilog/CMakeLists.txt +++ b/lib/Conversion/ImportVerilog/CMakeLists.txt @@ -1,6 +1,7 @@ include(SlangCompilerOptions) add_circt_translation_library(CIRCTImportVerilog + ImportVerilogDebugStream.cpp AssertionExpr.cpp Expressions.cpp FormatStrings.cpp diff --git a/lib/Conversion/ImportVerilog/ImportVerilog.cpp b/lib/Conversion/ImportVerilog/ImportVerilog.cpp index db5c4f0b9a28..443745c179fe 100644 --- a/lib/Conversion/ImportVerilog/ImportVerilog.cpp +++ b/lib/Conversion/ImportVerilog/ImportVerilog.cpp @@ -23,6 +23,7 @@ #include "slang/parsing/Preprocessor.h" #include "slang/syntax/SyntaxPrinter.h" #include "slang/util/VersionInfo.h" +#include "ImportVerilogDebugStream.h" using namespace mlir; using namespace circt; diff --git a/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.cpp b/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.cpp new file mode 100644 index 000000000000..cdd18715e8f2 --- /dev/null +++ b/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.cpp @@ -0,0 +1,28 @@ +#include "ImportVerilogDebugStream.h" +#include "ImportVerilogInternals.h" + +ImportVerilogDebugStream +dbgs(const std::optional sourceLocation, + const std::optional cl) { + + return ImportVerilogDebugStream(sourceLocation, cl); +} + +ImportVerilogDebugStream circt::ImportVerilog::Context::dbgs( + const std::optional> + sourceLocation, + const std::optional cl) { + + std::optional mlirLoc = {}; + + if (sourceLocation) { + if (auto *ml = std::get_if(&*sourceLocation)) { + mlirLoc = *ml; + } else if (auto *sl = + std::get_if(&*sourceLocation)) { + mlirLoc = convertLocation(*sl); + } + } + + return ImportVerilogDebugStream(mlirLoc, cl); +} diff --git a/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.h b/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.h new file mode 100644 index 000000000000..5ebf2f81c164 --- /dev/null +++ b/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.h @@ -0,0 +1,93 @@ +//===- ImportVerilogDebugStream.h - A debug stream wrapper for importverilog +//---------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef IMPORT_VERILOG_DEBUG_H +#define IMPORT_VERILOG_DEBUG_H + +#include "mlir/IR/Location.h" +#include "slang/ast/Compilation.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/raw_ostream.h" +#include +#include +#include + +// If includer defined DEBUG_TYPE, use that; else fall back to our default. +#ifndef IMPORTVERILOG_DEBUG_TYPE + #ifdef DEBUG_TYPE + #define IMPORTVERILOG_DEBUG_TYPE DEBUG_TYPE + #else + #define IMPORTVERILOG_DEBUG_TYPE "import-verilog" + #endif +#endif + +struct ImportVerilogDebugStream { + std::optional srcLoc; + std::optional compLoc; + llvm::SmallString<128> buffer; + llvm::raw_svector_ostream os; + + ImportVerilogDebugStream(std::optional sl, + std::optional cl) + : srcLoc(sl), compLoc(cl), os(buffer) {} + + inline ~ImportVerilogDebugStream() { + DEBUG_WITH_TYPE(IMPORTVERILOG_DEBUG_TYPE, { + if(compLoc) + llvm::dbgs() << compLoc->file_name() << ":" << compLoc->line() << " (" + << compLoc->function_name() << ") "; + if (srcLoc) { + srcLoc->print(llvm::dbgs()); + llvm::dbgs() << ": "; + } + llvm::dbgs() << buffer << "\n"; + }); + } + + inline llvm::raw_ostream &stream() { return os; } + inline const llvm::raw_ostream &stream() const { return os; } +}; + +// lvalue wrapper +template +inline ImportVerilogDebugStream &operator<<(ImportVerilogDebugStream &s, + const T &v) { + s.stream() << v; + return s; +} + +// rvalue/temporary wrapper +template +inline ImportVerilogDebugStream &&operator<<(ImportVerilogDebugStream &&s, + const T &v) { + s.stream() << v; + return std::move(s); +} + +// support ostream manipulators that look like functions, e.g. write_hex, etc. +inline ImportVerilogDebugStream & +operator<<(ImportVerilogDebugStream &s, + llvm::raw_ostream &(*manip)(llvm::raw_ostream &)) { + manip(s.stream()); + return s; +} +inline ImportVerilogDebugStream && +operator<<(ImportVerilogDebugStream &&s, + llvm::raw_ostream &(*manip)(llvm::raw_ostream &)) { + manip(s.stream()); + return std::move(s); +} + +/// Helper function to set up a debug stream with reasonable defaults +ImportVerilogDebugStream dbgs( + std::optional sourceLocation = {}, + std::optional cl = std::source_location::current()); + +#endif // IMPORT_VERILOG_DEBUG_H diff --git a/lib/Conversion/ImportVerilog/ImportVerilogInternals.h b/lib/Conversion/ImportVerilog/ImportVerilogInternals.h index 2bd1344d3e5f..29cbb4761c8f 100644 --- a/lib/Conversion/ImportVerilog/ImportVerilogInternals.h +++ b/lib/Conversion/ImportVerilog/ImportVerilogInternals.h @@ -10,6 +10,7 @@ #ifndef CONVERSION_IMPORTVERILOG_IMPORTVERILOGINTERNALS_H #define CONVERSION_IMPORTVERILOG_IMPORTVERILOGINTERNALS_H +#include "ImportVerilogDebugStream.h" #include "circt/Conversion/ImportVerilog.h" #include "circt/Dialect/Debug/DebugOps.h" #include "circt/Dialect/HW/HWOps.h" @@ -94,6 +95,12 @@ struct Context { /// Convert a slang `SourceRange` into an MLIR `Location`. Location convertLocation(slang::SourceRange range); + /// A convenient debug stream wrapper which attaches information about input source location and compiler source location; also accepts slang::SourceLocation and tries to unwrap it. + ImportVerilogDebugStream dbgs( + std::optional> + sourceLocation = {}, + std::optional cl = std::source_location::current()); + /// Convert a slang type into an MLIR type. Returns null on failure. Uses the /// provided location for error reporting, or tries to guess one from the /// given type. Types tend to have unreliable location information, so it's From c661d9a58553f1557f1abe0a749077b50ed630bf Mon Sep 17 00:00:00 2001 From: Moritz Scherer Date: Fri, 19 Sep 2025 09:53:39 +0200 Subject: [PATCH 2/2] Add debug prints --- lib/Conversion/ImportVerilog/Expressions.cpp | 4 ++- .../ImportVerilog/HierarchicalNames.cpp | 4 ++- .../ImportVerilog/ImportVerilog.cpp | 9 ++++--- .../ImportVerilog/ImportVerilogDebugStream.h | 18 ++++++------- .../ImportVerilog/ImportVerilogInternals.h | 6 +++-- lib/Conversion/ImportVerilog/Statements.cpp | 22 +++++++++++++--- lib/Conversion/ImportVerilog/Structure.cpp | 25 +++++++++++++------ 7 files changed, 61 insertions(+), 27 deletions(-) diff --git a/lib/Conversion/ImportVerilog/Expressions.cpp b/lib/Conversion/ImportVerilog/Expressions.cpp index ab939e7426a7..4b615a10cf16 100644 --- a/lib/Conversion/ImportVerilog/Expressions.cpp +++ b/lib/Conversion/ImportVerilog/Expressions.cpp @@ -994,8 +994,10 @@ struct RvalueExprVisitor : public ExprVisitor { case (1): value = context.convertRvalueExpression(*args[0]); - if (!value) + if (!value){ + dbgs(loc) << "Failed to convert RValue Expression of call " << subroutine.name; return {}; + } result = context.convertSystemCallArity1(subroutine, loc, value); break; diff --git a/lib/Conversion/ImportVerilog/HierarchicalNames.cpp b/lib/Conversion/ImportVerilog/HierarchicalNames.cpp index 2b20667c5784..627e49691f43 100644 --- a/lib/Conversion/ImportVerilog/HierarchicalNames.cpp +++ b/lib/Conversion/ImportVerilog/HierarchicalNames.cpp @@ -184,8 +184,10 @@ LogicalResult Context::traverseInstanceBody(const slang::ast::Symbol &symbol) { if (auto *instBodySymbol = symbol.as_if()) for (auto &member : instBodySymbol->members()) { auto loc = convertLocation(member.location); - if (failed(member.visit(InstBodyVisitor(*this, loc)))) + if (failed(member.visit(InstBodyVisitor(*this, loc)))) { + dbgs(member.location) << ": Failed to convert symbol " << member.name; return failure(); + } } return success(); } diff --git a/lib/Conversion/ImportVerilog/ImportVerilog.cpp b/lib/Conversion/ImportVerilog/ImportVerilog.cpp index 443745c179fe..068559a0b4c8 100644 --- a/lib/Conversion/ImportVerilog/ImportVerilog.cpp +++ b/lib/Conversion/ImportVerilog/ImportVerilog.cpp @@ -23,7 +23,6 @@ #include "slang/parsing/Preprocessor.h" #include "slang/syntax/SyntaxPrinter.h" #include "slang/util/VersionInfo.h" -#include "ImportVerilogDebugStream.h" using namespace mlir; using namespace circt; @@ -263,8 +262,10 @@ LogicalResult ImportDriver::importVerilog(ModuleOp module) { auto compilation = driver.createCompilation(); for (auto &diag : compilation->getAllDiagnostics()) driver.diagEngine.issue(diag); - if (!parseSuccess || driver.diagEngine.getNumErrors() > 0) + if (!parseSuccess || driver.diagEngine.getNumErrors() > 0) { + dbgs() << "Failed to parse and elaborate inputs!"; return failure(); + } compileTimer.stop(); // If we were only supposed to lint the input, return here. This leaves the @@ -279,8 +280,10 @@ LogicalResult ImportDriver::importVerilog(ModuleOp module) { debug::DebugDialect>(); auto conversionTimer = ts.nest("Verilog to dialect mapping"); Context context(options, *compilation, module, driver.sourceManager); - if (failed(context.convertCompilation())) + if (failed(context.convertCompilation())) { + dbgs() << "Failed to convert Slang compilation!"; return failure(); + } conversionTimer.stop(); // Run the verifier on the constructed module to ensure it is clean. diff --git a/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.h b/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.h index 5ebf2f81c164..71bf5afbb5ff 100644 --- a/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.h +++ b/lib/Conversion/ImportVerilog/ImportVerilogDebugStream.h @@ -21,11 +21,11 @@ // If includer defined DEBUG_TYPE, use that; else fall back to our default. #ifndef IMPORTVERILOG_DEBUG_TYPE - #ifdef DEBUG_TYPE - #define IMPORTVERILOG_DEBUG_TYPE DEBUG_TYPE - #else - #define IMPORTVERILOG_DEBUG_TYPE "import-verilog" - #endif +#ifdef DEBUG_TYPE +#define IMPORTVERILOG_DEBUG_TYPE DEBUG_TYPE +#else +#define IMPORTVERILOG_DEBUG_TYPE "import-verilog" +#endif #endif struct ImportVerilogDebugStream { @@ -40,7 +40,7 @@ struct ImportVerilogDebugStream { inline ~ImportVerilogDebugStream() { DEBUG_WITH_TYPE(IMPORTVERILOG_DEBUG_TYPE, { - if(compLoc) + if (compLoc) llvm::dbgs() << compLoc->file_name() << ":" << compLoc->line() << " (" << compLoc->function_name() << ") "; if (srcLoc) { @@ -86,8 +86,8 @@ operator<<(ImportVerilogDebugStream &&s, } /// Helper function to set up a debug stream with reasonable defaults -ImportVerilogDebugStream dbgs( - std::optional sourceLocation = {}, - std::optional cl = std::source_location::current()); +ImportVerilogDebugStream +dbgs(std::optional sourceLocation = {}, + std::optional cl = std::source_location::current()); #endif // IMPORT_VERILOG_DEBUG_H diff --git a/lib/Conversion/ImportVerilog/ImportVerilogInternals.h b/lib/Conversion/ImportVerilog/ImportVerilogInternals.h index 29cbb4761c8f..5dbb3fc6ed5d 100644 --- a/lib/Conversion/ImportVerilog/ImportVerilogInternals.h +++ b/lib/Conversion/ImportVerilog/ImportVerilogInternals.h @@ -95,10 +95,12 @@ struct Context { /// Convert a slang `SourceRange` into an MLIR `Location`. Location convertLocation(slang::SourceRange range); - /// A convenient debug stream wrapper which attaches information about input source location and compiler source location; also accepts slang::SourceLocation and tries to unwrap it. + /// A convenient debug stream wrapper which attaches information about input + /// source location and compiler source location; also accepts + /// slang::SourceLocation and tries to unwrap it. ImportVerilogDebugStream dbgs( std::optional> - sourceLocation = {}, + sourceLocation = {}, std::optional cl = std::source_location::current()); /// Convert a slang type into an MLIR type. Returns null on failure. Uses the diff --git a/lib/Conversion/ImportVerilog/Statements.cpp b/lib/Conversion/ImportVerilog/Statements.cpp index dd5673ac0ef8..3c01760c009f 100644 --- a/lib/Conversion/ImportVerilog/Statements.cpp +++ b/lib/Conversion/ImportVerilog/Statements.cpp @@ -9,7 +9,9 @@ #include "ImportVerilogInternals.h" #include "slang/ast/Compilation.h" #include "slang/ast/SystemSubroutine.h" +#include "slang/syntax/AllSyntax.h" #include "llvm/ADT/ScopeExit.h" +#include using namespace mlir; using namespace circt; @@ -138,8 +140,11 @@ struct StmtVisitor { mlir::emitWarning(loc, "unreachable code"); break; } - if (failed(context.convertStatement(*stmt))) + if (failed(context.convertStatement(*stmt))) { + context.dbgs(stmt->syntax->sourceRange().start()) + << "Failed to convert statement " << stmt->syntax->toString(); return failure(); + } } return success(); } @@ -157,17 +162,26 @@ struct StmtVisitor { std::get_if( &call->subroutine)) { auto handled = visitSystemCall(stmt, *call, *info); - if (failed(handled)) + if (failed(handled)) { + context.dbgs(stmt.sourceRange.start()) + << "Failed to convert system call " << stmt.syntax->toString(); return failure(); + } if (handled == true) return success(); } + context.dbgs(stmt.sourceRange.start()) + << "Assuming statement " << stmt.syntax->toString() + << " is not a system task"; } auto value = context.convertRvalueExpression(stmt.expr); - if (!value) + if (!value) { + context.dbgs(stmt.sourceRange.start()) + << "Failed to convert expression statement as RValue " + << stmt.expr.syntax->toString(); return failure(); - + } // Expressions like calls to void functions return a dummy value that has no // uses. If the returned value is trivially dead, remove it. if (auto *defOp = value.getDefiningOp()) diff --git a/lib/Conversion/ImportVerilog/Structure.cpp b/lib/Conversion/ImportVerilog/Structure.cpp index be345773fc67..455394dbeaa8 100644 --- a/lib/Conversion/ImportVerilog/Structure.cpp +++ b/lib/Conversion/ImportVerilog/Structure.cpp @@ -669,23 +669,30 @@ LogicalResult Context::convertCompilation() { for (auto *unit : root.compilationUnits) { for (const auto &member : unit->members()) { auto loc = convertLocation(member.location); - if (failed(member.visit(RootVisitor(*this, loc)))) + if (failed(member.visit(RootVisitor(*this, loc)))) { + dbgs(loc) << "Failed to convert top-level object " << member.name; return failure(); + } } } // Prime the root definition worklist by adding all the top-level modules. SmallVector topInstances; for (auto *inst : root.topInstances) - if (!convertModuleHeader(&inst->body)) + if (!convertModuleHeader(&inst->body)) { + dbgs(inst->location) << ": Failed to convert module header of module " + << inst->name; return failure(); - + } // Convert all the root module definitions. while (!moduleWorklist.empty()) { auto *module = moduleWorklist.front(); moduleWorklist.pop(); - if (failed(convertModuleBody(module))) + if (failed(convertModuleBody(module))) { + dbgs(module->location) + << ": Failed to convert module body of module " << module->name; return failure(); + } } return success(); @@ -1057,8 +1064,10 @@ Context::convertFunction(const slang::ast::SubroutineSymbol &subroutine) { // First get or create the function declaration. auto *lowering = declareFunction(subroutine); - if (!lowering) + if (!lowering) { + dbgs(subroutine.location) << "Failed to lower function " << subroutine.name; return failure(); + } ValueSymbolScope scope(valueSymbols); // Create a function body block and populate it with block arguments. @@ -1101,9 +1110,11 @@ Context::convertFunction(const slang::ast::SubroutineSymbol &subroutine) { valueSymbols.insert(subroutine.returnValVar, returnVar); } - if (failed(convertStatement(subroutine.getBody()))) + if (failed(convertStatement(subroutine.getBody()))) { + dbgs(subroutine.location) + << "Failed to convert body of function " << subroutine.name; return failure(); - + } // If there was no explicit return statement provided by the user, insert a // default one. if (builder.getBlock()) {