[CombToDatapath] Lower comb::SubOp (#9053)

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

Author: uenoku

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

lib/Conversion/CombToDatapath/CombToDatapath.cpp

@@ -92,6 +92,7 @@ struct ConvertCombToDatapathPass
 static void
 populateCombToDatapathConversionPatterns(RewritePatternSet &patterns) {
   patterns.add<CombAddOpConversion, CombMulOpConversion>(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>(
       [](comb::MulOp op) { return op.getNumOperands() > 2; });
+  target.addIllegalOp<comb::SubOp>();
 
   RewritePatternSet patterns(&getContext());
   populateCombToDatapathConversionPatterns(patterns);

lib/Conversion/CombToSynth/CombToSynth.cpp

@@ -855,25 +855,6 @@ struct CombAddOpConversion : OpConversionPattern<AddOp> {
   }
 };
 
-struct CombSubOpConversion : OpConversionPattern<SubOp> {
-  using OpConversionPattern<SubOp>::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<comb::AddOp>(
-        rewriter, op, ValueRange{lhs, notRhs, one}, true);
-    return success();
-  }
-};
-
 struct CombMulOpConversion : OpConversionPattern<MulOp> {
   using OpConversionPattern<MulOp>::OpConversionPattern;
   using OpAdaptor = typename OpConversionPattern<MulOp>::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<XorOp>, CombLowerVariadicOp<AddOp>,
       CombLowerVariadicOp<MulOp>>(patterns.getContext());
 
+  patterns.add(comb::convertSubToAdd);
+
   if (lowerToMIG) {
     patterns.add<CombOrToMIGConversion, CombLowerVariadicOp<OrOp>,
                  AndInverterToMIGConversion,

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<comb::ConcatOp>(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<comb::AddOp>(
+      rewriter, subOp, ValueRange{lhs, notRhs, one}, subOp.getTwoState());
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // ICmpOp
 //===----------------------------------------------------------------------===//

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
+}
 

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