Three fixes to various SpiderMonkey-related issues:

- Properly mask constant values down to appropriate width when
  generating a constant value directly in aarch64 backend. This was a
  miscompilation introduced in the new-isel refactor. In combination
  with failure to respect NarrowValueMode, this resulted in a very
  subtle bug when an `i32` constant was used in bit-twiddling logic.

- Add support for `iadd_ifcout` in aarch64 backend as used in explicit
  heap-check mode. With this change, we no longer fail heap-related
  tests with the huge-heap-region mode disabled.

- Remove a panic that was occurring in some tests that are currently
  ignored on aarch64, by simply returning empty/default information in
  `value_label` functionality rather than touching unimplemented APIs.
  This is not a bugfix per-se, but removes confusing panic messages from
  `cargo test` output that might otherwise mislead.
This commit is contained in:
Chris Fallin
2020-06-04 19:13:53 -07:00
parent 00abfcd943
commit fc2a6f273b
10 changed files with 153 additions and 14 deletions

View File

@@ -212,9 +212,14 @@ pub(crate) fn input_to_reg<C: LowerCtx<I = Inst>>(
let from_bits = ty_bits(ty) as u8;
let inputs = ctx.get_input(input.insn, input.input);
let in_reg = if let Some(c) = inputs.constant {
let masked = if from_bits < 64 {
c & ((1u64 << from_bits) - 1)
} else {
c
};
// Generate constants fresh at each use to minimize long-range register pressure.
let to_reg = ctx.alloc_tmp(Inst::rc_for_type(ty).unwrap(), ty);
for inst in Inst::gen_constant(to_reg, c, ty).into_iter() {
for inst in Inst::gen_constant(to_reg, masked, ty).into_iter() {
ctx.emit(inst);
}
to_reg.to_reg()

View File

@@ -1252,7 +1252,15 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
Opcode::Trapif | Opcode::Trapff => {
let trap_info = (ctx.srcloc(insn), inst_trapcode(ctx.data(insn)).unwrap());
let cond = if op == Opcode::Trapif {
let cond = if maybe_input_insn(ctx, inputs[0], Opcode::IaddIfcout).is_some() {
let condcode = inst_condcode(ctx.data(insn)).unwrap();
let cond = lower_condcode(condcode);
// The flags must not have been clobbered by any other
// instruction between the iadd_ifcout and this instruction, as
// verified by the CLIF validator; so we can simply use the
// flags here.
cond
} else if op == Opcode::Trapif {
let condcode = inst_condcode(ctx.data(insn)).unwrap();
let cond = lower_condcode(condcode);
let is_signed = condcode_is_signed(condcode);
@@ -1852,6 +1860,35 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
});
}
Opcode::IaddIfcout => {
// This is a two-output instruction that is needed for the
// legalizer's explicit heap-check sequence, among possible other
// uses. Its second output is a flags output only ever meant to
// check for overflow using the
// `backend.unsigned_add_overflow_condition()` condition.
//
// Note that the CLIF validation will ensure that no flag-setting
// operation comes between this IaddIfcout and its use (e.g., a
// Trapif). Thus, we can rely on implicit communication through the
// processor flags rather than explicitly generating flags into a
// register. We simply use the variant of the add instruction that
// sets flags (`adds`) here.
// Ensure that the second output isn't directly called for: it
// should only be used by a flags-consuming op, which will directly
// understand this instruction and merge the comparison.
assert!(!ctx.is_reg_needed(insn, ctx.get_output(insn, 1).to_reg()));
// Now handle the iadd as above, except use an AddS opcode that sets
// flags.
let rd = output_to_reg(ctx, outputs[0]);
let rn = input_to_reg(ctx, inputs[0], NarrowValueMode::None);
let rm = input_to_rse_imm12(ctx, inputs[1], NarrowValueMode::None);
let ty = ty.unwrap();
let alu_op = choose_32_64(ty, ALUOp::AddS32, ALUOp::AddS64);
ctx.emit(alu_inst_imm12(alu_op, rd, rn, rm));
}
Opcode::IaddImm
| Opcode::ImulImm
| Opcode::UdivImm
@@ -1862,7 +1899,6 @@ pub(crate) fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
| Opcode::IaddCin
| Opcode::IaddIfcin
| Opcode::IaddCout
| Opcode::IaddIfcout
| Opcode::IaddCarry
| Opcode::IaddIfcarry
| Opcode::IsubBin

View File

@@ -1,5 +1,6 @@
//! ARM 64-bit Instruction Set Architecture.
use crate::ir::condcodes::IntCC;
use crate::ir::Function;
use crate::isa::Builder as IsaBuilder;
use crate::machinst::{
@@ -92,6 +93,19 @@ impl MachBackend for AArch64Backend {
fn reg_universe(&self) -> &RealRegUniverse {
&self.reg_universe
}
fn unsigned_add_overflow_condition(&self) -> IntCC {
// Unsigned `>=`; this corresponds to the carry flag set on aarch64, which happens on
// overflow of an add.
IntCC::UnsignedGreaterThanOrEqual
}
fn unsigned_sub_overflow_condition(&self) -> IntCC {
// unsigned `<`; this corresponds to the carry flag cleared on aarch64, which happens on
// underflow of a subtract (aarch64 follows a carry-cleared-on-borrow convention, the
// opposite of x86).
IntCC::UnsignedLessThan
}
}
/// Create a new `isa::Builder`.

View File

@@ -5,6 +5,7 @@ use alloc::boxed::Box;
use regalloc::RealRegUniverse;
use target_lexicon::Triple;
use crate::ir::condcodes::IntCC;
use crate::ir::Function;
use crate::isa::Builder as IsaBuilder;
use crate::machinst::pretty_print::ShowWithRRU;
@@ -84,6 +85,18 @@ impl MachBackend for X64Backend {
fn reg_universe(&self) -> &RealRegUniverse {
&self.reg_universe
}
fn unsigned_add_overflow_condition(&self) -> IntCC {
// Unsigned `>=`; this corresponds to the carry flag set on x86, which happens on
// overflow of an add.
IntCC::UnsignedGreaterThanOrEqual
}
fn unsigned_sub_overflow_condition(&self) -> IntCC {
// unsigned `>=`; this corresponds to the carry flag set on x86, which happens on
// underflow of a subtract (carry is borrow for subtract).
IntCC::UnsignedGreaterThanOrEqual
}
}
/// Create a new `isa::Builder`.