Wasmtime GitHub notifications bot (Oct 25 2022 at 17:01): Wasmtime GitHub notifications bot (Oct 25 2022 at 17:01):cfallin commented on issue #5118:
In general, we have the invariant that high bits are undefined -- this is what allows us to, e.g., lower an 8-bit add to an
addinstruction with no masking afterward. And we account for this elsewhere by zero/sign-extending where necessary or using appropriate instruction widths (e.g. only using an 8-bitcmpwhen comparing i8s on x64). I suspect maybe what's going on here is that something in the compare-to-interpreter path is using a too-wide type and failing to mask the type of the compiled execution?
Wasmtime GitHub notifications bot (Oct 25 2022 at 17:36):afonso360 commented on issue #5118:
And we account for this elsewhere by zero/sign-extending where necessary or using appropriate instruction widths (e.g. only using an 8-bit cmp when comparing i8s on x64). I suspect maybe what's going on here is that something in the compare-to-interpreter path is using a too-wide type and failing to mask the type of the compiled execution?
I'm not sure I understand, the runtests don't involve the interpreter (they do but not in
test run) and both pass on s390x without any changes.
Wasmtime GitHub notifications bot (Oct 25 2022 at 17:43):cfallin commented on issue #5118:
Ah, sorry, I had autocompleted that thought based on "the fuzzer found..." but I see now the issue is actually in the runtest harness code that compares expected and actual return values.
Specifically, the masking introduced on the bmask lowering for aarch64 shouldn't be necessary to get the runtest passing, I think; if the low 8 bits of the returned value are correct, then the compiled code is doing the right thing. (A
zextorsexton the return type would signify the opposite, i.e. that the whole register must be defined in the specified way.)
Wasmtime GitHub notifications bot (Oct 25 2022 at 17:46):cfallin commented on issue #5118:
Actually, I take all the above back -- I was misunderstanding where the masking was required. On the input side of the compare, it is indeed necessary, exactly because of the undefined-upper-bits invariant. Sorry about that!
Last updated: Nov 22 2024 at 16:03 UTC