[Moore] Support four-valued integers in ConstantOp (#7463)

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 · d8c1f6d · d8c1f6d
1 parent 27968fc
commit d8c1f6d
Show file tree

Hide file tree

Showing 14 changed files with 105 additions and 74 deletions.
diff --git a/include/circt/Dialect/Moore/MooreAttributes.td b/include/circt/Dialect/Moore/MooreAttributes.td
@@ -17,6 +17,8 @@ def FVIntegerAttr : AttrDef<MooreDialect, "FVInteger"> {
   let summary = "An attribute containing a four-valued integer";
   let parameters = (ins "FVInt":$value);
   let hasCustomAssemblyFormat = 1;
+  let convertFromStorage = "$_self.getValue()";
+  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 FVIntegerAttr:$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());
+    // FIXME: 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,21 @@ void AssignedVarOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
 
 void ConstantOp::print(OpAsmPrinter &p) {
   p << " ";
-  p.printAttributeWithoutType(getValueAttr());
+  printFVInt(p, getValue());
   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 (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 +420,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 +449,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/lib/Support/FVInt.cpp b/lib/Support/FVInt.cpp
@@ -146,7 +146,8 @@ llvm::hash_code circt::hash_value(const FVInt &a) {
 
 void circt::printFVInt(AsmPrinter &p, const FVInt &value) {
   SmallString<32> buffer;
-  if (value.isNegative() && (-value).tryToString(buffer)) {
+  if (value.getBitWidth() > 1 && value.isNegative() &&
+      (-value).tryToString(buffer)) {
     p << "-" << buffer;
   } else if (value.tryToString(buffer)) {
     p << buffer;

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
@@ -651,7 +657,7 @@ module Expressions;
     {a, b, c} = a;
     // CHECK: moore.concat_ref %d, %e : (!moore.ref<l32>, !moore.ref<l32>) -> <l64>
     {d, e} = d;
-    // CHECK: [[TMP1:%.+]] = moore.constant false : i1
+    // CHECK: [[TMP1:%.+]] = moore.constant 0 : i1
     // CHECK: [[TMP2:%.+]] = moore.concat [[TMP1]] : (!moore.i1) -> i1
     // CHECK: moore.replicate [[TMP2]] : i1 -> i32
     a = {32{1'b0}};

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/Conversion/MooreToCore/basic.mlir b/test/Conversion/MooreToCore/basic.mlir
@@ -267,7 +267,7 @@ moore.module @Variable() {
   %b2 = moore.variable %0 : <i8>
 
   // CHECK: %true = hw.constant true
-  %1 = moore.constant true : i1
+  %1 = moore.constant 1 : i1
   // CHECK: [[CAST:%.+]] = hw.bitcast %true : (i1) -> i1
   %2 = moore.conversion %1 : !moore.i1 -> !moore.l1
   // CHECK: llhd.sig "l" [[CAST]] : i1

diff --git a/test/Dialect/Moore/attrs-error.mlir b/test/Dialect/Moore/attrs-error.mlir
@@ -2,3 +2,8 @@
 
 // expected-error @below {{integer literal requires at least 6 bits, but attribute specifies only 3}}
 hw.constant false {foo = #moore.fvint<42 : 3>}
+
+// -----
+
+// expected-error @below {{integer literal requires at least 1 bits, but attribute specifies only 0}}
+hw.constant false {foo = #moore.fvint<1 : 0>}
diff --git a/test/Dialect/Moore/attrs.mlir b/test/Dialect/Moore/attrs.mlir
@@ -1,5 +1,7 @@
 // RUN: circt-opt %s | circt-opt | FileCheck %s
 
+// CHECK: #moore.fvint<0 : 0>
+hw.constant false {foo = #moore.fvint<0 : 0>}
 // CHECK: #moore.fvint<42 : 32>
 hw.constant false {foo = #moore.fvint<42 : 32>}
 // CHECK: #moore.fvint<-42 : 32>

diff --git a/test/Dialect/Moore/basic.mlir b/test/Dialect/Moore/basic.mlir
@@ -140,12 +140,26 @@ 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 0 : i0
+  moore.constant 0 : i0
+  // CHECK: moore.constant 0 : i1
+  moore.constant 0 : i1
+  // CHECK: moore.constant 1 : i1
+  moore.constant 1 : i1
   // 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
 
 // -----
 
@@ -74,7 +79,7 @@ moore.yield %0 : i8
 
 // -----
 
-%0 = moore.constant true : i1
+%0 = moore.constant 1 : i1
 %1 = moore.constant 42 : i8
 %2 = moore.constant 42 : i32
 
@@ -87,7 +92,7 @@ moore.conditional %0 : i1 -> i32 {
 
 // -----
 
-%0 = moore.constant true : i1
+%0 = moore.constant 1 : i1
 %1 = moore.constant 42 : i32
 %2 = moore.constant 42 : i8