Skip to content

Conversation

@uenoku
Copy link
Member

@uenoku uenoku commented Oct 5, 2025

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 change makes SubOp it possible to leverage datapath optimizations

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

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
@uenoku uenoku requested a review from darthscsi as a code owner October 5, 2025 21:02
@uenoku uenoku requested a review from cowardsa October 5, 2025 21:08
Copy link
Contributor

@cowardsa cowardsa left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for embracing the compressors! I think this is a sensible step. But not entirely sure on the best long term strategy.

I've been dwelling on this for some time as currently the compress operator is unaware of constant ones. The problem is that each adder can accommodate a carry-in (at no logic depth cost) so we don't really need a compress operator at all here - but what we do need is an adder that can represent the carry-in - this is a bigger discussion (and certainly there may be other ways) so non-blocking as this step is clearly sensible!

Comment on lines +228 to +229
auto notRhs =
comb::createOrFoldNot(subOp.getLoc(), rhs, rewriter, subOp.getTwoState());
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat - did no realise there was a createOrFoldNot() function - will stop writing create constant one and Xor... Even though it presumably maps to this

@uenoku
Copy link
Member Author

uenoku commented Oct 6, 2025

I've been dwelling on this for some time as currently the compress operator is unaware of constant ones. The problem is that each adder can accommodate a carry-in (at no logic depth cost) so we don't really need a compress operator at all here

Yeah that makes sense. That sounds like clever optimization in DatapathToComb.

@uenoku uenoku merged commit 187f4e8 into main Oct 6, 2025
7 checks passed
@uenoku uenoku deleted the dev/hidetou/datapath-sub branch October 6, 2025 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants