From 2ef8f23dfe2dafcb4fc43b7c1ad8e5a3c2fae777 Mon Sep 17 00:00:00 2001 From: Fabian Schuiki 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 | 40 ++++++++++++------ lib/Support/FVInt.cpp | 3 +- test/Conversion/ImportVerilog/basic.sv | 8 +++- test/Conversion/ImportVerilog/errors.sv | 34 --------------- test/Conversion/MooreToCore/basic.mlir | 2 +- test/Dialect/Moore/attrs-error.mlir | 5 +++ test/Dialect/Moore/attrs.mlir | 2 + test/Dialect/Moore/basic.mlir | 14 +++++++ test/Dialect/Moore/errors.mlir | 15 ++++--- 14 files changed, 109 insertions(+), 74 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 { let hasCustomAssemblyFormat = 1; } +def FVIntAttr : Attr($_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(svint.getRawPtr(), numWords); + auto unknown = ArrayRef(svint.getRawPtr() + numWords, numWords); + return FVInt(APInt(svint.getBitWidth(), value), + APInt(svint.getBitWidth(), unknown)); + } + auto value = ArrayRef(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(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( - loc, intType, - APInt(value.getBitWidth(), - ArrayRef(value.getRawPtr(), value.getNumWords()))); + Value result = builder.create(loc, intType, fvint); if (result.getType() != type) result = builder.create(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..668bd69c6e16 100644 --- a/lib/Conversion/MooreToCore/MooreToCore.cpp +++ b/lib/Conversion/MooreToCore/MooreToCore.cpp @@ -183,8 +183,11 @@ struct ConstantOpConv : public OpConversionPattern { LogicalResult matchAndRewrite(ConstantOp op, OpAdaptor adaptor, ConversionPatternRewriter &rewriter) const override { - - rewriter.replaceOpWithNewOp(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( + op, type, rewriter.getIntegerAttr(type, value)); return success(); } }; diff --git a/lib/Dialect/Moore/MooreOps.cpp b/lib/Dialect/Moore/MooreOps.cpp index eae745de174c..6920c404cd93 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,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 index caa2fc61810e..e895923d1a1c 100644 --- 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 index 34ba203ebbfd..1e79f3beabd4 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 @@ -651,7 +657,7 @@ module Expressions; {a, b, c} = a; // CHECK: moore.concat_ref %d, %e : (!moore.ref, !moore.ref) -> {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 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/Conversion/MooreToCore/basic.mlir b/test/Conversion/MooreToCore/basic.mlir index a516525be4a0..31ad9917335e 100644 --- a/test/Conversion/MooreToCore/basic.mlir +++ b/test/Conversion/MooreToCore/basic.mlir @@ -267,7 +267,7 @@ moore.module @Variable() { %b2 = moore.variable %0 : // 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 index 89736fb5b272..fdb90e70eaa0 100644 --- 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 index 3337023dc342..5978e118ecd4 100644 --- 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 index d1a2b3a9db1c..eb7d9b03b678 100644 --- 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> in %refStruct1: !moore.ref> ) { + // 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 index de561d2dcb07..fa8b26bacd4c 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 // ----- @@ -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