[FIRRTL] LowerIntrinsics: rewrite to lower generic ops.

Move to module pass, and use rewriting framework for safety,
familiariy, and (with some help) simpler lowering code.

Add LowerIntmodules to the pipeline as this is now needed
for compatibility with designs not yet using the new intrinsic format.

include/circt/Dialect/FIRRTL/FIRRTLIntrinsics.h

@@ -1,4 +1,4 @@
-//===- FIRRTLDialect.h - FIRRTL dialect declaration ------------*- C++ --*-===//
+//===- FIRRTLIntrinsics.h - FIRRTL intrinsics ------------------*- C++ --*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,105 +13,228 @@
 #ifndef CIRCT_DIALECT_FIRRTL_FIRRTLINTRINSICS_H
 #define CIRCT_DIALECT_FIRRTL_FIRRTLINTRINSICS_H
 
-#include "circt/Dialect/FIRRTL/FIRRTLOpInterfaces.h"
 #include "circt/Dialect/FIRRTL/FIRRTLOps.h"
+#include "circt/Dialect/FIRRTL/FIRRTLTypes.h"
+#include "mlir/IR/Diagnostics.h"
+#include "mlir/Transforms/DialectConversion.h"
 
 namespace circt {
 namespace firrtl {
-class InstanceGraph;
 
-/// Base class for Intrinsic Converters.
-///
-/// Intrinsic converters contain validation logic, along with a converter
-/// method to transform instances to extmodules/intmodules into ops.
-class IntrinsicConverter {
-protected:
-  StringRef name;
-  FModuleLike mod;
+/// Helper class for checking and extracting information from the generic
+/// instrinsic op.
+struct GenericIntrinsic {
+  GenericIntrinsicOp op;
 
-public:
-  IntrinsicConverter(StringRef name, FModuleLike mod) : name(name), mod(mod) {}
+  GenericIntrinsic(GenericIntrinsicOp op) : op(op) {}
 
-  virtual ~IntrinsicConverter();
+  InFlightDiagnostic emitError() { return op.emitError(op.getIntrinsic()); }
 
-  /// Checks whether the intrinsic module is well-formed.
-  ///
-  /// This or's multiple ParseResults together, returning true on failure.
-  virtual bool check() { return false; }
+  //===--------------------------------------------------------------------===//
+  // Input checking
+  //===--------------------------------------------------------------------===//
+
+  ParseResult hasNInputs(unsigned n);
+
+  template <typename C>
+  ParseResult checkInputType(unsigned n, const Twine &msg, C &&call) {
+    if (n >= op.getNumOperands())
+      return emitError() << " missing input " << n;
+    if (!std::invoke(std::forward<C>(call), op.getOperand(n).getType()))
+      return emitError() << " input " << n << " " << msg;
+    return success();
+  }
 
-  /// Transform an instance of the intrinsic.
-  virtual LogicalResult convert(InstanceOp op) = 0;
+  template <typename C>
+  ParseResult checkInputType(unsigned n, C &&call) {
+    return checkInputType(n, "not of correct type", std::forward<C>(call));
+  }
 
-protected:
-  ParseResult hasNPorts(unsigned n);
+  template <typename T>
+  ParseResult typedInput(unsigned n) {
+    return checkInputType(n, [](auto ty) { return isa<T>(ty); });
+  }
 
-  ParseResult namedPort(unsigned n, StringRef portName);
+  ParseResult hasResetInput(unsigned n) {
+    return checkInputType(n, "must be reset type", [](auto ty) {
+      auto baseType = dyn_cast<FIRRTLBaseType>(ty);
+      return baseType && baseType.isResetType();
+    });
+  }
+
+  template <typename T>
+  ParseResult sizedInput(unsigned n, int32_t size) {
+    return checkInputType(n, "not size " + Twine(size), [size](auto ty) {
+      auto t = dyn_cast<T>(ty);
+      return t && t.getWidth() == size;
+    });
+  }
 
-  ParseResult resetPort(unsigned n);
+  //===--------------------------------------------------------------------===//
+  // Parameter checking
+  //===--------------------------------------------------------------------===//
 
   ParseResult hasNParam(unsigned n, unsigned c = 0);
 
   ParseResult namedParam(StringRef paramName, bool optional = false);
 
   ParseResult namedIntParam(StringRef paramName, bool optional = false);
 
+  ParamDeclAttr getParamByName(StringRef name) {
+    for (auto param : op.getParameters().getAsRange<ParamDeclAttr>())
+      if (param.getName().getValue().equals(name))
+        return param;
+    return {};
+  }
+
+  /// Get parameter value by name, if present, as requested type.
   template <typename T>
-  ParseResult typedPort(unsigned n) {
-    auto ports = mod.getPorts();
-    if (n >= ports.size()) {
-      mod.emitError(name) << " missing port " << n;
-      return failure();
-    }
-    if (!isa<T>(ports[n].type)) {
-      mod.emitError(name) << " port " << n << " not of correct type";
-      return failure();
-    }
+  T getParamValue(StringRef name) {
+    auto p = getParamByName(name);
+    if (!p)
+      return {};
+    return cast<T>(p.getValue());
+  }
+
+  //===--------------------------------------------------------------------===//
+  // Output checking
+  //===--------------------------------------------------------------------===//
+
+  ParseResult hasOutput() {
+    if (op.getNumResults() == 0)
+      return emitError() << " missing output";
+    return success();
+  }
+  ParseResult hasNoOutput() {
+    if (op.getNumResults() != 0)
+      return emitError() << " should not have outputs";
     return success();
   }
 
   template <typename T>
-  ParseResult sizedPort(unsigned n, int32_t size) {
-    auto ports = mod.getPorts();
-    if (failed(typedPort<T>(n)))
+  ParseResult typedOutput() {
+    if (failed(hasOutput()))
       return failure();
-    if (cast<T>(ports[n].type).getWidth() != size) {
-      mod.emitError(name) << " port " << n << " not size " << size;
+    if (!isa<T>(op.getResult().getType()))
+      return emitError() << " output not of correct type";
+    return success();
+  }
+
+  template <typename T>
+  ParseResult sizedOutput(int32_t size) {
+    if (failed(typedOutput<T>()))
       return failure();
-    }
+    if (cast<T>(op.getResult().getType()).getWidth() != size)
+      return emitError() << " output not size " << size;
+    return success();
+  }
+
+  //===--------------------------------------------------------------------===//
+  // Output bundle element checking
+  //===--------------------------------------------------------------------===//
+
+  mlir::TypedValue<BundleType> getOutputBundle() {
+    return dyn_cast_or_null<mlir::TypedValue<BundleType>>(op.getResult());
+  }
+
+  ParseResult hasNOutputElements(unsigned n);
+
+  template <typename C>
+  ParseResult checkOutputElement(unsigned n, StringRef name, const Twine &msg,
+                                 C &&call) {
+    auto b = getOutputBundle();
+    if (!b)
+      return emitError() << " missing output bundle";
+    auto ty = b.getType();
+    if (n >= ty.getNumElements())
+      return emitError() << " missing output element " << n;
+    auto element = ty.getElement(n);
+    if (element.name != name)
+      return emitError() << " output element " << n << " is named "
+                         << element.name << " not " << name;
+    if (!std::invoke(std::forward<C>(call), element.type))
+      return emitError() << " output element " << n << " " << msg;
     return success();
   }
+
+  template <typename C>
+  ParseResult checkOutputElement(unsigned n, StringRef name, C &&call) {
+    return checkOutputElement(n, name, "not of correct type",
+                              std::forward<C>(call));
+  }
+
+  ParseResult hasOutputElement(unsigned n, StringRef name) {
+    return checkOutputElement(n, name, [](auto ty) { return true; });
+  }
+
+  template <typename T>
+  ParseResult typedOutputElement(unsigned n, StringRef name) {
+    return checkOutputElement(n, name, [](auto ty) { return isa<T>(ty); });
+  }
+
+  template <typename T>
+  ParseResult sizedOutputElement(unsigned n, StringRef name, int32_t size) {
+    return checkOutputElement(n, name, "not size " + Twine(size),
+                              [size](auto ty) {
+                                auto t = dyn_cast<T>(ty);
+                                return t && t.getWidth() == size;
+                              });
+  }
+};
+
+/// Base class for Intrinsic Converters.
+///
+/// Intrinsic converters contain validation logic, along with a converter
+/// method to transform generic intrinsic ops to their implementation.
+class IntrinsicConverter {
+public:
+  virtual ~IntrinsicConverter() = default;
+
+  /// Checks whether the intrinsic is well-formed.
+  ///
+  /// This or's multiple ParseResults together, returning true on failure.
+  virtual bool check(GenericIntrinsic gi) = 0;
+
+  /// Transform the intrinsic to its implementation.
+  virtual void convert(GenericIntrinsic gi, GenericIntrinsicOpAdaptor adaptor,
+                       PatternRewriter &rewriter) = 0;
+};
+
+template <typename OpTy>
+class IntrinsicOpConverter : public IntrinsicConverter {
+public:
+  /// Transform the intrinsic to its implementation.
+  /// Handles the simple case of just forwarding to new op kind.
+  void convert(GenericIntrinsic gi, GenericIntrinsicOpAdaptor adaptor,
+               PatternRewriter &rewriter) final {
+    // Pass along result type and operands.  No attributes.
+    rewriter.replaceOpWithNewOp<OpTy>(gi.op, gi.op.getResultTypes(),
+                                      adaptor.getOperands());
+  }
 };
 
 /// Lowering helper which collects all intrinsic converters.
 class IntrinsicLowerings {
+public:
+  using ConversionMapTy =
+      llvm::DenseMap<StringAttr, std::unique_ptr<IntrinsicConverter>>;
+
 private:
-  using ConverterFn = std::function<LogicalResult(FModuleLike)>;
+  using ConverterFn = std::function<LogicalResult(GenericIntrinsicOp)>;
 
   /// Reference to the MLIR context.
   MLIRContext *context;
 
-  /// Reference to the instance graph to find module instances.
-  InstanceGraph &graph;
-
   /// Mapping from intrinsic names to converters.
-  DenseMap<StringAttr, ConverterFn> intmods;
-  /// Mapping from extmodule names to converters.
-  DenseMap<StringAttr, ConverterFn> extmods;
+  ConversionMapTy conversions;
 
 public:
-  IntrinsicLowerings(MLIRContext *context, InstanceGraph &graph)
-      : context(context), graph(graph) {}
+  IntrinsicLowerings(MLIRContext *context) : context(context) {}
 
   /// Registers a converter to an intrinsic name.
   template <typename T>
   void add(StringRef name) {
-    addConverter<T>(intmods, name);
-  }
-
-  /// Registers a converter to an extmodule name.
-  template <typename T>
-  void addExtmod(StringRef name) {
-    addConverter<T>(extmods, name);
+    addConverter<T>(name);
   }
 
   /// Registers a converter to multiple intrinsic names.
@@ -121,26 +244,18 @@ class IntrinsicLowerings {
     add<T>(args...);
   }
 
-  /// Lowers a module to an intrinsic, given an intrinsic name.
-  LogicalResult lower(CircuitOp circuit, bool allowUnknownIntrinsics = false);
-
-  /// Return the number of intrinsics converted.
-  unsigned getNumConverted() const { return numConverted; }
+  /// Lowers all intrinsics in a module.
+  LogicalResult lower(FModuleOp mod, bool allowUnknownIntrinsics = false);
 
 private:
   template <typename T>
-  void addConverter(DenseMap<StringAttr, ConverterFn> &map, StringRef name) {
+  typename std::enable_if_t<std::is_base_of_v<IntrinsicConverter, T>>
+  addConverter(StringRef name) {
     auto nameAttr = StringAttr::get(context, name);
-    map.try_emplace(nameAttr,
-                    [this, nameAttr](FModuleLike mod) -> LogicalResult {
-                      T conv(nameAttr.getValue(), mod);
-                      return doLowering(mod, conv);
-                    });
+    assert(!conversions.contains(nameAttr) &&
+           "duplicate conversion for intrinsic");
+    conversions.try_emplace(nameAttr, std::make_unique<T>());
   }
-
-  LogicalResult doLowering(FModuleLike mod, IntrinsicConverter &conv);
-
-  unsigned numConverted = 0;
 };
 
 } // namespace firrtl

include/circt/Dialect/FIRRTL/Passes.h

@@ -63,8 +63,7 @@ std::unique_ptr<mlir::Pass> createLowerCHIRRTLPass();
 std::unique_ptr<mlir::Pass>
 createLowerIntmodulesPass(bool fixupEICGWrapper = false);
 
-std::unique_ptr<mlir::Pass>
-createLowerIntrinsicsPass(bool fixupEICGWrapper = false);
+std::unique_ptr<mlir::Pass> createLowerIntrinsicsPass();
 
 std::unique_ptr<mlir::Pass> createIMConstPropPass();
 

include/circt/Dialect/FIRRTL/Passes.td

@@ -697,17 +697,12 @@ def LowerIntmodules : Pass<"firrtl-lower-intmodules", "firrtl::CircuitOp"> {
   ];
 }
 
-def LowerIntrinsics : Pass<"firrtl-lower-intrinsics", "firrtl::CircuitOp"> {
+def LowerIntrinsics : Pass<"firrtl-lower-intrinsics", "firrtl::FModuleOp"> {
   let summary = "Lower intrinsics";
   let description = [{
-    This pass lowers intrinsics encoded as extmodule with annotation and
-    intmodule to their implementation or op.
+    This pass lowers generic intrinsic ops to their implementation or op.
   }];
   let constructor = "circt::firrtl::createLowerIntrinsicsPass()";
-  let options = [
-    Option<"fixupEICGWrapper", "fixup-eicg-wrapper", "bool", "false",
-      "Lower `EICG_wrapper` modules into clock gate intrinsics">,
-  ];
 }
 
 def LowerOpenAggs : Pass<"firrtl-lower-open-aggs", "firrtl::CircuitOp"> {