T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

aarch64: Refactor lower_icmp to use a single materialize_bool_result

Browse Source
This commit is contained in:
Afonso Bordado
2021-07-16 00:25:03 +01:00
committed by Andrew Brown
parent 14d1c7ee9f
commit 3a38400447
2 changed files with 31 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
cranelift/codegen/src/isa/aarch64/lower.rs
View File

@@ -21,7 +21,6 @@ use crate::isa::aarch64::AArch64Backend;
use super::lower_inst; use super::lower_inst;
use crate::data_value::DataValue; use crate::data_value::DataValue;
use crate::ir::instructions::Opcode::Icmp;
use log::{debug, trace}; use log::{debug, trace};
use regalloc::{Reg, Writable}; use regalloc::{Reg, Writable};
use smallvec::SmallVec; use smallvec::SmallVec;
@@ -1256,17 +1255,17 @@ pub(crate) fn maybe_input_insn_via_conv<C: LowerCtx<I = Inst>>(
/// Specifies what [lower_icmp] should do when lowering /// Specifies what [lower_icmp] should do when lowering
#[derive(Debug, Clone, PartialEq)] #[derive(Debug, Clone, PartialEq)]
pub(crate) enum IcmpOutput { pub(crate) enum IcmpOutput {
/// Set flags, discarding the results. The flag to be used needs to be checked /// Lowers the comparison into a cond code, discarding the results. The cond code emitted can
/// in [IcmpResult] /// be checked in the resulting [IcmpResult].
Flags, CondCode,
/// Materializes the results into a register. The flags set may be incorrect /// Materializes the results into a register. This may overwrite any flags previously set.
Register(Writable<Reg>), Register(Writable<Reg>),
} }
impl IcmpOutput { impl IcmpOutput {
pub fn reg(&self) -> Option<Writable<Reg>> { pub fn reg(&self) -> Option<Writable<Reg>> {
match self { match self {
IcmpOutput::Flags => None, IcmpOutput::CondCode => None,
IcmpOutput::Register(reg) => Some(*reg), IcmpOutput::Register(reg) => Some(*reg),
} }
} }
@@ -1275,8 +1274,8 @@ impl IcmpOutput {
/// The output of an Icmp lowering. /// The output of an Icmp lowering.
#[derive(Debug, Clone, PartialEq)] #[derive(Debug, Clone, PartialEq)]
pub(crate) enum IcmpResult { pub(crate) enum IcmpResult {
/// The result was output into the flag corresponding with this [Cond]. Other flags are left in /// The result was output into the given [Cond]. Callers may perform operations using this [Cond]
/// an undefined state. /// and its inverse, other [Cond]'s are not guaranteed to be correct.
CondCode(Cond), CondCode(Cond),
/// The result was materialized into the output register. /// The result was materialized into the output register.
Register, Register,
@@ -1286,7 +1285,7 @@ impl IcmpResult {
pub fn unwrap_cond(&self) -> Cond { pub fn unwrap_cond(&self) -> Cond {
match self { match self {
IcmpResult::CondCode(c) => *c, IcmpResult::CondCode(c) => *c,
_ => panic!("Unwrapped flag, but IcmpResult was {:?}", self), _ => panic!("Unwrapped cond, but IcmpResult was {:?}", self),
} }
} }
} }
@@ -1318,6 +1317,7 @@ pub(crate) fn lower_icmp<C: LowerCtx<I = Inst>>(
(false, true) => NarrowValueMode::SignExtend64, (false, true) => NarrowValueMode::SignExtend64,
(false, false) => NarrowValueMode::ZeroExtend64, (false, false) => NarrowValueMode::ZeroExtend64,
}; };
let mut should_materialize = output.reg().is_some();
let out_condcode = if ty == I128 { let out_condcode = if ty == I128 {
let lhs = put_input_in_regs(ctx, inputs[0]); let lhs = put_input_in_regs(ctx, inputs[0]);
@@ -1351,10 +1351,6 @@ pub(crate) fn lower_icmp<C: LowerCtx<I = Inst>>(
rn: tmp1.to_reg(), rn: tmp1.to_reg(),
rm: tmp2.to_reg(), rm: tmp2.to_reg(),
}); });
if let IcmpOutput::Register(rd) = output {
materialize_bool_result(ctx, insn, rd, cond);
}
} }
IntCC::Overflow | IntCC::NotOverflow => { IntCC::Overflow | IntCC::NotOverflow => {
// We can do an 128bit add while throwing away the results // We can do an 128bit add while throwing away the results
@@ -1376,10 +1372,6 @@ pub(crate) fn lower_icmp<C: LowerCtx<I = Inst>>(
rn: lhs.regs()[1], rn: lhs.regs()[1],
rm: rhs.regs()[1], rm: rhs.regs()[1],
}); });
if let IcmpOutput::Register(rd) = output {
materialize_bool_result(ctx, insn, rd, cond);
}
} }
_ => { _ => {
// cmp lhs_lo, rhs_lo // cmp lhs_lo, rhs_lo
@@ -1412,7 +1404,7 @@ pub(crate) fn lower_icmp<C: LowerCtx<I = Inst>>(
rm: tmp2.to_reg(), rm: tmp2.to_reg(),
}); });
if output == IcmpOutput::Flags { if output == IcmpOutput::CondCode {
// We only need to guarantee that the flags for `cond` are correct, so we can // We only need to guarantee that the flags for `cond` are correct, so we can
// compare rd with 0 or 1 // compare rd with 0 or 1
@@ -1443,11 +1435,16 @@ pub(crate) fn lower_icmp<C: LowerCtx<I = Inst>>(
rm, rm,
}); });
} }
// Prevent a second materialize_bool_result to be emitted at the end of the function
should_materialize = false;
} }
} }
cond cond
} else if ty.is_vector() { } else if ty.is_vector() {
assert_ne!(output, IcmpOutput::Flags); assert_ne!(output, IcmpOutput::CondCode);
should_materialize = false;
let rn = put_input_in_reg(ctx, inputs[0], narrow_mode); let rn = put_input_in_reg(ctx, inputs[0], narrow_mode);
let rm = put_input_in_reg(ctx, inputs[1], narrow_mode); let rm = put_input_in_reg(ctx, inputs[1], narrow_mode);
lower_vector_compare(ctx, rd, rn, rm, ty, cond)?; lower_vector_compare(ctx, rd, rn, rm, ty, cond)?;
@@ -1496,17 +1493,20 @@ pub(crate) fn lower_icmp<C: LowerCtx<I = Inst>>(
let alu_op = choose_32_64(ty, ALUOp::SubS32, ALUOp::SubS64); let alu_op = choose_32_64(ty, ALUOp::SubS32, ALUOp::SubS64);
ctx.emit(alu_inst_imm12(alu_op, writable_zero_reg(), rn, rm)); ctx.emit(alu_inst_imm12(alu_op, writable_zero_reg(), rn, rm));
if let IcmpOutput::Register(rd) = output {
materialize_bool_result(ctx, insn, rd, cond);
}
cond cond
}; };
// Most of the comparisons above produce flags by default, if the caller requested the result
// in a register we materialize those flags into a register. Some branches do end up producing
// the result as a register by default, so we ignore those.
if should_materialize {
materialize_bool_result(ctx, insn, rd, cond);
}
Ok(match output { Ok(match output {
// We currently never emit a different register than what was asked for // We currently never emit a different register than what was asked for
IcmpOutput::Register(_) => IcmpResult::Register, IcmpOutput::Register(_) => IcmpResult::Register,
IcmpOutput::Flags => IcmpResult::CondCode(out_condcode), IcmpOutput::CondCode => IcmpResult::CondCode(out_condcode),
}) })
} }

