From 167f4313873c5174986bb27e9f8d224ae068640c Mon Sep 17 00:00:00 2001 From: Fabian Schuiki Date: Thu, 7 Nov 2024 16:13:25 -0800 Subject: [PATCH] [Moore] Add explicit truncation and zero/sign-extension Add the `moore.trunc` operation to explicitly truncate the bit width of `IntType` values, and `moore.zext` and `moore.sext` to explicitly extend such a value with zeroes or its sign bit. This requires tweaking the way how ImportVerilog generates conversion ops. Currently `moore.conversion` is used as a catch-all operation that expresses any kind of type conversion. In the future, we'll want to split this up into multiple dedicated operations. These width adjustment ops are the first step in that direction. Making sign-extension explicit also fixes a long-standing issue where a `$signed` or `signed'(x)` expression would be erroneously converted into a zero-extension. --- include/circt/Dialect/Moore/MooreOps.td | 62 ++++++++++++++++++- lib/Conversion/ImportVerilog/Expressions.cpp | 53 +++++++++++++--- .../ImportVerilog/ImportVerilogInternals.h | 5 ++ lib/Conversion/MooreToCore/MooreToCore.cpp | 56 ++++++++++++++++- test/Conversion/ImportVerilog/basic.sv | 39 +++++++++--- test/Conversion/MooreToCore/basic.mlir | 43 +++++++++---- 6 files changed, 228 insertions(+), 30 deletions(-) diff --git a/include/circt/Dialect/Moore/MooreOps.td b/include/circt/Dialect/Moore/MooreOps.td index ba9096ffc048..b2375620a5d7 100644 --- a/include/circt/Dialect/Moore/MooreOps.td +++ b/include/circt/Dialect/Moore/MooreOps.td @@ -36,6 +36,11 @@ class ResultIsSingleBitMatchingInputDomain : llvm::cast($_self).getDomain()) }]>; +class TypeDomainsMatch values> : + AllMatchSameOperatorTrait($_self.getType()).getDomain() + }], "domain">; + //===----------------------------------------------------------------------===// // Structure //===----------------------------------------------------------------------===// @@ -504,7 +509,7 @@ def StringConstantOp : MooreOp<"string_constant", [Pure]> { } //===----------------------------------------------------------------------===// -// Expressions +// Casting and Resizing //===----------------------------------------------------------------------===// def ConversionOp : MooreOp<"conversion", [Pure]> { @@ -538,6 +543,61 @@ def ConversionOp : MooreOp<"conversion", [Pure]> { let hasFolder = 1; } +def TruncOp : MooreOp<"trunc", [ + Pure, + TypeDomainsMatch<["input", "result"]>, + PredOpTrait<"result width must be smaller than input width", CPred<[{ + cast($result.getType()).getBitSize() < + cast($input.getType()).getBitSize() + }]>>, +]> { + let summary = "Truncate a value"; + let description = [{ + Reduce the bit width of a value by removing some of its most significant + bits. This can only change the bit width of an integer type. + }]; + let arguments = (ins IntType:$input); + let results = (outs IntType:$result); + let assemblyFormat = [{ + $input attr-dict `:` type($input) `->` type($result) + }]; +} + +class ExtOpBase : MooreOp, + PredOpTrait<"result width must be larger than input width", CPred<[{ + cast($result.getType()).getBitSize() > + cast($input.getType()).getBitSize() + }]>>, +]> { + let arguments = (ins IntType:$input); + let results = (outs IntType:$result); + let assemblyFormat = [{ + $input attr-dict `:` type($input) `->` type($result) + }]; +} + +def ZExtOp : ExtOpBase<"zext"> { + let summary = "Zero-extend a value"; + let description = [{ + Increase the bit width of a value by inserting additional zero most + significant bits. This keeps the unsigned value constant. + }]; +} + +def SExtOp : ExtOpBase<"sext"> { + let summary = "Sign-extend a value"; + let description = [{ + Increase the bit width of a value by replicating its most significant bit. + This keeps the signed value constant. + }]; +} + +//===----------------------------------------------------------------------===// +// Expressions +//===----------------------------------------------------------------------===// + def NegOp : MooreOp<"neg", [Pure, SameOperandsAndResultType]> { let summary = "Arithmetic negation"; let description = [{ diff --git a/lib/Conversion/ImportVerilog/Expressions.cpp b/lib/Conversion/ImportVerilog/Expressions.cpp index 15f1b4e0590e..6f0706ef155a 100644 --- a/lib/Conversion/ImportVerilog/Expressions.cpp +++ b/lib/Conversion/ImportVerilog/Expressions.cpp @@ -610,20 +610,16 @@ struct RvalueExprVisitor { // Handle left expression. builder.setInsertionPointToStart(&trueBlock); - auto trueValue = context.convertRvalueExpression(expr.left()); + auto trueValue = context.convertRvalueExpression(expr.left(), type); if (!trueValue) return {}; - if (trueValue.getType() != type) - trueValue = builder.create(loc, type, trueValue); builder.create(loc, trueValue); // Handle right expression. builder.setInsertionPointToStart(&falseBlock); - auto falseValue = context.convertRvalueExpression(expr.right()); + auto falseValue = context.convertRvalueExpression(expr.right(), type); if (!falseValue) return {}; - if (falseValue.getType() != type) - falseValue = builder.create(loc, type, falseValue); builder.create(loc, falseValue); return conditionalOp.getResult(); @@ -967,8 +963,9 @@ Value Context::convertRvalueExpression(const slang::ast::Expression &expr, Type requiredType) { auto loc = convertLocation(expr.sourceRange); auto value = expr.visit(RvalueExprVisitor(*this, loc)); - if (value && requiredType && value.getType() != requiredType) - value = builder.create(loc, requiredType, value); + if (value && requiredType) + value = + materializeConversion(requiredType, value, expr.type->isSigned(), loc); return value; } @@ -1064,3 +1061,43 @@ Value Context::convertToSimpleBitVector(Value value) { << " cannot be cast to a simple bit vector"; return {}; } + +Value Context::materializeConversion(Type type, Value value, bool isSigned, + Location loc) { + if (type == value.getType()) + return value; + auto dstPacked = dyn_cast(type); + auto srcPacked = dyn_cast(value.getType()); + + // Resize the value if needed. + if (dstPacked && srcPacked && dstPacked.getBitSize() && + srcPacked.getBitSize() && + *dstPacked.getBitSize() != *srcPacked.getBitSize()) { + auto dstWidth = *dstPacked.getBitSize(); + auto srcWidth = *srcPacked.getBitSize(); + + // Convert the value to a simple bit vector which we can extend or truncate. + auto srcWidthType = moore::IntType::get(value.getContext(), srcWidth, + srcPacked.getDomain()); + if (value.getType() != srcWidthType) + value = builder.create(value.getLoc(), srcWidthType, + value); + + // Create truncation or sign/zero extension ops depending on the source and + // destination width. + auto dstWidthType = moore::IntType::get(value.getContext(), dstWidth, + srcPacked.getDomain()); + if (dstWidth < srcWidth) { + value = builder.create(loc, dstWidthType, value); + } else if (dstWidth > srcWidth) { + if (isSigned) + value = builder.create(loc, dstWidthType, value); + else + value = builder.create(loc, dstWidthType, value); + } + } + + if (value.getType() != type) + value = builder.create(loc, type, value); + return value; +} diff --git a/lib/Conversion/ImportVerilog/ImportVerilogInternals.h b/lib/Conversion/ImportVerilog/ImportVerilogInternals.h index 2d281b6599a0..b045ffe00713 100644 --- a/lib/Conversion/ImportVerilog/ImportVerilogInternals.h +++ b/lib/Conversion/ImportVerilog/ImportVerilogInternals.h @@ -121,6 +121,11 @@ struct Context { /// if the given value is null. Value convertToSimpleBitVector(Value value); + /// Helper function to insert the necessary operations to cast a value from + /// one type to another. + Value materializeConversion(Type type, Value value, bool isSigned, + Location loc); + /// Helper function to materialize an `SVInt` as an SSA value. Value materializeSVInt(const slang::SVInt &svint, const slang::ast::Type &type, Location loc); diff --git a/lib/Conversion/MooreToCore/MooreToCore.cpp b/lib/Conversion/MooreToCore/MooreToCore.cpp index 63f9ee728893..7853e71c0630 100644 --- a/lib/Conversion/MooreToCore/MooreToCore.cpp +++ b/lib/Conversion/MooreToCore/MooreToCore.cpp @@ -967,6 +967,10 @@ struct CaseXZEqOpConversion : public OpConversionPattern { } }; +//===----------------------------------------------------------------------===// +// Conversions +//===----------------------------------------------------------------------===// + struct ConversionOpConversion : public OpConversionPattern { using OpConversionPattern::OpConversionPattern; @@ -991,6 +995,50 @@ struct ConversionOpConversion : public OpConversionPattern { } }; +struct TruncOpConversion : public OpConversionPattern { + using OpConversionPattern::OpConversionPattern; + + LogicalResult + matchAndRewrite(TruncOp op, OpAdaptor adaptor, + ConversionPatternRewriter &rewriter) const override { + rewriter.replaceOpWithNewOp(op, adaptor.getInput(), 0, + op.getType().getWidth()); + return success(); + } +}; + +struct ZExtOpConversion : public OpConversionPattern { + using OpConversionPattern::OpConversionPattern; + + LogicalResult + matchAndRewrite(ZExtOp op, OpAdaptor adaptor, + ConversionPatternRewriter &rewriter) const override { + auto targetWidth = op.getType().getWidth(); + auto inputWidth = op.getInput().getType().getWidth(); + + auto zeroExt = rewriter.create( + op.getLoc(), rewriter.getIntegerType(targetWidth - inputWidth), 0); + + rewriter.replaceOpWithNewOp( + op, ValueRange{zeroExt, adaptor.getInput()}); + return success(); + } +}; + +struct SExtOpConversion : public OpConversionPattern { + using OpConversionPattern::OpConversionPattern; + + LogicalResult + matchAndRewrite(SExtOp op, OpAdaptor adaptor, + ConversionPatternRewriter &rewriter) const override { + auto type = typeConverter->convertType(op.getType()); + auto value = + comb::createOrFoldSExt(op.getLoc(), adaptor.getInput(), type, rewriter); + rewriter.replaceOp(op, value); + return success(); + } +}; + //===----------------------------------------------------------------------===// // Statement Conversion //===----------------------------------------------------------------------===// @@ -1498,10 +1546,16 @@ static void populateOpConversion(RewritePatternSet &patterns, VariableOpConversion, NetOpConversion, + // Patterns for conversion operations. + ConversionOpConversion, + TruncOpConversion, + ZExtOpConversion, + SExtOpConversion, + // Patterns of miscellaneous operations. ConstantOpConv, ConcatOpConversion, ReplicateOpConversion, ExtractOpConversion, DynExtractOpConversion, DynExtractRefOpConversion, - ConversionOpConversion, ReadOpConversion, + ReadOpConversion, StructExtractOpConversion, StructExtractRefOpConversion, ExtractRefOpConversion, StructCreateOpConversion, ConditionalOpConversion, YieldOpConversion, OutputOpConversion, StringConstantOpConv, diff --git a/test/Conversion/ImportVerilog/basic.sv b/test/Conversion/ImportVerilog/basic.sv index d1e87f2ac8a4..c2e9630e0ab5 100644 --- a/test/Conversion/ImportVerilog/basic.sv +++ b/test/Conversion/ImportVerilog/basic.sv @@ -759,8 +759,9 @@ module Expressions; // CHECK: moore.neg [[TMP1]] : i32 c = -a; // CHECK: [[TMP1:%.+]] = moore.read %v - // CHECK: [[TMP2:%.+]] = moore.conversion [[TMP1]] : !moore.array<2 x i4> -> !moore.i32 - // CHECK: moore.neg [[TMP2]] : i32 + // CHECK: [[TMP2:%.+]] = moore.conversion [[TMP1]] : !moore.array<2 x i4> -> !moore.i8 + // CHECK: [[TMP3:%.+]] = moore.zext [[TMP2]] : i8 -> i32 + // CHECK: moore.neg [[TMP3]] : i32 c = -v; // CHECK: [[TMP1:%.+]] = moore.read %a // CHECK: moore.not [[TMP1]] : i32 @@ -831,8 +832,9 @@ module Expressions; c = a + b; // CHECK: [[TMP1:%.+]] = moore.read %a // CHECK: [[TMP2:%.+]] = moore.read %v - // CHECK: [[TMP3:%.+]] = moore.conversion [[TMP2]] : !moore.array<2 x i4> -> !moore.i32 - // CHECK: moore.add [[TMP1]], [[TMP3]] : i32 + // CHECK: [[TMP3:%.+]] = moore.conversion [[TMP2]] : !moore.array<2 x i4> -> !moore.i8 + // CHECK: [[TMP4:%.+]] = moore.zext [[TMP3]] : i8 -> i32 + // CHECK: moore.add [[TMP1]], [[TMP4]] : i32 c = a + v; // CHECK: [[TMP1:%.+]] = moore.read %a // CHECK: [[TMP2:%.+]] = moore.read %b @@ -1305,15 +1307,15 @@ module Conversion; // Implicit conversion. // CHECK: %a = moore.variable // CHECK: [[TMP1:%.+]] = moore.read %a - // CHECK: [[TMP2:%.+]] = moore.conversion [[TMP1]] : !moore.i16 -> !moore.i32 + // CHECK: [[TMP2:%.+]] = moore.sext [[TMP1]] : i16 -> i32 // CHECK: %b = moore.variable [[TMP2]] shortint a; int b = a; // Explicit conversion. // CHECK: [[TMP1:%.+]] = moore.read %a - // CHECK: [[TMP2:%.+]] = moore.conversion [[TMP1]] : !moore.i16 -> !moore.i8 - // CHECK: [[TMP3:%.+]] = moore.conversion [[TMP2]] : !moore.i8 -> !moore.i32 + // CHECK: [[TMP2:%.+]] = moore.trunc [[TMP1]] : i16 -> i8 + // CHECK: [[TMP3:%.+]] = moore.sext [[TMP2]] : i8 -> i32 // CHECK: %c = moore.variable [[TMP3]] int c = byte'(a); @@ -1327,7 +1329,7 @@ module Conversion; // Width conversion. // CHECK: [[TMP1:%.+]] = moore.read %b - // CHECK: [[TMP2:%.+]] = moore.conversion [[TMP1]] : !moore.i32 -> !moore.i19 + // CHECK: [[TMP2:%.+]] = moore.trunc [[TMP1]] : i32 -> i19 // CHECK: %e = moore.variable [[TMP2]] bit signed [18:0] e = 19'(b); @@ -2049,3 +2051,24 @@ endmodule module PortCastB (input bit [0:0][31:0] a, output bit [0:0][31:0] b); assign b = a; endmodule + +// CHECK-LABEL: func.func private @SignCastsA( +// CHECK-SAME: %arg0: !moore.l16 +function void SignCastsA(logic [15:0] value); + // CHECK: [[TMP:%.+]] = moore.zext %arg0 : l16 -> l32 + // CHECK: call @SignCastsB([[TMP]]) + SignCastsB($unsigned(value)); + // CHECK: [[TMP:%.+]] = moore.sext %arg0 : l16 -> l32 + // CHECK: call @SignCastsB([[TMP]]) + SignCastsB($signed(value)); + + // CHECK: [[TMP:%.+]] = moore.zext %arg0 : l16 -> l32 + // CHECK: call @SignCastsB([[TMP]]) + SignCastsB(unsigned'(value)); + // CHECK: [[TMP:%.+]] = moore.sext %arg0 : l16 -> l32 + // CHECK: call @SignCastsB([[TMP]]) + SignCastsB(signed'(value)); +endfunction + +function void SignCastsB(logic [31:0] value); +endfunction diff --git a/test/Conversion/MooreToCore/basic.mlir b/test/Conversion/MooreToCore/basic.mlir index fe3478d72855..ea1baf413d2c 100644 --- a/test/Conversion/MooreToCore/basic.mlir +++ b/test/Conversion/MooreToCore/basic.mlir @@ -65,18 +65,6 @@ func.func @Expressions(%arg0: !moore.i1, %arg1: !moore.l1, %arg2: !moore.i6, %ar moore.constant 12 : !moore.i32 moore.constant 3 : !moore.i6 - moore.conversion %arg0 : !moore.i1 -> !moore.l1 - // CHECK-NEXT: [[V0:%.+]] = hw.constant 0 : i2 - // CHECK-NEXT: comb.concat [[V0]], %arg2 : i2, i6 - moore.conversion %arg2 : !moore.i6 -> !moore.l8 - // CHECK-NEXT: [[V0:%.+]] = comb.extract %arg2 from 4 : (i6) -> i2 - // CHECK-NEXT: [[V1:%.+]] = hw.constant 0 : i2 - // CHECK-NEXT: [[V2:%.+]] = comb.icmp eq [[V0]], [[V1]] : i2 - // CHECK-NEXT: [[V3:%.+]] = comb.extract %arg2 from 0 : (i6) -> i4 - // CHECK-NEXT: [[V4:%.+]] = hw.constant -1 : i4 - // CHECK-NEXT: comb.mux [[V2]], [[V3]], [[V4]] : i4 - moore.conversion %arg2 : !moore.i6 -> !moore.l4 - // CHECK-NEXT: [[V0:%.+]] = hw.constant 0 : i5 // CHECK-NEXT: [[V1:%.+]] = comb.concat [[V0]], %arg0 : i5, i1 // CHECK-NEXT: comb.shl %arg2, [[V1]] : i6 @@ -879,3 +867,34 @@ moore.module @StringConstant() { moore.return } } + +// CHECK-LABEL: func.func @Conversions +func.func @Conversions(%arg0: !moore.i16, %arg1: !moore.l16) { + // CHECK: [[TMP:%.+]] = comb.extract %arg0 from 0 : (i16) -> i8 + // CHECK: dbg.variable "trunc", [[TMP]] + %0 = moore.trunc %arg0 : i16 -> i8 + dbg.variable "trunc", %0 : !moore.i8 + + // CHECK: [[ZEXT:%.+]] = hw.constant 0 : i16 + // CHECK: [[TMP:%.+]] = comb.concat [[ZEXT]], %arg0 : i16, i16 + // CHECK: dbg.variable "zext", [[TMP]] + %1 = moore.zext %arg0 : i16 -> i32 + dbg.variable "zext", %1 : !moore.i32 + + // CHECK: [[SIGN:%.+]] = comb.extract %arg0 from 15 : (i16) -> i1 + // CHECK: [[SEXT:%.+]] = comb.replicate [[SIGN]] : (i1) -> i16 + // CHECK: [[TMP:%.+]] = comb.concat [[SEXT]], %arg0 : i16, i16 + // CHECK: dbg.variable "sext", [[TMP]] + %2 = moore.sext %arg0 : i16 -> i32 + dbg.variable "sext", %2 : !moore.i32 + + // CHECK: dbg.variable "i2l", %arg0 : i16 + %3 = moore.conversion %arg0 : !moore.i16 -> !moore.l16 + dbg.variable "i2l", %3 : !moore.l16 + + // CHECK: dbg.variable "l2i", %arg1 : i16 + %4 = moore.conversion %arg1 : !moore.l16 -> !moore.i16 + dbg.variable "l2i", %4 : !moore.i16 + + return +}