Wasmtime GitHub notifications bot (Apr 15 2023 at 15:22): Wasmtime GitHub notifications bot (Apr 15 2023 at 15:22):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
sorubefore the operation name.
Wasmtime GitHub notifications bot (Apr 15 2023 at 15:22):T0b1-iOS requested abrown for a review on PR #6218.
Wasmtime GitHub notifications bot (Apr 15 2023 at 15:22):T0b1-iOS requested wasmtime-compiler-reviewers for a review on PR #6218.
Wasmtime GitHub notifications bot (Apr 15 2023 at 15:23):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
sorubefore the operation name.
Wasmtime GitHub notifications bot (Apr 17 2023 at 17:37):afonso360 assigned PR #6218 to afonso360.
Wasmtime GitHub notifications bot (Apr 17 2023 at 18:23):abrown requested afonso360 for a review on PR #6218.
Wasmtime GitHub notifications bot (Apr 18 2023 at 12:17):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (Apr 18 2023 at 12:17):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (Apr 18 2023 at 12:17):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
DataValuenow have the names of the opcodes!
Wasmtime GitHub notifications bot (Apr 18 2023 at 12:28):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
sorubefore the operation name.Edit(afonso360): Closes #5398
Wasmtime GitHub notifications bot (Apr 18 2023 at 13:52):T0b1-iOS updated PR #6218 (assigned to afonso360).
Wasmtime GitHub notifications bot (Apr 18 2023 at 13:52):T0b1-iOS submitted PR review.
Wasmtime GitHub notifications bot (Apr 18 2023 at 13:52):T0b1-iOS created PR review comment:
I changed it. Hope it's better now
Wasmtime GitHub notifications bot (Apr 18 2023 at 14:00):afonso360 has enabled auto merge for PR #6218.
Wasmtime GitHub notifications bot (Apr 18 2023 at 14:00):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (Apr 18 2023 at 14:00):afonso360 created PR review comment:
Looks great, thanks!
Wasmtime GitHub notifications bot (Apr 18 2023 at 15:04):afonso360 merged PR #6218.
Last updated: Nov 22 2024 at 17:03 UTC