[Moore] Support four-valued integers in ConstantOp

Extend Moore's `ConstantOp` to use the new `FVIntegerAttr`, which now allows us to materialize four-valued integer constants in the IR. Also adjust ImportVerilog to finally properly captured integer literals with X and Z bits. With this change, `circt-verilog` is now capable of fully parsing the Snitch RISC-V core used as a benchmark in the original LLHD paper at PLDI 2020, and to produce a corresponding blob of IR. Examples of the extended op: moore.constant 0 : i32 moore.constant 2 : i2 moore.constant -2 : i2 moore.constant h123456789ABCDEF0 : i64 moore.constant h123456789ABCDEF0XZ : l72 moore.constant b1010 : i8 moore.constant b1010XZ : l8
llvm · Aug 8, 2024 · 8ff5ec9 · 8ff5ec9
1 parent 67009e2
commit 8ff5ec9
Show file tree

Hide file tree

Showing 10 changed files with 111 additions and 69 deletions.
diff --git a/include/circt/Dialect/Moore/MooreAttributes.td b/include/circt/Dialect/Moore/MooreAttributes.td
@@ -19,4 +19,10 @@ def FVIntegerAttr : AttrDef<MooreDialect, "FVInteger"> {
   let hasCustomAssemblyFormat = 1;
 }
 
+def FVIntAttr : Attr<CPred<"llvm::isa<FVIntegerAttr>($_self)">,
+                     "arbitrary precision four-valued integer attribute"> {
+  let storageType = [{ circt::moore::FVIntegerAttr }];
+  let returnType = [{ circt::FVInt }];
+}
+
 #endif // CIRCT_DIALECT_MOORE_MOOREATTRIBUTES
diff --git a/include/circt/Dialect/Moore/MooreOps.h b/include/circt/Dialect/Moore/MooreOps.h
@@ -14,6 +14,7 @@
 #define CIRCT_DIALECT_MOORE_MOOREOPS_H
 
 #include "circt/Dialect/HW/HWTypes.h"
+#include "circt/Dialect/Moore/MooreAttributes.h"
 #include "circt/Dialect/Moore/MooreDialect.h"
 #include "circt/Dialect/Moore/MooreTypes.h"
 #include "mlir/IR/RegionKindInterface.h"

diff --git a/include/circt/Dialect/Moore/MooreOps.td b/include/circt/Dialect/Moore/MooreOps.td
@@ -9,6 +9,7 @@
 #ifndef CIRCT_DIALECT_MOORE_MOOREOPS
 #define CIRCT_DIALECT_MOORE_MOOREOPS
 
+include "circt/Dialect/Moore/MooreAttributes.td"
 include "circt/Dialect/Moore/MooreDialect.td"
 include "circt/Dialect/Moore/MooreTypes.td"
 include "mlir/IR/OpAsmInterface.td"
