Skip to content

Conversation

@MrRoy09
Copy link

@MrRoy09 MrRoy09 commented Nov 8, 2025

Adds canonicalization patterns and tests for comb.truth_table:

  • Folds truth tables with constant output (all true or all false entries) into hw.constant
  • Simplifies truth tables that depend on a single input to the corresponding input value
  • Adds tests verifying canonicalization behavior for comb.truth_table

Closes #8925

@MrRoy09 MrRoy09 requested a review from darthscsi as a code owner November 8, 2025 14:38
@MrRoy09 MrRoy09 force-pushed the comb/canonicalize-truth-table branch from bacdbbc to 5ff85a1 Compare November 9, 2025 18:57
@MrRoy09 MrRoy09 force-pushed the comb/canonicalize-truth-table branch from 5ff85a1 to 0091687 Compare November 9, 2025 18:59
@fzi-hielscher
Copy link
Contributor

Thanks! The implementation and tests look very nice. I'm just not sure whether this really should be a canonicalization pattern. Considering the exponential growth of truth tables this could very easily have a massive effect on runtime . And it doesn't make much sense to apply this pattern repeatedly in the canonicalization convergence loop since it only depends on the truth table itself. I think we should move this to a dedicated pass instead. WDYT @uenoku?

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 adding this! LGTM!

@fzi-hielscher fzi-hielscher added the Comb Involving the `comb` dialect label Nov 14, 2025
@uenoku
Copy link
Member

uenoku commented Nov 14, 2025

And it doesn't make much sense to apply this pattern repeatedly in the canonicalization convergence loop since it only depends on the truth table itself.

I think that's fair concern (though the table size is already 2^n so i don't think it's going to be a critical regression). Maybe we can use the same pattern for canonicalization and dedicated pass and for canonicalization it might makes sense to restrict the lut size to small (e.g. 4 inputs). WDYT? @fzi-hielscher

Alternatively we can also disable the canonicalization for negation and simply find identity patterns
00001111, 00110011 and 01010101 which may be suitable for even folder.

@fzi-hielscher
Copy link
Contributor

Yeah, not touching truth tables beyond a certain limit sounds sensible to me. That being said, I don't have an idea what sizes we are dealing with in practice and what a "reasonable" limit would be.

@teqdruid
Copy link
Contributor

We've been talking about introducing a '-O1' pass for the core dialect as we're definitely abusing canonicalizers for some of them. If there's any debate, this would be a good candidate for having in that pass.

@MrRoy09 MrRoy09 requested a review from uenoku November 16, 2025 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Comb Involving the `comb` dialect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Comb] Fold comb.truth_table when truth table depends on a single operand

4 participants