Stream: git-wasmtime

Topic: wasmtime / PR #5046 cranelift: Fix i128 iconst on AArch64

Wasmtime GitHub notifications bot (Oct 12 2022 at 11:42):

afonso360 opened PR #5046 from aarch64-fix-i128-iconst to main:

:wave: Hey,

This is the first of the bugs that the fuzzer found in the new egraph work. On AArch64 iconst.i128 is not implemented. This PR implements it for values that fit in a u64, which is (I think) all the values that can be passed in by the frontend.

We've had discussions in the past about removing iconst.i128 (I can't find the link), and I think I've previously disagreed with this, but maybe we can restrict iconst to i64 and add a helper method to the frontend that does the right thing? so, for example for -1 we can lower it to iconst.i64 -1 + sextend, or iconst + iconst + iconcat etc..

What do you guys think about that?

cc: @cfallin

Wasmtime GitHub notifications bot (Oct 12 2022 at 11:56):

afonso360 edited PR #5046 from aarch64-fix-i128-iconst to main:

:wave: Hey,

This is the first of the bugs that the fuzzer found in the new egraph work. On AArch64 iconst.i128 is not implemented. This PR implements it for values that fit in a u64, which is (I think) all the values that can be passed in by the frontend.

We've had discussions in the past about removing iconst.i128 (I can't find the link), and I think I've previously disagreed with this, but maybe we can restrict iconst to i64 and add a helper method to the frontend that does the right thing? so, for example for -1 we can lower it to iconst.i64 -1 + sextend, or iconst + iconst + iconcat for other values etc..

What do you guys think about that?

cc: @cfallin

Last updated: Dec 23 2024 at 13:07 UTC