Skip to content

Conversation

@Scheremo
Copy link
Contributor

Fixes incorrect zext/sext widths and mismatched constant signs in dynamic part-select extraction.

  • Correct getSelectIndex to size and sign-extend indices based on full range bounds:
    • Signed if any bound is negative.
    • Bit-width = max(log2(max(|lo|, |hi|)+1), incoming width), with sign bit if signed.
  • Apply consistent signedness to offset constants.
  • Add regression tests:
    • partselect_index_pos_be: big-endian [1:4], positive offset.
    • partselect_index_neg_le: little-endian [0:-3], negative offset.

Further fixes third_party/tools/icarus/ivtest/ivltests/pr3054101[e-h].v in the chips alliance tests.

@Scheremo Scheremo marked this pull request as ready for review October 19, 2025 20:30
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

LGTM!

@Scheremo Scheremo force-pushed the pr-partsel-crash branch 3 times, most recently from 4655596 to de2e460 Compare October 20, 2025 18:46
Fixes incorrect zext/sext widths and mismatched constant signs in dynamic part-select extraction.

- Correct `getSelectIndex` to size and sign-extend indices based on full range bounds:
  - Signed if any bound is negative.
  - Bit-width = max(log2(max(|lo|, |hi|)+1), incoming width), with sign bit if signed.
- Apply consistent signedness to offset constants.
- Add regression tests:
  - `partselect_index_pos_be`: big-endian `[1:4]`, positive offset.
  - `partselect_index_neg_le`: little-endian `[0:-3]`, negative offset.

Further fixes third_party/tools/icarus/ivtest/ivltests/pr3054101[e-h].v in the chips alliance tests.
@Scheremo Scheremo merged commit 0fca2bd into llvm:main Oct 20, 2025
7 checks passed
@Scheremo Scheremo deleted the pr-partsel-crash branch October 21, 2025 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants