T0b1-iOS opened PR #6218 from T0b1-iOS:rem_unsigned_data_values
to bytecodealliance:main
:
As discussed in issue #5398 and Zulip previously, the unsigned variants of DataValue do not make a lot of sense given that Cranelift does not distinguish between signed/unsigned values in the IR and the operations specify signedness when it matters.
This changes makes the interpreter reflect that by using explicit signed/unsigned operations when required that follow the IR naming scheme by prepending
s
oru
before the operation name.
T0b1-iOS requested abrown for a review on PR #6218.
T0b1-iOS requested wasmtime-compiler-reviewers for a review on PR #6218.
T0b1-iOS edited PR #6218:
As discussed in issue #5398 and Zulip previously, the unsigned variants of DataValue do not make a lot of sense given that Cranelift does not distinguish between signed/unsigned values in the IR and the operations specify signedness when it matters.
Given that there seems to have been no public progress for a couple of months I decided to pick it up.This changes makes the interpreter reflect that by using explicit signed/unsigned operations when required that follow the IR naming scheme by prepending
s
oru
before the operation name.
afonso360 assigned PR #6218 to afonso360.
abrown requested afonso360 for a review on PR #6218.
afonso360 submitted PR review.
afonso360 submitted PR review.
afonso360 created PR review comment:
Small nit about these. If we're following the IR naming scheme I think these would be better named as
sadd_overflow
/smul_overflow
/etc.. since that is the name of the opcode.It's also really neat that the other operations in
DataValue
now have the names of the opcodes!
afonso360 edited PR #6218 (assigned to afonso360):
As discussed in issue #5398 and Zulip previously, the unsigned variants of DataValue do not make a lot of sense given that Cranelift does not distinguish between signed/unsigned values in the IR and the operations specify signedness when it matters.
Given that there seems to have been no public progress for a couple of months I decided to pick it up.This changes makes the interpreter reflect that by using explicit signed/unsigned operations when required that follow the IR naming scheme by prepending
s
oru
before the operation name.Edit(afonso360): Closes #5398
T0b1-iOS updated PR #6218 (assigned to afonso360).
T0b1-iOS submitted PR review.
T0b1-iOS created PR review comment:
I changed it. Hope it's better now
afonso360 has enabled auto merge for PR #6218.
afonso360 submitted PR review.
afonso360 created PR review comment:
Looks great, thanks!
afonso360 merged PR #6218.
Last updated: Dec 23 2024 at 13:07 UTC