[dsl][bugfix][enhancement][#432] Implement constant folding for select1hot#433
Conversation
.gitignore: Add .milestones/* to exclude milestone tracking files python/assassyn/ir/expr/expr.md: Document constant folding for Select1Hot python/assassyn/ir/value.md: Document constant folding behavior for select1hot method python/unit-tests/test_select1hot_constant_folding.py: Add comprehensive test cases This milestone creates documentation and test infrastructure for implementing constant folding in select1hot operations. The implementation will detect when both the selector and all values are constants, then compute the result at compile time instead of generating IR nodes. Test status: 0/1 tests passed (implementation not yet complete). Related to issue Synthesys-Lab#432.
python/assassyn/ir/value.py: Add constant folding logic in select1hot method python/unit-tests/test_select1hot_constant_folding.py: Fix test values to be in valid range python/assassyn/codegen/verilog/elaborate.py: Fix pylint issues (line length and complexity) When the selector of a select1hot operation is a constant, the implementation now evaluates the selection at compile time and returns the selected value directly, avoiding unnecessary IR node creation. This follows the same pattern as existing constant folding in Const.concat() and Const.__getitem__(). The implementation verifies the one-hot property (exactly one bit set) and selects the value at the corresponding index. This fixes code generation issues when using select1hot with all constant operands. Resolves Synthesys-Lab#432.
There was a problem hiding this comment.
Pull request overview
This PR implements constant folding optimization for select1hot operations, fixing code generation issues when using all constant operands. When the selector is a constant, the operation is evaluated at compile time and returns the selected value directly.
- Adds constant folding logic in
Value.select1hot()method that checks if the selector is constant and returns the corresponding value - Adds comprehensive test coverage with multiple test cases covering different bit positions and data widths
- Updates documentation in both value.md and expr.md to explain the constant folding behavior
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
python/assassyn/ir/value.py |
Implements constant folding logic in select1hot method with one-hot validation |
python/unit-tests/test_select1hot_constant_folding.py |
Adds new test file with comprehensive test cases for constant folding scenarios |
python/assassyn/ir/value.md |
Documents the constant folding behavior with examples |
python/assassyn/ir/expr/expr.md |
Notes that Value.select1hot() applies automatic constant folding |
python/assassyn/codegen/verilog/elaborate.py |
Adds pylint disable comment and fixes line length issue |
.gitignore |
Excludes .milestones/* directory |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for i, arg in enumerate(args): | ||
| if (cond_value >> i) & 1: | ||
| return arg | ||
|
|
There was a problem hiding this comment.
The constant folding logic should validate that the number of arguments matches the selector width. If the selector has a bit set at position i but there are fewer than i+1 arguments provided, the loop will complete without returning a value, causing the function to unexpectedly fall through to creating a Select1Hot node instead of raising an error.
Consider adding validation after the one-hot check to ensure the selector width is appropriate for the number of arguments provided.
| # If we reach here, the one-hot bit is outside the range of provided args | |
| raise AssertionError( | |
| "select1hot selector width is incompatible with number of arguments: " | |
| f"selector value {cond_value} has a set bit beyond index {len(args) - 1}" | |
| ) |
| f"select1hot requires one-hot selector, " | ||
| f"got {bit_count} bits set in {cond_value}" | ||
| ) | ||
|
|
There was a problem hiding this comment.
The one-hot validation doesn't handle the edge case where cond_value is 0 (no bits set). The assertion only checks that exactly one bit is set, but when cond_value is 0, bit_count will be 0 and the assertion will correctly fail. However, the error message could be misleading since it says "got {bit_count} bits set" without clarifying that 0 bits is also invalid.
Consider updating the assertion message to be clearer that both 0 bits and multiple bits are invalid states.
| f"select1hot requires one-hot selector, " | |
| f"got {bit_count} bits set in {cond_value}" | |
| ) | |
| "select1hot requires a one-hot selector (exactly one bit set); " | |
| f"got {bit_count} bits set in {cond_value} " | |
| "(0 bits or multiple bits set are invalid)" | |
| ) |
| # Test case from the bug report - bit 10 set, selecting from 32 values | ||
| select = [Bits(5)(i) for i in range(32)] | ||
| one_hot = Bits(32)(1 << 10) | ||
| select_one_hot = one_hot.select1hot(*select) | ||
| log('Select one hot bit 10: {}', select_one_hot) | ||
|
|
||
| # Test bit 0 selection | ||
| one_hot_0 = Bits(4)(1 << 0) | ||
| result_0 = one_hot_0.select1hot(Bits(8)(100), Bits(8)(200), Bits(8)(50), Bits(8)(150)) | ||
| log('Test bit 0: {}', result_0) | ||
|
|
||
| # Test bit 3 selection (last bit in 4-bit selector) | ||
| one_hot_3 = Bits(4)(1 << 3) | ||
| result_3 = one_hot_3.select1hot(Bits(8)(100), Bits(8)(200), Bits(8)(50), Bits(8)(150)) | ||
| log('Test bit 3: {}', result_3) | ||
|
|
||
| # Test bit 1 selection | ||
| one_hot_1 = Bits(4)(1 << 1) | ||
| result_1 = one_hot_1.select1hot(Bits(8)(10), Bits(8)(20), Bits(8)(30), Bits(8)(40)) | ||
| log('Test bit 1: {}', result_1) | ||
|
|
||
| # Test with different data types | ||
| one_hot_2 = Bits(8)(1 << 2) | ||
| result_2 = one_hot_2.select1hot( | ||
| Bits(16)(0x1234), | ||
| Bits(16)(0x5678), | ||
| Bits(16)(0xABCD), | ||
| Bits(16)(0xEF01), | ||
| Bits(16)(0x2345), | ||
| Bits(16)(0x6789), | ||
| Bits(16)(0xBCDE), | ||
| Bits(16)(0xF012) | ||
| ) | ||
| log('Test bit 2 with 16-bit values: 0x{:x}', result_2) |
There was a problem hiding this comment.
The test should include a negative test case to verify that the one-hot validation works correctly. Currently, all test cases use valid one-hot selectors, but there's no test to ensure that invalid selectors (with 0 bits set or multiple bits set) are properly rejected with an appropriate error message.
| are properly folded at compile time, avoiding unnecessary code generation. | ||
| """ | ||
|
|
||
| from assassyn.frontend import * |
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module assassyn.frontend does not define 'all'.
| from assassyn.frontend import * | |
| from assassyn.frontend import Module, module, Bits, log |
Summary
Implemented constant folding for
select1hotoperations to fix code generation issues when using all constant operands. When the selector is a constant, the operation is now evaluated at compile time and returns the selected value directly, avoiding unnecessary IR node creation. This follows the same pattern as existing constant folding inConst.concat()andConst.__getitem__().Changes
Modified
python/assassyn/ir/value.py:166-185to add constant folding logic inselect1hotmethodConstinstanceSelect1Hotnode creation for non-constant selectorsAdded
python/unit-tests/test_select1hot_constant_folding.pywith comprehensive test coverageUpdated
python/assassyn/ir/value.md:365-391to document constant folding behaviorUpdated
python/assassyn/ir/expr/expr.md:150to note automatic constant foldingSelect1Hotconstruction bypasses foldingValue.select1hot()method applies folding automaticallyUpdated
.gitignore:32to exclude.milestones/*directoryFixed
python/assassyn/codegen/verilog/elaborate.py:161,228-230pylint issuesTesting
All tests pass successfully (91 unit tests + 54 CI tests):
New test file:
python/unit-tests/test_select1hot_constant_folding.pyExisting tests: Verified
python/ci-tests/test_select1hot.pystill passesSelect1HotnodesCode quality: 10.00/10 pylint rating, all linting checks pass
Manual verification: IR dumps confirm:
(10:b5),(100:b8))Select1Hotnodes created as beforeRelated Issue
Closes #432