Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/circt/Dialect/Moore/MooreAttributes.td
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions include/circt/Dialect/Moore/MooreOps.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 3 additions & 1 deletion include/circt/Dialect/Moore/MooreOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)>,
Comment on lines +427 to 429
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my perspective, I think FVInt can shoulder the creation of moore.constant. So should we still need APInt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree! The builder with APInt is just here for convenience now, it internally calls the builder with FVInt and converts the APInt to an FVInt without any X or Z bits.

];
Expand Down
42 changes: 26 additions & 16 deletions lib/Conversion/ImportVerilog/Expressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions lib/Conversion/MooreToCore/MooreToCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
};
Expand Down
40 changes: 27 additions & 13 deletions lib/Dialect/Moore/MooreOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand All @@ -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();
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/Support/FVInt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion test/Conversion/ImportVerilog/basic.sv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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}};
Expand Down
34 changes: 0 additions & 34 deletions test/Conversion/ImportVerilog/errors.sv
Original file line number Diff line number Diff line change
Expand Up @@ -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
maerhart marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion test/Conversion/MooreToCore/basic.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions test/Dialect/Moore/attrs-error.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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>}
2 changes: 2 additions & 0 deletions test/Dialect/Moore/attrs.mlir
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
14 changes: 14 additions & 0 deletions test/Dialect/Moore/basic.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 10 additions & 5 deletions test/Dialect/Moore/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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

// -----

Expand All @@ -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

Expand All @@ -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

Expand Down
Loading