From 8ff5ec921d65430894a40e00e8dfdb38cacf30e2 Mon Sep 17 00:00:00 2001
From: Fabian Schuiki <fabian@schuiki.ch>
Date: Wed, 7 Aug 2024 17:44:08 -0700
Subject: [PATCH] [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
---
.../circt/Dialect/Moore/MooreAttributes.td | 6 +++
include/circt/Dialect/Moore/MooreOps.h | 1 +
include/circt/Dialect/Moore/MooreOps.td | 4 +-
lib/Conversion/ImportVerilog/Expressions.cpp | 42 +++++++++------
lib/Conversion/MooreToCore/MooreToCore.cpp | 7 ++-
lib/Dialect/Moore/MooreOps.cpp | 53 ++++++++++++++-----
test/Conversion/ImportVerilog/basic.sv | 6 +++
test/Conversion/ImportVerilog/errors.sv | 34 ------------
test/Dialect/Moore/basic.mlir | 16 ++++++
test/Dialect/Moore/errors.mlir | 11 ++--
10 files changed, 111 insertions(+), 69 deletions(-)
diff --git a/include/circt/Dialect/Moore/MooreAttributes.td b/include/circt/Dialect/Moore/MooreAttributes.td
index d184553bf3c4..766bd64d2c08 100644
--- 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
index 5a27457159d8..e9f2773eab66 100644
--- 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
index a0e3018fb1b5..62a60ee18422 100644
--- 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
index 79b23269f417..b9e26c8590ca 100644
--- 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,7 +445,7 @@ struct RvalueExprVisitor {
auto type = context.convertType(*expr.type);
if (!type)
return {};
- return convertSVInt(expr.getValue(), type);
+ return materializeSVInt(expr.getValue(), type);
}
// Handle integer literals.
@@ -443,7 +453,7 @@ struct RvalueExprVisitor {
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
index 6ea5561eb685..47068bc76f69 100644
--- 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
index eae745de174c..cbd7ed7aa197 100644
--- 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
index 34ba203ebbfd..27ed4a208c90 100644
--- 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
index 73c1bfb2a9de..554c36028861 100644
--- 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
index d1a2b3a9db1c..b2fb3a010abf 100644
--- 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
index de561d2dcb07..09965a9d40ca 100644
--- 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
// -----