Skip to content

[Synth] Implement operation reuse for LowerVariadic#9850

Open
joaovam wants to merge 4 commits intollvm:mainfrom
joaovam:fix/lower-variadic-subset-reuse
Open

[Synth] Implement operation reuse for LowerVariadic#9850
joaovam wants to merge 4 commits intollvm:mainfrom
joaovam:fix/lower-variadic-subset-reuse

Conversation

@joaovam
Copy link

@joaovam joaovam commented Mar 5, 2026

This patch implements a subset-sharing heuristic in the LowerVariadic pass, as suggested in Issue #9712.
Currently, the LowerVariadic pass lowers variadic operations into binary trees independently, leading to redundant AIG gates.

I added a greedy subset-matching heuristic. Before a variadic aig.and_inv is lowered to a binary tree, the pass scans the current block for existing operations of the same type. Checks if the operation is a strict subset of the current one. Substitute the operands with the result of the other operation.

Example

For the testcase:

module Test(
  input  [5:0] in,	
  output out1, out2
  );

  assign out1 = in[0] & in[1] & in[2] & in[3] & in[4];
  assign out2 =         in[1] & in[2] & in[3] & in[4];
endmodule

The output is:

module {
  hw.module @Test(in %in : i6, out out1 : i1, out out2 : i1) {
    %0 = comb.extract %in from 0 : (i6) -> i1
    %1 = comb.extract %in from 1 : (i6) -> i1
    %2 = comb.extract %in from 2 : (i6) -> i1
    %3 = comb.extract %in from 3 : (i6) -> i1
    %4 = comb.extract %in from 4 : (i6) -> i1
    %5 = synth.aig.and_inv %1, %2 : i1
    %6 = synth.aig.and_inv %3, %4 : i1
    %7 = synth.aig.and_inv %5, %6 : i1
    %8 = synth.aig.and_inv %7, %0 : i1
    hw.output %8, %7 : i1, i1
  }
}

Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

Cool, thank you for sending for PR! Can we make this an option of the pass for area/timing trade off? Note that there are failures in LEC but this change itself is not at fault. This patch exposed a bug in other pass, let me investigate :) Ah it seems there is a correctness issue with the current implementation. Let me extract a counterexample.

@uenoku uenoku changed the title Implement operation reuse for LowerVariadic [Synth] Implement operation reuse for LowerVariadic Mar 6, 2026
@joaovam
Copy link
Author

joaovam commented Mar 9, 2026

Hi @uenoku, I believe I'm almost done with making it O(N). I have a question, you asked me to make it part of an option. Did you mean I should create a new option that enables this specific optimization, or should I use an existing one?

@uenoku
Copy link
Member

uenoku commented Mar 10, 2026

Yes please add a new option to

"Lower operators with timing information">
.

@joaovam
Copy link
Author

joaovam commented Mar 10, 2026

@uenoku I have updated the pass to use a map of the available andInverterOp operations in the block. I have also added a new option, reuse-subsets as a guard to the new code. But I have a question about it, how to enable the option in circt-synth? I could only test it via a unit test.

@uenoku
Copy link
Member

uenoku commented Mar 11, 2026

Could you conditionally pass the option here when options.synthesisStrategy == OptimizationStrategyArea for now?

pm.addPass(createLowerVariadicPass<comb::XorOp>(options.timingAware));
} else if (options.targetIR.getValue() == TargetIR::MIG) {
// For MIG, lower variadic And, Or, and Xor since MIG cannot keep variadic
// representation.
pm.addPass(createLowerVariadicPass<comb::AndOp, comb::OrOp, comb::XorOp>(

@joaovam
Copy link
Author

joaovam commented Mar 11, 2026

Done. I see that createLowerVariadicPass is called in other places in that same file. Should I also update those calls?

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