Skip to content

Conversation

@MrRoy09
Copy link
Contributor

@MrRoy09 MrRoy09 commented Nov 26, 2025

Adds dedicated Simplification Pass and tests for comb.truth_table op:

  • Simplifies 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 correctness of simplification for comb.truth_table

Related to #9214
Closes #8925

@MrRoy09 MrRoy09 requested a review from darthscsi as a code owner November 26, 2025 18:39
@MrRoy09
Copy link
Contributor Author

MrRoy09 commented Nov 26, 2025

Not sure if isOpTriviallyRecursive is necessary here but I stuck to the original implementation in pr #9214.

@MrRoy09 MrRoy09 marked this pull request as draft November 26, 2025 20:50
@MrRoy09 MrRoy09 marked this pull request as ready for review November 26, 2025 20:56
@MrRoy09 MrRoy09 force-pushed the comb-simplify-truth-table branch from 83215db to 9dd1b63 Compare November 26, 2025 21:13
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.

LGTM! Thank you for implementing it as a pass. Maybe in a follow up we can remove operands that don't affect outputs instead of keeping a single operand (e.g. 5 input LUT -> 2 input LUT) but I think the current implementation looks good to me.

Do you mind if you could check few tests case with LEC to make sure the transformation is semantically correct?

circt-opt test/Dialect/Comb/simplify-truth-table.mlir -convert-comb -o before.mlir
circt-opt test/Dialect/Comb/simplify-truth-table.mlir --comb-simplify-tt -convert-comb  -o after.mlir
circt-lec --c1 truth_table_middle_input_inverted --c2 truth_table_middle_input_inverted  before.mlir after.mlir --shared-libs=<path_to_libz3.so>

@MrRoy09 MrRoy09 force-pushed the comb-simplify-truth-table branch 2 times, most recently from 105e142 to f3d2d67 Compare November 27, 2025 06:45
@MrRoy09 MrRoy09 force-pushed the comb-simplify-truth-table branch from f3d2d67 to fe11278 Compare November 27, 2025 06:48
@MrRoy09
Copy link
Contributor Author

MrRoy09 commented Nov 27, 2025

LGTM! Thank you for implementing it as a pass. Maybe in a follow up we can remove operands that don't affect outputs instead of keeping a single operand (e.g. 5 input LUT -> 2 input LUT) but I think the current implementation looks good to me.

Do you mind if you could check few tests case with LEC to make sure the transformation is semantically correct?

circt-opt test/Dialect/Comb/simplify-truth-table.mlir -convert-comb -o before.mlir
circt-opt test/Dialect/Comb/simplify-truth-table.mlir --comb-simplify-tt -convert-comb  -o after.mlir
circt-lec --c1 truth_table_middle_input_inverted --c2 truth_table_middle_input_inverted  before.mlir after.mlir --shared-libs=<path_to_libz3.so>

I got an error when running with -convert-comb (unknown option). I used the following to check for equivalence (--lower-comb instead of -convert-comb) and it passed for all testcases.

circt-opt test/Dialect/Comb/simplify-truth-table.mlir --lower-comb -o before.mlir
circt-opt test/Dialect/Comb/simplify-truth-table.mlir --comb-simplify-tt --lower-comb -o after.mlir
circt-lec --c1=truth_table_middle_input_inverted --c2=truth_table_middle_input_inverted before.mlir after.mlir --shared-libs=/usr/lib/x86_64-linux-gnu/libz3.so
c1 == c2

Please let me know if I need to run the check any other way

@uenoku
Copy link
Member

uenoku commented Nov 27, 2025

Thanks! I'm going to merge. Thank you for the PR :)

@uenoku uenoku merged commit 394deb3 into llvm:main Nov 27, 2025
7 checks passed
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.

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

2 participants