From 423323a34596fab335424984e82d86fd60c22ecd Mon Sep 17 00:00:00 2001 From: Hideto Ueno Date: Sun, 5 Oct 2025 13:46:42 -0700 Subject: [PATCH] [CombToDatapath] Lower comb::SubOp Adds support for lowering comb::SubOp in the CombToDatapath conversion pass by refactoring the SubOp-to-AddOp conversion into a shared utility function that can be reused across multiple conversion passes. This commit extracts the subtraction lowering logic from CombToSynth into a dialect-agnostic utility function `comb::convertSubToAdd` that transforms `sub(lhs, rhs)` into `add(lhs, ~rhs, 1)` using two's complement representation. AIG logic depth improved 29 -> 24 for i128 subtraction --- include/circt/Dialect/Comb/CombOps.h | 4 ++++ .../CombToDatapath/CombToDatapath.cpp | 2 ++ lib/Conversion/CombToSynth/CombToSynth.cpp | 23 +++---------------- lib/Dialect/Comb/CombOps.cpp | 17 ++++++++++++++ .../CombToDatapath/comb-to-datapath.mlir | 11 +++++++++ .../CombToSynth/comb-to-aig-arith.mlir | 5 ++-- 6 files changed, 40 insertions(+), 22 deletions(-) diff --git a/include/circt/Dialect/Comb/CombOps.h b/include/circt/Dialect/Comb/CombOps.h index 66684befb8c9..77681f1c8a3d 100644 --- a/include/circt/Dialect/Comb/CombOps.h +++ b/include/circt/Dialect/Comb/CombOps.h @@ -86,6 +86,10 @@ Value createDynamicInject(OpBuilder &builder, Location loc, Value value, Value createInject(OpBuilder &builder, Location loc, Value value, unsigned offset, Value replacement); +/// Replace a subtraction with an addition of the two's complement. +LogicalResult convertSubToAdd(comb::SubOp subOp, + mlir::PatternRewriter &rewriter); + /// Enum for mux chain folding styles. enum MuxChainWithComparisonFoldingStyle { None, BalancedMuxTree, ArrayGet }; /// Mux chain folding that converts chains of muxes with index diff --git a/lib/Conversion/CombToDatapath/CombToDatapath.cpp b/lib/Conversion/CombToDatapath/CombToDatapath.cpp index f8887c80a093..138d0bbbc348 100644 --- a/lib/Conversion/CombToDatapath/CombToDatapath.cpp +++ b/lib/Conversion/CombToDatapath/CombToDatapath.cpp @@ -92,6 +92,7 @@ struct ConvertCombToDatapathPass static void populateCombToDatapathConversionPatterns(RewritePatternSet &patterns) { patterns.add(patterns.getContext()); + patterns.add(comb::convertSubToAdd); } void ConvertCombToDatapathPass::runOnOperation() { @@ -106,6 +107,7 @@ void ConvertCombToDatapathPass::runOnOperation() { // TODO: determine lowering of multi-input multipliers target.addDynamicallyLegalOp( [](comb::MulOp op) { return op.getNumOperands() > 2; }); + target.addIllegalOp(); RewritePatternSet patterns(&getContext()); populateCombToDatapathConversionPatterns(patterns); diff --git a/lib/Conversion/CombToSynth/CombToSynth.cpp b/lib/Conversion/CombToSynth/CombToSynth.cpp index 2bcab53dc6d9..1beb13d77fb0 100644 --- a/lib/Conversion/CombToSynth/CombToSynth.cpp +++ b/lib/Conversion/CombToSynth/CombToSynth.cpp @@ -855,25 +855,6 @@ struct CombAddOpConversion : OpConversionPattern { } }; -struct CombSubOpConversion : OpConversionPattern { - using OpConversionPattern::OpConversionPattern; - LogicalResult - matchAndRewrite(SubOp op, OpAdaptor adaptor, - ConversionPatternRewriter &rewriter) const override { - auto lhs = op.getLhs(); - auto rhs = op.getRhs(); - // Since `-rhs = ~rhs + 1` holds, rewrite `sub(lhs, rhs)` to: - // sub(lhs, rhs) => add(lhs, -rhs) => add(lhs, add(~rhs, 1)) - // => add(lhs, ~rhs, 1) - auto notRhs = synth::aig::AndInverterOp::create(rewriter, op.getLoc(), rhs, - /*invert=*/true); - auto one = hw::ConstantOp::create(rewriter, op.getLoc(), op.getType(), 1); - replaceOpWithNewOpAndCopyNamehint( - rewriter, op, ValueRange{lhs, notRhs, one}, true); - return success(); - } -}; - struct CombMulOpConversion : OpConversionPattern { using OpConversionPattern::OpConversionPattern; using OpAdaptor = typename OpConversionPattern::OpAdaptor; @@ -1376,13 +1357,15 @@ populateCombToAIGConversionPatterns(RewritePatternSet &patterns, CombAndOpConversion, CombXorOpConversion, CombMuxOpConversion, CombParityOpConversion, // Arithmetic Ops - CombSubOpConversion, CombMulOpConversion, CombICmpOpConversion, + CombMulOpConversion, CombICmpOpConversion, // Shift Ops CombShlOpConversion, CombShrUOpConversion, CombShrSOpConversion, // Variadic ops that must be lowered to binary operations CombLowerVariadicOp, CombLowerVariadicOp, CombLowerVariadicOp>(patterns.getContext()); + patterns.add(comb::convertSubToAdd); + if (lowerToMIG) { patterns.add, AndInverterToMIGConversion, diff --git a/lib/Dialect/Comb/CombOps.cpp b/lib/Dialect/Comb/CombOps.cpp index da2f98c4d9fd..3fd5a9547b46 100644 --- a/lib/Dialect/Comb/CombOps.cpp +++ b/lib/Dialect/Comb/CombOps.cpp @@ -12,6 +12,7 @@ #include "circt/Dialect/Comb/CombOps.h" #include "circt/Dialect/HW/HWOps.h" +#include "circt/Support/Naming.h" #include "mlir/IR/Builders.h" #include "mlir/IR/ImplicitLocOpBuilder.h" #include "mlir/IR/Matchers.h" @@ -217,6 +218,22 @@ Value comb::createInject(OpBuilder &builder, Location loc, Value value, return builder.createOrFold(loc, fragments); } +llvm::LogicalResult comb::convertSubToAdd(comb::SubOp subOp, + mlir::PatternRewriter &rewriter) { + auto lhs = subOp.getLhs(); + auto rhs = subOp.getRhs(); + // Since `-rhs = ~rhs + 1` holds, rewrite `sub(lhs, rhs)` to: + // sub(lhs, rhs) => add(lhs, -rhs) => add(lhs, add(~rhs, 1)) + // => add(lhs, ~rhs, 1) + auto notRhs = + comb::createOrFoldNot(subOp.getLoc(), rhs, rewriter, subOp.getTwoState()); + auto one = + hw::ConstantOp::create(rewriter, subOp.getLoc(), subOp.getType(), 1); + replaceOpWithNewOpAndCopyNamehint( + rewriter, subOp, ValueRange{lhs, notRhs, one}, subOp.getTwoState()); + return success(); +} + //===----------------------------------------------------------------------===// // ICmpOp //===----------------------------------------------------------------------===// diff --git a/test/Conversion/CombToDatapath/comb-to-datapath.mlir b/test/Conversion/CombToDatapath/comb-to-datapath.mlir index bedea0091645..9d453e61fef9 100644 --- a/test/Conversion/CombToDatapath/comb-to-datapath.mlir +++ b/test/Conversion/CombToDatapath/comb-to-datapath.mlir @@ -26,4 +26,15 @@ hw.module @zero_width(in %arg0: i0, in %arg1: i0, in %arg2: i0) { // CHECK-NEXT: hw.constant 0 : i0 %0 = comb.add %arg0, %arg1, %arg2 : i0 } +// CHECK-LABEL: @sub +hw.module @sub(in %arg0: i4, in %arg1: i4, out out: i4) { + %0 = comb.sub %arg0, %arg1 : i4 + // CHECK-NEXT: %[[C_NEG1:.+]] = hw.constant -1 : i4 + // CHECK-NEXT: %[[XOR:.+]] = comb.xor %arg1, %[[C_NEG1]] : i4 + // CHECK-NEXT: %[[C1:.+]] = hw.constant 1 : i4 + // CHECK-NEXT: %[[COMP:.+]]:2 = datapath.compress %arg0, %[[XOR]], %[[C1]] : i4 [3 -> 2] + // CHECK-NEXT: %[[ADD:.+]] = comb.add bin %[[COMP]]#0, %[[COMP]]#1 : i4 + // CHECK-NEXT: hw.output %[[ADD]] : i4 + hw.output %0 : i4 +} diff --git a/test/Conversion/CombToSynth/comb-to-aig-arith.mlir b/test/Conversion/CombToSynth/comb-to-aig-arith.mlir index d754bfe95d64..fcd76d113201 100644 --- a/test/Conversion/CombToSynth/comb-to-aig-arith.mlir +++ b/test/Conversion/CombToSynth/comb-to-aig-arith.mlir @@ -146,9 +146,10 @@ hw.module @add_64(in %lhs: i64, in %rhs: i64, out out: i64) { // CHECK-LABEL: @sub // ALLOW_ADD-LABEL: @sub -// ALLOW_ADD-NEXT: %[[NOT_RHS:.+]] = synth.aig.and_inv not %rhs +// ALLOW_ADD-NEXT: %[[C_NEG1:.+]] = hw.constant -1 : i4 +// ALLOW_ADD-NEXT: %[[NOT_RHS:.+]] = comb.xor %rhs, %[[C_NEG1]] : i4 // ALLOW_ADD-NEXT: %[[CONST:.+]] = hw.constant 1 : i4 -// ALLOW_ADD-NEXT: %[[ADD:.+]] = comb.add bin %lhs, %[[NOT_RHS]], %[[CONST]] {sv.namehint = "sub"} +// ALLOW_ADD-NEXT: %[[ADD:.+]] = comb.add %lhs, %[[NOT_RHS]], %[[CONST]] {sv.namehint = "sub"} // ALLOW_ADD-NEXT: hw.output %[[ADD]] hw.module @sub(in %lhs: i4, in %rhs: i4, out out: i4) { %0 = comb.sub %lhs, %rhs {sv.namehint = "sub"} : i4