Kmeakin opened issue #7835:
Feature
The condition codes for performing the
>
and>=
comparisons on integers should be removed from the Cranelift IRBenefit
Makes the IR more strongly normalizing. At the moment, there are two equivalent ways of specifying the same computation:
x > y
andy < 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,
ugtand
ugecondition 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 edited issue #7835:
Feature
The condition codes for performing the
>
and>=
comparisons on integers should be removed from the Cranelift IRBenefit
Makes the IR more strongly normalizing. At the moment, there are two equivalent ways of specifying the same computation:
x > y
andy < 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
anduge
condition codes when consuming textual IR, but convert them to their swapped forms and swap the arguments toicmp
during parsing. Update legalization/optimization code to fix any codegen regressions.Alternatives
Do nothing
fitzgen commented on issue #7835:
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 added the cranelift label to Issue #7835.
fitzgen added the cranelift:area:clif label to Issue #7835.
bjorn3 commented on issue #7835:
For
icmp_imm
IntCC
will have to preserve them or we would need to addircmp_imm
like we haveirsub_imm
.
Kmeakin commented on issue #7835:
I can work on an implementation from this, but I will need to get permission from my employer before I can submit a PR
Solo-steven commented on issue #7835:
Hi I would like to take this issue if possible, but I am beginner of wasmtime, so might take a while to fix it !
Last updated: Nov 22 2024 at 16:03 UTC