@@ -417,12 +418,13 @@ def EventOp : MooreOp<"wait_event", [
 
 def ConstantOp : MooreOp<"constant", [Pure]> {
   let summary = "A constant integer value";
-  let arguments = (ins APIntAttr:$value);
+  let arguments = (ins FVIntAttr:$value);
   let results = (outs IntType:$result);
   let hasCustomAssemblyFormat = 1;
   let hasVerifier = 1;
   let hasFolder = 1;
   let builders = [
+    OpBuilder<(ins "IntType":$type, "const FVInt &":$value)>,
     OpBuilder<(ins "IntType":$type, "const APInt &":$value)>,
     OpBuilder<(ins "IntType":$type, "int64_t":$value)>,
   ];

diff --git a/lib/Conversion/ImportVerilog/Expressions.cpp b/lib/Conversion/ImportVerilog/Expressions.cpp
@@ -14,6 +14,19 @@ using namespace circt;
 using namespace ImportVerilog;
 using moore::Domain;
 
+/// Convert a Slang `SVInt` to a CIRCT `FVInt`.
+static FVInt convertSVIntToFVInt(const slang::SVInt &svint) {
+  if (svint.hasUnknown()) {
+    unsigned numWords = svint.getNumWords() / 2;
+    auto value = ArrayRef<uint64_t>(svint.getRawPtr(), numWords);
+    auto unknown = ArrayRef<uint64_t>(svint.getRawPtr() + numWords, numWords);
+    return FVInt(APInt(svint.getBitWidth(), value),
+                 APInt(svint.getBitWidth(), unknown));
+  }
+  auto value = ArrayRef<uint64_t>(svint.getRawPtr(), svint.getNumWords());
+  return FVInt(APInt(svint.getBitWidth(), value));
+}
+
 // NOLINTBEGIN(misc-no-recursion)
 namespace {
 struct RvalueExprVisitor {
@@ -98,7 +111,7 @@ struct RvalueExprVisitor {
       auto type = context.convertType(*expr.type);
       if (!type)
         return {};
-      return convertSVInt(constant.integer(), type);
+      return materializeSVInt(constant.integer(), type);
     }
 
     // Otherwise some other part of ImportVerilog should have added an MLIR
@@ -411,20 +424,17 @@ struct RvalueExprVisitor {
     return {};
   }
 
-  // Materialize a Slang integer literal as a constant op.
-  Value convertSVInt(const slang::SVInt &value, Type type) {
-    if (value.hasUnknown()) {
-      mlir::emitError(loc, "literals with X or Z bits not supported");
-      return {};
-    }
-    auto intType =
-        moore::IntType::get(context.getContext(), value.getBitWidth(),
-                            value.hasUnknown() ? moore::Domain::FourValued
+  /// Materialize a Slang integer literal as a constant op.
+  Value materializeSVInt(const slang::SVInt &svint, Type type) {
+    auto fvint = convertSVIntToFVInt(svint);
+    bool typeIsFourValued = false;
+    if (auto unpackedType = dyn_cast<moore::UnpackedType>(type))
+      typeIsFourValued = unpackedType.getDomain() == moore::Domain::FourValued;
+    auto intType = moore::IntType::get(
+        context.getContext(), fvint.getBitWidth(),
+        fvint.hasUnknown() || typeIsFourValued ? moore::Domain::FourValued
                                                : moore::Domain::TwoValued);
-    Value result = builder.create<moore::ConstantOp>(
-        loc, intType,
-        APInt(value.getBitWidth(),
-              ArrayRef<uint64_t>(value.getRawPtr(), value.getNumWords())));
+    Value result = builder.create<moore::ConstantOp>(loc, intType, fvint);
     if (result.getType() != type)
       result = builder.create<moore::ConversionOp>(loc, type, result);
     return result;
@@ -435,15 +445,15 @@ struct RvalueExprVisitor {
     auto type = context.convertType(*expr.type);
     if (!type)
       return {};
-    return convertSVInt(expr.getValue(), type);
+    return materializeSVInt(expr.getValue(), type);
   }
 
   // Handle integer literals.
   Value visit(const slang::ast::IntegerLiteral &expr) {
     auto type = context.convertType(*expr.type);
     if (!type)
       return {};
-    return convertSVInt(expr.getValue(), type);
+    return materializeSVInt(expr.getValue(), type);
   }
 
   // Handle concatenations.

diff --git a/lib/Conversion/MooreToCore/MooreToCore.cpp b/lib/Conversion/MooreToCore/MooreToCore.cpp
@@ -183,8 +183,11 @@ struct ConstantOpConv : public OpConversionPattern<ConstantOp> {
   LogicalResult
   matchAndRewrite(ConstantOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
-
-    rewriter.replaceOpWithNewOp<hw::ConstantOp>(op, op.getValueAttr());
+    // Discard unknown bits and map them to 0 for now.
+    auto value = op.getValue().toAPInt(false);
+    auto type = rewriter.getIntegerType(value.getBitWidth());
+    rewriter.replaceOpWithNewOp<hw::ConstantOp>(
+        op, type, rewriter.getIntegerAttr(type, value));
     return success();
   }
 };

diff --git a/lib/Dialect/Moore/MooreOps.cpp b/lib/Dialect/Moore/MooreOps.cpp
@@ -13,6 +13,7 @@
 #include "circt/Dialect/Moore/MooreOps.h"
 #include "circt/Dialect/HW/CustomDirectiveImpl.h"
 #include "circt/Dialect/HW/ModuleImplementation.h"
+#include "circt/Dialect/Moore/MooreAttributes.h"
 #include "circt/Support/CustomDirectiveImpl.h"
 #include "mlir/IR/Builders.h"
 #include "llvm/ADT/SmallString.h"
@@ -385,19 +386,34 @@ void AssignedVarOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
 
 void ConstantOp::print(OpAsmPrinter &p) {
   p << " ";
-  p.printAttributeWithoutType(getValueAttr());
+  const auto &value = getValue();
+  if (value == FVInt(1, 0)) {
+    p << "false";
+  } else if (value == FVInt(1, 1)) {
+    p << "true";
+  } else {
+    printFVInt(p, value);
+  }
   p.printOptionalAttrDict((*this)->getAttrs(), /*elidedAttrs=*/{"value"});
   p << " : ";
   p.printStrippedAttrOrType(getType());
 }
 
 ParseResult ConstantOp::parse(OpAsmParser &parser, OperationState &result) {
-  // Parse the constant value without bit width.
-  APInt value;
+  // Parse the constant value.
+  FVInt value;
   auto valueLoc = parser.getCurrentLocation();
+  if (succeeded(parser.parseOptionalKeyword("false"))) {
+    value = FVInt(1, 0);
+  } else if (succeeded(parser.parseOptionalKeyword("true"))) {
+    value = FVInt(1, 1);
+  } else {
+    if (parseFVInt(parser, value))
+      return failure();
+  }
 
-  if (parser.parseInteger(value) ||
-      parser.parseOptionalAttrDict(result.attributes) || parser.parseColon())
+  // Parse any optional attributes and the `:`.
+  if (parser.parseOptionalAttrDict(result.attributes) || parser.parseColon())
     return failure();
 
   // Parse the result type.
@@ -417,16 +433,21 @@ ParseResult ConstantOp::parse(OpAsmParser &parser, OperationState &result) {
     unsigned neededBits =
         value.isNegative() ? value.getSignificantBits() : value.getActiveBits();
     if (type.getWidth() < neededBits)
-      return parser.emitError(valueLoc,
-                              "constant out of range for result type ")
-             << type;
+      return parser.emitError(valueLoc)
+             << "value requires " << neededBits
+             << " bits, but result type only has " << type.getWidth();
     value = value.trunc(type.getWidth());
   }
 
-  // Build the attribute and op.
-  auto attrType = IntegerType::get(parser.getContext(), type.getWidth());
-  auto attrValue = IntegerAttr::get(attrType, value);
+  // If the constant contains any X or Z bits, the result type must be
+  // four-valued.
+  if (value.hasUnknown() && type.getDomain() != Domain::FourValued)
+    return parser.emitError(valueLoc)
+           << "value contains X or Z bits, but result type " << type
+           << " only allows two-valued bits";
 
+  // Build the attribute and op.
+  auto attrValue = FVIntegerAttr::get(parser.getContext(), value);
   result.addAttribute("value", attrValue);
   result.addTypes(type);
   return success();
@@ -441,12 +462,18 @@ LogicalResult ConstantOp::verify() {
   return success();
 }
 
+void ConstantOp::build(OpBuilder &builder, OperationState &result, IntType type,
+                       const FVInt &value) {
+  assert(type.getWidth() == value.getBitWidth() &&
+         "FVInt width must match type width");
+  build(builder, result, type, FVIntegerAttr::get(builder.getContext(), value));
+}
+
 void ConstantOp::build(OpBuilder &builder, OperationState &result, IntType type,
                        const APInt &value) {
   assert(type.getWidth() == value.getBitWidth() &&
          "APInt width must match type width");
-  build(builder, result, type,
-        builder.getIntegerAttr(builder.getIntegerType(type.getWidth()), value));
+  build(builder, result, type, FVInt(value));
 }
 
 /// This builder allows construction of small signed integers like 0, 1, -1

diff --git a/test/Conversion/ImportVerilog/basic.sv b/test/Conversion/ImportVerilog/basic.sv
@@ -630,6 +630,10 @@ module Expressions;
     c = '0;
     // CHECK: moore.constant -1 : i32
     c = '1;
+    // CHECK: moore.constant hXXXXXXXX : l32
+    f = 'X;
+    // CHECK: moore.constant hZZZZZZZZ : l32
+    f = 'Z;
     // CHECK: moore.constant 42 : i32
     c = 42;
     // CHECK: moore.constant 42 : i19
@@ -638,6 +642,8 @@ module Expressions;
     c = 19'sd42;
     // CHECK: moore.constant 123456789123456789123456789123456789 : i128
     c = 128'd123456789123456789123456789123456789;
+    // CHECK: moore.constant h123XZ : l19
+    f = 19'h123XZ;
     // CHECK: [[TMP1:%.+]] = moore.read %a
     // CHECK: [[TMP2:%.+]] = moore.read %b
     // CHECK: [[TMP3:%.+]] = moore.read %c

diff --git a/test/Conversion/ImportVerilog/errors.sv b/test/Conversion/ImportVerilog/errors.sv
@@ -73,43 +73,9 @@ module Foo;
   initial if (x matches 42) x = y;
 endmodule
 
-// -----
-module Foo;
-  // expected-error @below {{literals with X or Z bits not supported}}
-  logic a = 'x;
-endmodule
-
-// -----
-module Foo;
-  // expected-error @below {{literals with X or Z bits not supported}}
-  logic a = 'z;
-endmodule
-
 // -----
 module Foo;
   int a, b[3];
   // expected-error @below {{unpacked arrays in 'inside' expressions not supported}}
   int c = a inside { b };
 endmodule
-
-// -----
-module Foo;
-  logic a, b;
-  initial begin
-    casez (a)
-    // expected-error @below {{literals with X or Z bits not supported}}
-    1'bz : b = 1'b1;
-    endcase
-  end
-endmodule
-
-// -----
-module Foo;
-  logic a;
-  initial begin
-    // expected-error @below {{literals with X or Z bits not supported}}
-    casez (1'bz)
-    1'bz : a = 1'b1;
-    endcase
-  end
-endmodule
diff --git a/test/Dialect/Moore/basic.mlir b/test/Dialect/Moore/basic.mlir
@@ -140,12 +140,28 @@ moore.module @Expressions(
   // CHECK-SAME: in [[REF_STRUCT1:%.+]] : !moore.ref<struct<{a: i32, b: i32}>>
   in %refStruct1: !moore.ref<struct<{a: i32, b: i32}>>
 ) {
+  // CHECK: moore.constant false : i1
+  moore.constant false : i1
+  // CHECK: moore.constant false : l1
+  moore.constant false : l1
+  // CHECK: moore.constant true : i1
+  moore.constant true : i1
+  // CHECK: moore.constant true : l1
+  moore.constant true : l1
   // CHECK: moore.constant 0 : i32
   moore.constant 0 : i32
   // CHECK: moore.constant -2 : i2
   moore.constant 2 : i2
   // CHECK: moore.constant -2 : i2
   moore.constant -2 : i2
+  // CHECK: moore.constant 1311768467463790320 : i64
+  moore.constant h123456789ABCDEF0 : i64
+  // CHECK: moore.constant h123456789ABCDEF0XZ : l72
+  moore.constant h123456789ABCDEF0XZ : l72
+  // CHECK: moore.constant 10 : i8
+  moore.constant b1010 : i8
+  // CHECK: moore.constant b1010XZ : l8
+  moore.constant b1010XZ : l8
 
   // CHECK: moore.conversion [[A]] : !moore.i32 -> !moore.l32
   moore.conversion %a : !moore.i32 -> !moore.l32

diff --git a/test/Dialect/Moore/errors.mlir b/test/Dialect/Moore/errors.mlir
@@ -53,18 +53,23 @@ moore.module @Foo(out a: !moore.string) {
 
 // -----
 
-// expected-error @below {{constant out of range for result type '!moore.i1'}}
+// expected-error @below {{value requires 6 bits, but result type only has 1}}
 moore.constant 42 : !moore.i1
 
 // -----
 
-// expected-error @below {{constant out of range for result type '!moore.i1'}}
+// expected-error @below {{value requires 2 bits, but result type only has 1}}
 moore.constant -2 : !moore.i1
 
 // -----
 
+// expected-error @below {{value contains X or Z bits, but result type '!moore.i4' only allows two-valued bits}}
+moore.constant b10XZ : !moore.i4
+
+// -----
+
 // expected-error @below {{attribute width 9 does not match return type's width 8}}
-"moore.constant" () {value = 42 : i9} : () -> !moore.i8
+"moore.constant" () {value = #moore.fvint<42 : 9>} : () -> !moore.i8
 
 // -----