Cranelift: fix branch-of-icmp/fcmp regression: look through uextend. (#5487)
In #5031, we removed `bool` types from CLIF, using integers instead for "truthy" values. This greatly simplified the IR, and was generally an improvement. However, because x86's `SETcc` instruction sets only the low 8 bits of a register, we chose to use `i8` types as the result of `icmp` and `fcmp`, to avoid the need for a masking operation when materializing the result. Unfortunately this means that uses of truthy values often now have `uextend` operations, especially when coming from Wasm (where truthy values are naturally `i32`-typed). For example, where we previously had `(brz (icmp ...))`, we now have `(brz (uextend (icmp ...)))`. It's arguable whether or not we should switch to `i32` truthy values -- in most cases we can avoid materializing a value that's immediately used for a branch or select, so a mask would in most cases be unnecessary, and it would be a win at the IR level -- but irrespective of that, this change *did* regress our generated code quality: our backends had patterns for e.g. `(brz (icmp ...))` but not with the `uextend`, so we were *always* materializing truthy values. Many blocks thus ended with "cmp; setcc; cmp; test; branch" rather than "cmp; branch". In #5391 we noticed this and fixed it on x64, but it was a general problem on aarch64 and riscv64 as well. This PR introduces a `maybe_uextend` extractor that "looks through" uextends, and uses it where we consume truthy values, thus fixing the regression. This PR also adds compile filetests to ensure we don't regress again. The riscv64 backend has not been updated here because doing so appears to trigger another issue in its branch handling; fixing that is TBD.
This commit is contained in:
@@ -547,6 +547,21 @@ macro_rules! isle_lower_prelude_methods {
|
||||
self.lower_ctx.sink_inst(inst);
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn maybe_uextend(&mut self, value: Value) -> Option<Value> {
|
||||
if let Some(def_inst) = self.def_inst(value) {
|
||||
if let InstructionData::Unary {
|
||||
opcode: Opcode::Uextend,
|
||||
arg,
|
||||
} = self.lower_ctx.data(def_inst)
|
||||
{
|
||||
return Some(*arg);
|
||||
}
|
||||
}
|
||||
|
||||
Some(value)
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn preg_to_reg(&mut self, preg: PReg) -> Reg {
|
||||
preg.into()
|
||||
|
||||
Reference in New Issue
Block a user