[ImportVerilog] Convert empty modules and instances (#6743)

Extend the `ImportVerilog` conversion to support empty SystemVerilog
module definitions and instances of such modules.

To do this, add a `ImportVerilogInternals.h` header that defines a
`Context` helper struct to aid in the conversion. Similar to other
parsers and translations, the context has a builder, tracks operations,
and offers entry points to convert different nodes of the Slang AST, and
handles diagnostics and location conversion.

An annoying quirk of Slang is the fact that the top-level declarations
in an SV file are reordered and rearranged. All instantiable nodes, such
as modules, programs, or interfaces, are moved into a separate list of
top-level instances which is unordered. To allow for better testing with
FileCheck and to produce more predictable output, the helper `Context`
tracks an ordered `std::map` of the created top-level MLIR ops, indexed
by their Slang location in the input. This map is used to determine the
insertion point for new modules as the AST is converted, such that the
resulting MLIR ops are essentially in source file order.

This new conversion discards any module ports, and any module members
except for instances and stray semicolons. This is already sufficient to
create module hierarchies, and also to define modules nested in other
modules, which Slang already helpfully resolves to an instance and
outlined module definition.

This commit also adds corresponding `moore.module` and `moore.instance`
operations to capture the empty modules and instances. These will be
extended in the future to deal with all the quirks of Verilog ingestion.

Most of the module hierarchy conversion is in a new `Structure.cpp` file
in the `ImportVerilog` conversion, in preparation for future additions
of statement, expression, and type conversion in separate files.

include/circt/Dialect/Moore/MooreOps.td

@@ -28,4 +28,49 @@ class MIROp<string mnemonic, list<Trait> traits = []> :
 include "circt/Dialect/Moore/MIRExpressions.td"
 include "circt/Dialect/Moore/MIRStatements.td"
 
+//===----------------------------------------------------------------------===//
+// Structure
+//===----------------------------------------------------------------------===//
+
+def SVModuleOp : MooreOp<"module", [
+  IsolatedFromAbove,
+  Symbol,
+  NoTerminator,
+  SingleBlock
+]> {
+  let summary = "A module definition";
+  let description = [{
+    The `moore.module` operation represents a SystemVerilog module, including
+    its name, port list, and the constituent parts that make up its body. The
+    module's body is an SSACFG region, since declarations within SystemVerilog
+    modules generally have to appear before their uses, and dedicated assignment
+    operators are used to make connections after declarations.
+
+    See IEEE 1800-2017 § 3.3 "Modules" and § 23.2 "Module definitions".
+  }];
+
+  let arguments = (ins SymbolNameAttr:$sym_name);
+  let regions = (region SizedRegion<1>:$bodyRegion);
+  let assemblyFormat = [{
+    $sym_name attr-dict-with-keyword $bodyRegion
+  }];
+}
+
+def InstanceOp : MooreOp<"instance", [
+  DeclareOpInterfaceMethods<SymbolUserOpInterface>
+]> {
+  let summary = "Create an instance of a module";
+  let description = [{
+    The `moore.instance` operation instantiates a `moore.module` operation.
+
+    See IEEE 1800-2017 § 23.3 "Module instances".
+  }];
+
+  let arguments = (ins StrAttr:$instanceName,
+                       FlatSymbolRefAttr:$moduleName);
+  let assemblyFormat = [{
+    $instanceName $moduleName attr-dict
+  }];
+}
+
 #endif // CIRCT_DIALECT_MOORE_MOOREOPS

lib/Conversion/ImportVerilog/CMakeLists.txt

@@ -30,11 +30,13 @@ endif ()
 
 add_circt_translation_library(CIRCTImportVerilog
   ImportVerilog.cpp
+  Structure.cpp
 
   DEPENDS
   slang_slang
 
   LINK_LIBS PUBLIC
+  CIRCTMoore
   MLIRTranslateLib
   PRIVATE
   slang_slang

lib/Conversion/ImportVerilog/ImportVerilog.cpp

@@ -10,8 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "circt/Conversion/ImportVerilog.h"
-#include "mlir/IR/BuiltinOps.h"
+#include "ImportVerilogInternals.h"
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/Diagnostics.h"
 #include "mlir/IR/Verifier.h"
@@ -28,6 +27,7 @@
 
 using namespace mlir;
 using namespace circt;
+using namespace ImportVerilog;
 
 using llvm::SourceMgr;
 
@@ -60,6 +60,10 @@ convertLocation(MLIRContext *context, const slang::SourceManager &sourceManager,
   return UnknownLoc::get(context);
 }
 
+Location Context::convertLocation(slang::SourceLocation loc) {
+  return ::convertLocation(getContext(), sourceManager, bufferFilePaths, loc);
+}
+
 namespace {
 /// A converter that can be plugged into a slang `DiagnosticEngine` as a client
 /// that will map slang diagnostics to their MLIR counterpart and emit them.
@@ -145,9 +149,9 @@ struct DenseMapInfo<slang::BufferID> {
 namespace {
 const static ImportVerilogOptions defaultOptions;
 
-struct ImportContext {
-  ImportContext(MLIRContext *mlirContext, TimingScope &ts,
-                const ImportVerilogOptions *options)
+struct ImportDriver {
+  ImportDriver(MLIRContext *mlirContext, TimingScope &ts,
+               const ImportVerilogOptions *options)
       : mlirContext(mlirContext), ts(ts),
         options(options ? *options : defaultOptions) {}
 
@@ -177,8 +181,8 @@ struct ImportContext {
 
 /// Populate the Slang driver with source files from the given `sourceMgr`, and
 /// configure driver options based on the `ImportVerilogOptions` passed to the
-/// `ImportContext` constructor.
-LogicalResult ImportContext::prepareDriver(SourceMgr &sourceMgr) {
+/// `ImportDriver` constructor.
+LogicalResult ImportDriver::prepareDriver(SourceMgr &sourceMgr) {
   // Use slang's driver which conveniently packages a lot of the things we
   // need for compilation.
   auto diagClient =
@@ -235,7 +239,7 @@ LogicalResult ImportContext::prepareDriver(SourceMgr &sourceMgr) {
 
 /// Parse and elaborate the prepared source files, and populate the given MLIR
 /// `module` with corresponding operations.
-LogicalResult ImportContext::importVerilog(ModuleOp module) {
+LogicalResult ImportDriver::importVerilog(ModuleOp module) {
   // Parse the input.
   auto parseTimer = ts.nest("Verilog parser");
   bool parseSuccess = driver.parseAllSources();
@@ -250,8 +254,13 @@ LogicalResult ImportContext::importVerilog(ModuleOp module) {
     return failure();
   compileTimer.stop();
 
-  // TODO: Traverse the parsed Verilog AST and map it to the equivalent CIRCT
-  // ops.
+  // Traverse the parsed Verilog AST and map it to the equivalent CIRCT ops.
+  mlirContext->loadDialect<moore::MooreDialect>();
+  auto conversionTimer = ts.nest("Verilog to dialect mapping");
+  Context context(module, driver.sourceManager, bufferFilePaths);
+  if (failed(context.convertCompilation(*compilation)))
+    return failure();
+  conversionTimer.stop();
 
   // Run the verifier on the constructed module to ensure it is clean.
   auto verifierTimer = ts.nest("Post-parse verification");
@@ -260,7 +269,7 @@ LogicalResult ImportContext::importVerilog(ModuleOp module) {
 
 /// Preprocess the prepared source files and print them to the given output
 /// stream.
-LogicalResult ImportContext::preprocessVerilog(llvm::raw_ostream &os) {
+LogicalResult ImportDriver::preprocessVerilog(llvm::raw_ostream &os) {
   auto parseTimer = ts.nest("Verilog preprocessing");
 
   // Run the preprocessor to completion across all sources previously added with
@@ -337,10 +346,10 @@ LogicalResult circt::importVerilog(SourceMgr &sourceMgr,
                                    ModuleOp module,
                                    const ImportVerilogOptions *options) {
   return catchExceptions([&] {
-    ImportContext context(mlirContext, ts, options);
-    if (failed(context.prepareDriver(sourceMgr)))
+    ImportDriver importDriver(mlirContext, ts, options);
+    if (failed(importDriver.prepareDriver(sourceMgr)))
       return failure();
-    return context.importVerilog(module);
+    return importDriver.importVerilog(module);
   });
 }
 
@@ -351,10 +360,10 @@ LogicalResult circt::preprocessVerilog(SourceMgr &sourceMgr,
                                        TimingScope &ts, llvm::raw_ostream &os,
                                        const ImportVerilogOptions *options) {
   return catchExceptions([&] {
-    ImportContext context(mlirContext, ts, options);
-    if (failed(context.prepareDriver(sourceMgr)))
+    ImportDriver importDriver(mlirContext, ts, options);
+    if (failed(importDriver.prepareDriver(sourceMgr)))
       return failure();
-    return context.preprocessVerilog(os);
+    return importDriver.preprocessVerilog(os);
   });
 }
 

lib/Conversion/ImportVerilog/ImportVerilogInternals.h

@@ -0,0 +1,72 @@
+//===- ImportVerilogInternals.h - Internal implementation details ---------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// NOLINTNEXTLINE(llvm-header-guard)
+#ifndef CONVERSION_IMPORTVERILOG_IMPORTVERILOGINTERNALS_H
+#define CONVERSION_IMPORTVERILOG_IMPORTVERILOGINTERNALS_H
+
+#include "circt/Conversion/ImportVerilog.h"
+#include "circt/Dialect/Moore/MooreOps.h"
+#include "slang/ast/ASTVisitor.h"
+#include "llvm/Support/Debug.h"
+#include <map>
+#include <queue>
+
+#define DEBUG_TYPE "import-verilog"
+
+namespace circt {
+namespace ImportVerilog {
+
+/// A helper class to facilitate the conversion from a Slang AST to MLIR
+/// operations. Keeps track of the destination MLIR module, builders, and
+/// various worklists and utilities needed for conversion.
+struct Context {
+  Context(mlir::ModuleOp intoModuleOp,
+          const slang::SourceManager &sourceManager,
+          SmallDenseMap<slang::BufferID, StringRef> &bufferFilePaths)
+      : intoModuleOp(intoModuleOp), sourceManager(sourceManager),
+        bufferFilePaths(bufferFilePaths),
+        builder(OpBuilder::atBlockEnd(intoModuleOp.getBody())),
+        symbolTable(intoModuleOp) {}
+  Context(const Context &) = delete;
+
+  /// Return the MLIR context.
+  MLIRContext *getContext() { return intoModuleOp.getContext(); }
+
+  /// Convert a slang `SourceLocation` into an MLIR `Location`.
+  Location convertLocation(slang::SourceLocation loc);
+
+  /// Convert hierarchy and structure AST nodes to MLIR ops.
+  LogicalResult convertCompilation(slang::ast::Compilation &compilation);
+  moore::SVModuleOp
+  convertModuleHeader(const slang::ast::InstanceBodySymbol *module);
+  LogicalResult convertModuleBody(const slang::ast::InstanceBodySymbol *module);
+
+  mlir::ModuleOp intoModuleOp;
+  const slang::SourceManager &sourceManager;
+  SmallDenseMap<slang::BufferID, StringRef> &bufferFilePaths;
+
+  /// The builder used to create IR operations.
+  OpBuilder builder;
+  /// A symbol table of the MLIR module we are emitting into.
+  SymbolTable symbolTable;
+
+  /// The top-level operations ordered by their Slang source location. This is
+  /// used to produce IR that follows the source file order.
+  std::map<slang::SourceLocation, Operation *> orderedRootOps;
+  /// How we have lowered modules to MLIR.
+  DenseMap<const slang::ast::InstanceBodySymbol *, moore::SVModuleOp> moduleOps;
+  /// A list of modules for which the header has been created, but the body has
+  /// not been converted yet.
+  std::queue<const slang::ast::InstanceBodySymbol *> moduleWorklist;
+};
+
+} // namespace ImportVerilog
+} // namespace circt
+
+#endif // CONVERSION_IMPORTVERILOG_IMPORTVERILOGINTERNALS_H

lib/Conversion/ImportVerilog/Structure.cpp

@@ -0,0 +1,161 @@
+//===- Structure.cpp - Slang hierarchy conversion -------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "ImportVerilogInternals.h"
+#include "slang/ast/Compilation.h"
+
+using namespace circt;
+using namespace ImportVerilog;
+
+//===----------------------------------------------------------------------===//
+// Module Member Conversion
+//===----------------------------------------------------------------------===//
+
+namespace {
+struct MemberVisitor {
+  Context &context;
+  Location loc;
+  OpBuilder &builder;
+
+  MemberVisitor(Context &context, Location loc)
+      : context(context), loc(loc), builder(context.builder) {}
+
+  /// Skip semicolons.
+  LogicalResult visit(const slang::ast::EmptyMemberSymbol &) {
+    return success();
+  }
+
+  LogicalResult visit(const slang::ast::InstanceSymbol &instNode) {
+    auto targetModule = context.convertModuleHeader(&instNode.body);
+    if (!targetModule)
+      return failure();
+
+    builder.create<moore::InstanceOp>(
+        loc, builder.getStringAttr(instNode.name),
+        FlatSymbolRefAttr::get(targetModule.getSymNameAttr()));
+
+    return success();
+  }
+
+  /// Emit an error for all other members.
+  template <typename T>
+  LogicalResult visit(T &&node) {
+    mlir::emitError(loc, "unsupported construct: ")
+        << slang::ast::toString(node.kind);
+    return failure();
+  }
+};
+} // namespace
+
+//===----------------------------------------------------------------------===//
+// Structure and Hierarchy Conversion
+//===----------------------------------------------------------------------===//
+
+/// Convert an entire Slang compilation to MLIR ops. This is the main entry
+/// point for the conversion.
+LogicalResult
+Context::convertCompilation(slang::ast::Compilation &compilation) {
+  const auto &root = compilation.getRoot();
+
+  // Visit all top-level declarations in all compilation units. This does not
+  // include instantiable constructs like modules, interfaces, and programs,
+  // which are listed separately as top instances.
+  for (auto *unit : root.compilationUnits) {
+    for (const auto &member : unit->members()) {
+      // Error out on all top-level declarations.
+      auto loc = convertLocation(member.location);
+      return mlir::emitError(loc, "unsupported construct: ")
+             << slang::ast::toString(member.kind);
+    }
+  }
+
+  // Prime the root definition worklist by adding all the top-level modules.
+  SmallVector<const slang::ast::InstanceSymbol *> topInstances;
+  for (auto *inst : root.topInstances)
+    convertModuleHeader(&inst->body);
+
+  // Convert all the root module definitions.
+  while (!moduleWorklist.empty()) {
+    auto *module = moduleWorklist.front();
+    moduleWorklist.pop();
+    if (failed(convertModuleBody(module)))
+      return failure();
+  }
+
+  return success();
+}
+
+/// Convert a module and its ports to an empty module op in the IR. Also adds
+/// the op to the worklist of module bodies to be lowered. This acts like a
+/// module "declaration", allowing instances to already refer to a module even
+/// before its body has been lowered.
+moore::SVModuleOp
+Context::convertModuleHeader(const slang::ast::InstanceBodySymbol *module) {
+  if (auto op = moduleOps.lookup(module))
+    return op;
+  auto loc = convertLocation(module->location);
+  OpBuilder::InsertionGuard g(builder);
+
+  // We only support modules for now. Extension to interfaces and programs
+  // should be trivial though, since they are essentially the same thing with
+  // only minor differences in semantics.
+  if (module->getDefinition().definitionKind !=
+      slang::ast::DefinitionKind::Module) {
+    mlir::emitError(loc, "unsupported construct: ")
+        << module->getDefinition().getKindString();
+    return {};
+  }
+
+  // Handle the port list.
+  for (auto *symbol : module->getPortList()) {
+    auto portLoc = convertLocation(symbol->location);
+    mlir::emitError(portLoc, "unsupported module port: ")
+        << slang::ast::toString(symbol->kind);
+    return {};
+  }
+
+  // Pick an insertion point for this module according to the source file
+  // location.
+  auto it = orderedRootOps.lower_bound(module->location);
+  if (it == orderedRootOps.end())
+    builder.setInsertionPointToEnd(intoModuleOp.getBody());
+  else
+    builder.setInsertionPoint(it->second);
+
+  // Create an empty module that corresponds to this module.
+  auto moduleOp = builder.create<moore::SVModuleOp>(loc, module->name);
+  orderedRootOps.insert(it, {module->location, moduleOp});
+  moduleOp.getBodyRegion().emplaceBlock();
+
+  // Add the module to the symbol table of the MLIR module, which uniquifies its
+  // name as we'd expect.
+  symbolTable.insert(moduleOp);
+
+  // Schedule the body to be lowered.
+  moduleWorklist.push(module);
+  moduleOps.insert({module, moduleOp});
+  return moduleOp;
+}
+
+/// Convert a module's body to the corresponding IR ops. The module op must have
+/// already been created earlier through a `convertModuleHeader` call.
+LogicalResult
+Context::convertModuleBody(const slang::ast::InstanceBodySymbol *module) {
+  auto moduleOp = moduleOps.lookup(module);
+  assert(moduleOp);
+  OpBuilder::InsertionGuard g(builder);
+  builder.setInsertionPointToEnd(moduleOp.getBody());
+
+  for (auto &member : module->members()) {
+    auto loc = convertLocation(member.location);
+    if (failed(member.visit(MemberVisitor(*this, loc))))
+      return failure();
+  }
+
+  return success();
+}

lib/Dialect/Moore/MooreOps.cpp

@@ -16,6 +16,26 @@
 using namespace circt;
 using namespace circt::moore;
 
+//===----------------------------------------------------------------------===//
+// InstanceOp
+//===----------------------------------------------------------------------===//
+
+LogicalResult InstanceOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
+  auto *module =
+      symbolTable.lookupNearestSymbolFrom(*this, getModuleNameAttr());
+  if (module == nullptr)
+    return emitError("unknown symbol name '") << getModuleName() << "'";
+
+  // It must be some sort of module.
+  if (!isa<SVModuleOp>(module))
+    return emitError("symbol '")
+           << getModuleName()
+           << "' must reference a 'moore.module', but got a '"
+           << module->getName() << "' instead";
+
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // ConcatOp
 //===----------------------------------------------------------------------===//

test/Conversion/ImportVerilog/basic.sv

@@ -4,7 +4,35 @@
 // Internal issue in Slang v3 about jump depending on uninitialised value.
 // UNSUPPORTED: valgrind
 
-// CHECK: module {
-// CHECK: }
-module Foo;
+// CHECK-LABEL: moore.module @Empty {
+// CHECK:       }
+module Empty;
+  ; // empty member
+endmodule
+
+// CHECK-LABEL: moore.module @NestedA {
+// CHECK:         moore.instance "NestedB" @NestedB
+// CHECK:       }
+// CHECK-LABEL: moore.module @NestedB {
+// CHECK:         moore.instance "NestedC" @NestedC
+// CHECK:       }
+// CHECK-LABEL: moore.module @NestedC {
+// CHECK:       }
+module NestedA;
+  module NestedB;
+    module NestedC;
+    endmodule
+  endmodule
+endmodule
+
+// CHECK-LABEL: moore.module @Child {
+// CHECK:       }
+module Child;
+endmodule
+
+// CHECK-LABEL: moore.module @Parent
+// CHECK:         moore.instance "child" @Child
+// CHECK:       }
+module Parent;
+  Child child();
 endmodule

test/Conversion/ImportVerilog/errors.sv

@@ -20,9 +20,33 @@ endmodule
 
 // -----
 
-module Baz;
+module Foo;
   mailbox a;
   string b;
   // expected-error @below {{value of type 'string' cannot be assigned to type 'mailbox'}}
   initial a = b;
 endmodule
+
+// -----
+
+module Foo;
+  // expected-error @below {{unsupported construct}}
+  genvar a;
+endmodule
+
+// -----
+
+module Foo(
+  // expected-error @below {{unsupported module port}}
+  input a
+);
+endmodule
+
+// -----
+
+// expected-error @below {{unsupported construct}}
+package Foo;
+endpackage
+
+module Bar;
+endmodule

test/Dialect/Moore/basic.mlir

@@ -1,5 +1,15 @@
 // RUN: circt-opt %s -verify-diagnostics | circt-opt -verify-diagnostics | FileCheck %s
 
+// CHECK-LABEL: moore.module @Foo
+moore.module @Foo {
+  // CHECK: moore.instance "foo" @Foo
+  moore.instance "foo" @Foo
+}
+
+// CHECK-LABEL: moore.module @Bar
+moore.module @Bar {
+}
+
 // CHECK-LABEL: llhd.entity @test1
 llhd.entity @test1() -> () {
   // CHECK-NEXT: [[CONST:%.*]] = moore.mir.constant 5 : !moore.int

test/Dialect/Moore/errors.mlir

@@ -0,0 +1,10 @@
+// RUN: circt-opt %s --verify-diagnostics --split-input-file
+
+func.func @Foo() {
+  return
+}
+
+moore.module @Bar {
+  // expected-error @below {{symbol 'Foo' must reference a 'moore.module', but got a 'func.func' instead}}
+  moore.instance "foo" @Foo
+}