13
cranelift/codegen/src/isa/aarch64/lower_inst.rs
View File

@@ -1656,7 +1656,7 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
maybe_input_insn_via_conv(ctx, flag_input, Opcode::Icmp, Opcode::Bint) maybe_input_insn_via_conv(ctx, flag_input, Opcode::Icmp, Opcode::Bint)
{ {
let condcode = ctx.data(icmp_insn).cond_code().unwrap(); let condcode = ctx.data(icmp_insn).cond_code().unwrap();
lower_icmp(ctx, icmp_insn, condcode, IcmpOutput::Flags)?.unwrap_cond() lower_icmp(ctx, icmp_insn, condcode, IcmpOutput::CondCode)?.unwrap_cond()
} else if let Some(fcmp_insn) = } else if let Some(fcmp_insn) =
maybe_input_insn_via_conv(ctx, flag_input, Opcode::Fcmp, Opcode::Bint) maybe_input_insn_via_conv(ctx, flag_input, Opcode::Fcmp, Opcode::Bint)
{ {
@@ -1722,7 +1722,7 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
// Verification ensures that the input is always a // Verification ensures that the input is always a
// single-def ifcmp. // single-def ifcmp.
let ifcmp_insn = maybe_input_insn(ctx, inputs[0], Opcode::Ifcmp).unwrap(); let ifcmp_insn = maybe_input_insn(ctx, inputs[0], Opcode::Ifcmp).unwrap();
let cond = lower_icmp(ctx, ifcmp_insn, condcode, IcmpOutput::Flags)?.unwrap_cond(); let cond = lower_icmp(ctx, ifcmp_insn, condcode, IcmpOutput::CondCode)?.unwrap_cond();
// csel.COND rd, rn, rm // csel.COND rd, rn, rm
let rd = get_output_reg(ctx, outputs[0]).only_reg().unwrap(); let rd = get_output_reg(ctx, outputs[0]).only_reg().unwrap();
@@ -2042,7 +2042,7 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
// Verification ensures that the input is always a single-def ifcmp. // Verification ensures that the input is always a single-def ifcmp.
let ifcmp_insn = maybe_input_insn(ctx, inputs[0], Opcode::Ifcmp).unwrap(); let ifcmp_insn = maybe_input_insn(ctx, inputs[0], Opcode::Ifcmp).unwrap();
lower_icmp(ctx, ifcmp_insn, condcode, IcmpOutput::Flags)?.unwrap_cond() lower_icmp(ctx, ifcmp_insn, condcode, IcmpOutput::CondCode)?.unwrap_cond()
} else { } else {
let condcode = ctx.data(insn).fp_cond_code().unwrap(); let condcode = ctx.data(insn).fp_cond_code().unwrap();
let cond = lower_fp_condcode(condcode); let cond = lower_fp_condcode(condcode);
@@ -3647,7 +3647,7 @@ pub(crate) fn lower_branch<C: LowerCtx<I = Inst>>(
{ {
let condcode = ctx.data(icmp_insn).cond_code().unwrap(); let condcode = ctx.data(icmp_insn).cond_code().unwrap();
let cond = let cond =
lower_icmp(ctx, icmp_insn, condcode, IcmpOutput::Flags)?.unwrap_cond(); lower_icmp(ctx, icmp_insn, condcode, IcmpOutput::CondCode)?.unwrap_cond();
let negated = op0 == Opcode::Brz; let negated = op0 == Opcode::Brz;
let cond = if negated { cond.invert() } else { cond }; let cond = if negated { cond.invert() } else { cond };
@@ -3698,7 +3698,8 @@ pub(crate) fn lower_branch<C: LowerCtx<I = Inst>>(
} }
Opcode::BrIcmp => { Opcode::BrIcmp => {
let condcode = ctx.data(branches[0]).cond_code().unwrap(); let condcode = ctx.data(branches[0]).cond_code().unwrap();
let cond = lower_icmp(ctx, branches[0], condcode, IcmpOutput::Flags)?.unwrap_cond(); let cond =
lower_icmp(ctx, branches[0], condcode, IcmpOutput::CondCode)?.unwrap_cond();
ctx.emit(Inst::CondBr { ctx.emit(Inst::CondBr {
taken, taken,
@@ -3716,7 +3717,7 @@ pub(crate) fn lower_branch<C: LowerCtx<I = Inst>>(
}; };
if let Some(ifcmp_insn) = maybe_input_insn(ctx, flag_input, Opcode::Ifcmp) { if let Some(ifcmp_insn) = maybe_input_insn(ctx, flag_input, Opcode::Ifcmp) {
let cond = let cond =
lower_icmp(ctx, ifcmp_insn, condcode, IcmpOutput::Flags)?.unwrap_cond(); lower_icmp(ctx, ifcmp_insn, condcode, IcmpOutput::CondCode)?.unwrap_cond();
ctx.emit(Inst::CondBr { ctx.emit(Inst::CondBr {
taken, taken,
not_taken, not_taken,