From 642255a1c06d5b53439c5fa17f8e584052e4eebe Mon Sep 17 00:00:00 2001 From: Andrew Lenharth Date: Wed, 1 Apr 2026 09:28:58 -0700 Subject: [PATCH] Reject adding truncations for connections on inferwidths. This is a basic safety issue which has caused bugs in RTL. --- lib/Dialect/FIRRTL/Transforms/InferWidths.cpp | 21 +++++++------------ test/Dialect/FIRRTL/infer-widths-errors.mlir | 15 +++++++++++++ test/Dialect/FIRRTL/infer-widths.mlir | 7 +++++-- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/InferWidths.cpp b/lib/Dialect/FIRRTL/Transforms/InferWidths.cpp index 6e55d95ee86e..611363f16cf1 100644 --- a/lib/Dialect/FIRRTL/Transforms/InferWidths.cpp +++ b/lib/Dialect/FIRRTL/Transforms/InferWidths.cpp @@ -2081,9 +2081,9 @@ FailureOr InferenceTypeUpdate::updateOperation(Operation *op) { anyChanged |= *result; } - // If this is a connect operation, width inference might have inferred a RHS - // that is wider than the LHS, in which case an additional BitsPrimOp is - // necessary to truncate the value. + // If this is a connect operation, check that width inference did not infer a + // RHS that is wider than the LHS. Such a situation would require an + // implicit truncation, which is not allowed. if (auto con = dyn_cast(op)) { auto lhs = con.getDest(); auto rhs = con.getSrc(); @@ -2097,17 +2097,12 @@ FailureOr InferenceTypeUpdate::updateOperation(Operation *op) { auto lhsWidth = lhsType.getBitWidthOrSentinel(); auto rhsWidth = rhsType.getBitWidthOrSentinel(); if (lhsWidth >= 0 && rhsWidth >= 0 && lhsWidth < rhsWidth) { - OpBuilder builder(op); - auto trunc = builder.createOrFold(con.getLoc(), con.getSrc(), - rhsWidth - lhsWidth); - if (type_isa(rhsType)) - trunc = - builder.createOrFold(con.getLoc(), lhsType, trunc); - - LLVM_DEBUG(llvm::dbgs() - << "Truncating RHS to " << lhsType << " in " << con << "\n"); - con->replaceUsesOfWith(con.getSrc(), trunc); + con.emitError("connect would require truncation after width inference: " + "destination ") + << lhsType << " is not as wide as the source " << rhsType; + return failure(); } + return anyChanged; } diff --git a/test/Dialect/FIRRTL/infer-widths-errors.mlir b/test/Dialect/FIRRTL/infer-widths-errors.mlir index 464fcc4c988c..93f6c19cd593 100644 --- a/test/Dialect/FIRRTL/infer-widths-errors.mlir +++ b/test/Dialect/FIRRTL/infer-widths-errors.mlir @@ -182,3 +182,18 @@ firrtl.circuit "MuxSelTooWide" { firrtl.connect %0, %c2_ui2 : !firrtl.uint, !firrtl.uint<2> } } + +// ----- + +// Connections that would require truncation after width inference should +// produce a dedicated error rather than implicitly inserting truncation. +firrtl.circuit "TruncateConnect" { + firrtl.module @TruncateConnect() { + %w = firrtl.wire : !firrtl.uint + %c1_ui1 = firrtl.constant 1 : !firrtl.uint<1> + firrtl.connect %w, %c1_ui1 : !firrtl.uint, !firrtl.uint<1> + %w1 = firrtl.wire : !firrtl.uint<0> + // expected-error @+1 {{connect would require truncation after width inference: destination '!firrtl.uint<0>' is not as wide as the source '!firrtl.uint<1>'}} + firrtl.connect %w1, %w : !firrtl.uint<0>, !firrtl.uint + } +} diff --git a/test/Dialect/FIRRTL/infer-widths.mlir b/test/Dialect/FIRRTL/infer-widths.mlir index 969f34db943e..d60dc02fa6df 100644 --- a/test/Dialect/FIRRTL/infer-widths.mlir +++ b/test/Dialect/FIRRTL/infer-widths.mlir @@ -479,8 +479,10 @@ firrtl.circuit "Foo" { // CHECK: firrtl.connect %x, %c200_si9 : !firrtl.sint<9> %x = firrtl.wire : !firrtl.sint %c200_si = firrtl.constant 200 : !firrtl.sint - firrtl.connect %y, %x : !firrtl.sint<4>, !firrtl.sint firrtl.connect %x, %c200_si : !firrtl.sint, !firrtl.sint + %0 = firrtl.tail %x, 5 : (!firrtl.sint) -> !firrtl.uint + %1 = firrtl.asSInt %0 : (!firrtl.uint) -> !firrtl.sint + firrtl.connect %y, %1 : !firrtl.sint<4>, !firrtl.sint } // Should truncate all the way to 0 bits if its has to. @@ -492,7 +494,8 @@ firrtl.circuit "Foo" { %w1 = firrtl.wire : !firrtl.uint<0> // CHECK: %0 = firrtl.tail %w, 1 : (!firrtl.uint<1>) -> !firrtl.uint<0> // CHECK: firrtl.connect %w1, %0 : !firrtl.uint<0> - firrtl.connect %w1, %w : !firrtl.uint<0>, !firrtl.uint + %0 = firrtl.tail %w, 1 : (!firrtl.uint) -> !firrtl.uint + firrtl.connect %w1, %0 : !firrtl.uint<0>, !firrtl.uint } // Issue #1110: Width inference should infer 0 width when appropriate