Skip to content

Fix i64x2 shift on big-endian #8748

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kripken merged 1 commit into from
May 21, 2026
+5 −4
Merged

Fix i64x2 shift on big-endian #8748

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/wasm/literal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1483,8 +1483,8 @@ Literal Literal::shl(const Literal& other) const {
return Literal(uint32_t(i32)
<< Bits::getEffectiveShifts(other.i32, Type::i32));
case Type::i64:
return Literal(uint64_t(i64)
<< Bits::getEffectiveShifts(other.i64, Type::i64));
return Literal(uint64_t(i64) << Bits::getEffectiveShifts(
other.getInteger(), Type::i64));
Copy link
Copy Markdown
Member

@kripken kripken May 21, 2026

Choose a reason for hiding this comment

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

Hmm, by the wasm validation rules, the type of other must be identical. Are you seeing a situation where it is not?

Copy link
Copy Markdown
Contributor Author

@sertonix sertonix May 21, 2026

Choose a reason for hiding this comment

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

Not with the indirect usage of Literal::shl through Literal::shlI64x2:

binaryen/src/wasm/literal.cpp

Lines 2264 to 2266 in 2f63efb

Literal Literal::shlI64x2(const Literal& other) const {
return shift<2, &Literal::getLanesI64x2, &Literal::shl>(*this, other);
}

binaryen/src/wasm/wasm-validator.cpp

Lines 1616 to 1628 in 2f63efb

void FunctionValidator::visitSIMDShift(SIMDShift* curr) {
shouldBeTrue(getModule()->features.hasSIMD(),
curr,
"SIMD operations require SIMD [--enable-simd]");
shouldBeEqualOrFirstIsUnreachable(
curr->type, Type(Type::v128), curr, "vector shift must have type v128");
shouldBeEqualOrFirstIsUnreachable(
curr->vec->type, Type(Type::v128), curr, "expected operand of type v128");
shouldBeEqualOrFirstIsUnreachable(curr->shift->type,
Type(Type::i32),
curr,
"expected shift amount to have type i32");
}

Copy link
Copy Markdown
Member

@kripken kripken May 21, 2026

Choose a reason for hiding this comment

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

I see, thanks. I guess this makes sense - wasm semantics are more strict than our Literal APIs should allow. I was confused before.

default:
WASM_UNREACHABLE("unexpected type");
}
Expand All @@ -1495,7 +1495,8 @@ Literal Literal::shrS(const Literal& other) const {
case Type::i32:
return Literal(i32 >> Bits::getEffectiveShifts(other.i32, Type::i32));
case Type::i64:
return Literal(i64 >> Bits::getEffectiveShifts(other.i64, Type::i64));
return Literal(i64 >>
Bits::getEffectiveShifts(other.getInteger(), Type::i64));
default:
WASM_UNREACHABLE("unexpected type");
}
Expand All @@ -1508,7 +1509,7 @@ Literal Literal::shrU(const Literal& other) const {
Bits::getEffectiveShifts(other.i32, Type::i32));
case Type::i64:
return Literal(uint64_t(i64) >>
Bits::getEffectiveShifts(other.i64, Type::i64));
Bits::getEffectiveShifts(other.getInteger(), Type::i64));
default:
WASM_UNREACHABLE("unexpected type");
}
Expand Down
Loading