Skip to content

Fix rotate_left and add rotate_right intrinsic - #1098

Draft
lucasmt wants to merge 2 commits into
masterfrom
rotation-fixes
Draft

lucasmt wants to merge 2 commits into
masterfrom
rotation-fixes

Conversation

@lucasmt

@lucasmt lucasmt commented Oct 14, 2026

Copy link
Copy Markdown

This PR fixes the implementation of the rotate_left intrinsic and adds a corresponding rotate_right, along with tests for those operators.

Specifically, the two bugs fixed in rotate_left are the following:

  • #rotateLeftInt shifts the value left without truncating, so the bits that wrap around also stay above the type's width (e.g. 0x8000_0000_u32 rotated left by 1 was stored as 0x1_0000_0001). The existing test missed this because the r == c assertion was compiled into a SwitchInt on r, and #switchMatch already inserts a truncation before comparing. However, in other cases, the missing truncation can lead to incorrect behavior. For example, a shift right could incorrectly bring the extra bits back. This was fixed by having #execRotateLeft truncate the result of #rotateLeftInt, and a new test was added covering this case.
  • The argument was being converted to an unsigned integer before rotating and not being converted back afterwards, causing signed integers that should be negative when rotated to instead be represented by KMIR as a positive integer. The addition of the truncation to fix the previous bug solves this as well, since truncate takes an argument specifying whether the result should be interpreted as signed or unsigned. However, this required splitting the definition of the rule to handle each case separately.

rotate_right was then implemented following the corrected rotate_left implementation, with corresponding tests added as well.

…t behavior

rotateLeftInt shifts the value left without masking, so the bits that
wrap around also stay above the type's width (e.g. 0x8000_0000_u32
rotated left by 1 was stored as 0x1_0000_0001), and signed types got the
unsigned representation. The existing test did not notice because its
`r == c` assertion becomes a SwitchInt on `r`, and #switchMatch truncates
the discriminant before comparing.
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.

1 participant