wasmtime icon indicating copy to clipboard operation
wasmtime copied to clipboard

Remove `>` and `>=` comparisons from IR

Open Kmeakin opened this issue 1 year ago • 4 comments

Feature

The condition codes for performing the > and >= comparisons on integers should be removed from the Cranelift IR

Benefit

Makes the IR more strongly normalizing. At the moment, there are two equivalent ways of specifying the same computation: x > y and y < x are semantically equivalent but have different representations in the IR.

Having only one possible representation would mean fewer cases to consider when writing lowering/optimization code, and fewer tests have to be performed at runtime when legalizing/optimizing programs.

Implementation

Continue to accept the sgt, sge, ugt and uge condition codes when consuming textual IR, but convert them to their swapped forms and swap the arguments to icmp during parsing. Update legalization/optimization code to fix any codegen regressions.

Alternatives

Do nothing

Kmeakin avatar Jan 27 '24 21:01 Kmeakin

I think this is a good idea, however we will want to additionally make sure that the InstBuilder trait continues to have >/>= methods, but just swaps operands and emits </<= instructions. This is the kind of thing we've talked about before in other contexts around simplifying the IR, but also not losing ergonomics for frontends or saddling them with undue breaking changes.

fitzgen avatar Jan 29 '24 15:01 fitzgen

For icmp_imm IntCC will have to preserve them or we would need to add ircmp_imm like we have irsub_imm.

bjorn3 avatar Jan 29 '24 15:01 bjorn3

I can work on an implementation from this, but I will need to get permission from my employer before I can submit a PR

Kmeakin avatar Jan 29 '24 21:01 Kmeakin

Hi I would like to take this issue if possible, but I am beginner of wasmtime, so might take a while to fix it !

Solo-steven avatar Jul 28 '24 09:07 Solo-steven