Skip to content

fix(cranelift): correct sign extension operators to use ireduce result#12609

Open
radik878 wants to merge 1 commit intobytecodealliance:mainfrom
radik878:fix/sign-extend-bug
Open

fix(cranelift): correct sign extension operators to use ireduce result#12609
radik878 wants to merge 1 commit intobytecodealliance:mainfrom
radik878:fix/sign-extend-bug

Conversation

@radik878
Copy link

What was wrong: The sign extension operators (i32.extend8_s, i32.extend16_s, i64.extend8_s, i64.extend16_s, i64.extend32_s) were incorrectly discarding the ireduce result. The code was pushing the ireduce result to the value stack and then immediately popping a different value (the next value on the stack) for the sextend operation.
Changes: Store the ireduce result in a local variable and use it directly in sextend instead of popping from the stack.

@radik878 radik878 requested a review from a team as a code owner February 17, 2026 20:13
@radik878 radik878 requested review from fitzgen and removed request for a team February 17, 2026 20:13
@cfallin
Copy link
Member

cfallin commented Feb 17, 2026

@radik878 I don't think your description is correct.

The existing code pops, reduces, pushes; then pops, sign-extends, and pushes. The intermediate push-then-pop has the effect of carrying through the result of the reduce to the sign-extend. Nothing is discarded, and the "next value on the stack" is not used.

I didn't write this code so I don't know the intent for sure but I can imagine this is meant to express the operator as a composition of the two operations. It's a little odd, but it's not incorrect.

Your description ("fix") implies that you observed an incorrect execution. Did you, and if so can you show the test case here? And can you explain your reasoning a bit more?

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.

2 participants