Stream: git-wasmtime

Topic: wasmtime / issue #4426 MachInst::gen_constant should no l...

Wasmtime GitHub notifications bot (Jul 09 2022 at 17:39):

uweigand opened issue #4426:

In implementing SIMD support for s390x, I ran into an issue with the MachInst::gen_constant routine that all platforms currently have to define. In a platform that otherwise fully uses ISLE for code generation, having to implement that gen_constant routine is annoying, since to achive full performance, we'd really have to duplicate all the various ways to optimally generate each particular constant. Specifically vector constants, which I've now added, would have required significantly extending the gen_constant routine.

Looking at the sources, it appears gen_constant is now solely used by the common put_value_in_regs routine. I'm wondering if it wouldn't be better to handle constant rematerialization instead on the ISLE side, e.g. in the ISLE put_in_regs constructor? That should hopefully allow backends to re-use the ISLE rules for constant emission they already have ...

FYI @cfallin

Wasmtime GitHub notifications bot (Jul 11 2022 at 16:21):

akirilov-arm labeled issue #4426:

In implementing SIMD support for s390x, I ran into an issue with the MachInst::gen_constant routine that all platforms currently have to define. In a platform that otherwise fully uses ISLE for code generation, having to implement that gen_constant routine is annoying, since to achive full performance, we'd really have to duplicate all the various ways to optimally generate each particular constant. Specifically vector constants, which I've now added, would have required significantly extending the gen_constant routine.

Looking at the sources, it appears gen_constant is now solely used by the common put_value_in_regs routine. I'm wondering if it wouldn't be better to handle constant rematerialization instead on the ISLE side, e.g. in the ISLE put_in_regs constructor? That should hopefully allow backends to re-use the ISLE rules for constant emission they already have ...

FYI @cfallin

Wasmtime GitHub notifications bot (Jul 11 2022 at 16:21):

akirilov-arm labeled issue #4426:

In implementing SIMD support for s390x, I ran into an issue with the MachInst::gen_constant routine that all platforms currently have to define. In a platform that otherwise fully uses ISLE for code generation, having to implement that gen_constant routine is annoying, since to achive full performance, we'd really have to duplicate all the various ways to optimally generate each particular constant. Specifically vector constants, which I've now added, would have required significantly extending the gen_constant routine.

Looking at the sources, it appears gen_constant is now solely used by the common put_value_in_regs routine. I'm wondering if it wouldn't be better to handle constant rematerialization instead on the ISLE side, e.g. in the ISLE put_in_regs constructor? That should hopefully allow backends to re-use the ISLE rules for constant emission they already have ...

FYI @cfallin

Wasmtime GitHub notifications bot (Jul 11 2022 at 17:31):

fitzgen commented on issue #4426:

This makes sense to me; the current architecture is basically a left over of incremental migration to ISLE.

Wasmtime GitHub notifications bot (Dec 13 2022 at 21:03):

uweigand closed issue #4426:

In implementing SIMD support for s390x, I ran into an issue with the MachInst::gen_constant routine that all platforms currently have to define. In a platform that otherwise fully uses ISLE for code generation, having to implement that gen_constant routine is annoying, since to achive full performance, we'd really have to duplicate all the various ways to optimally generate each particular constant. Specifically vector constants, which I've now added, would have required significantly extending the gen_constant routine.

Looking at the sources, it appears gen_constant is now solely used by the common put_value_in_regs routine. I'm wondering if it wouldn't be better to handle constant rematerialization instead on the ISLE side, e.g. in the ISLE put_in_regs constructor? That should hopefully allow backends to re-use the ISLE rules for constant emission they already have ...

FYI @cfallin

Last updated: Nov 22 2024 at 16:03 UTC