Skip to content

[CombToSynth] Add a lowering for mod/div where rhs is constant#10584

Open
okekayode wants to merge 3 commits into
llvm:mainfrom
okekayode:dev/mod-div-lowering
Open

[CombToSynth] Add a lowering for mod/div where rhs is constant#10584
okekayode wants to merge 3 commits into
llvm:mainfrom
okekayode:dev/mod-div-lowering

Conversation

@okekayode
Copy link
Copy Markdown
Contributor

@okekayode okekayode commented Jun 4, 2026

Concerns #10569

In this PR we add lowerings for comb.divu, comb.modu, comb.divs, and comb.mods in the case that the RHS is a constant value. For the division operators , we utilise UnsignedDivisionByConstantInfo and SignedDivisionByConstantInfo to replace the operation with magic multiplication, shifts, and adds. For modulo, we reuse the division lowering via x % k = x - ((x / k) * k) = remainder

Assisted-by: claude: sonnet 4.6

@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented Jun 4, 2026

Results of circt-tests run for 759682f compared to results for 7aa41cc: no change to test results.

Comment thread test/Conversion/CombToSynth/comb-to-aig-arith.mlir
Comment thread lib/Conversion/CombToSynth/CombToSynth.cpp Outdated
@okekayode okekayode force-pushed the dev/mod-div-lowering branch from 759682f to 1767468 Compare June 4, 2026 20:21
@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented Jun 4, 2026

Results of circt-tests run for 1767468 compared to results for 7aa41cc: no change to test results.

@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented Jun 5, 2026

Results of circt-tests run for 4bb59c4 compared to results for 7aa41cc: no change to test results.

@uenoku
Copy link
Copy Markdown
Member

uenoku commented Jun 6, 2026

This fails LEC. I enumerated all possible inputs up to i8, but mods %lhs, -1 was only failure, so it's mostly correct. I think this mods %lhs, -1 is always zero, but apparently the result is different.

hw.module @const_divmod_mods_neg1_i3(in %lhs: i3, out out: i3) {
  %c_neg1_i3 = hw.constant -1 : i3
 
  %0 = comb.mods %lhs, %c_neg1_i3 : i3
  hw.output %0 : i3
}

@okekayode
Copy link
Copy Markdown
Contributor Author

This fails LEC. I enumerated all possible inputs up to i8, but mods %lhs, -1 was only failure, so it's mostly correct. I think this mods %lhs, -1 is always zero, but apparently the result is different.

hw.module @const_divmod_mods_neg1_i3(in %lhs: i3, out out: i3) {
  %c_neg1_i3 = hw.constant -1 : i3
 
  %0 = comb.mods %lhs, %c_neg1_i3 : i3
  hw.output %0 : i3
}

oops, let me add that case right now, yes it is always zero, seems i only considered the 0 case in mods

@uenoku
Copy link
Copy Markdown
Member

uenoku commented Jun 6, 2026

It's great that there is only one failure 👍 Great work!

@okekayode okekayode force-pushed the dev/mod-div-lowering branch from 7745373 to 5a47792 Compare June 6, 2026 05:57
@circt-bot
Copy link
Copy Markdown

circt-bot Bot commented Jun 6, 2026

Results of circt-tests run for 5a47792 compared to results for 7aa41cc: no change to test results.

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