Browse Source

Fix ImmLogic.invert(), and with it, `fcopysign` and `float_misc` test.

Previously, `fcopysign` was mysteriously failing to pass the
`float_misc` spec test. This was tracked down to bad logical-immediate
masks used to separate the sign and not-sign bits. In particular, the
masks for the and-not operations were wrong. The `invert()` function on
an `ImmLogic` immediate, it turns out, assumed every immediate would be
used by a 64-bit instruction; `ImmLogic` immediates are subtly different
for 32-bit instructions. This change tracks the instruction size (32 or
64 bits) intended for use with each such immediate, and passes it back
into `maybe_from_u64` when computing the inverted immediate.

Addresses several of the failures (`float_misc`, `f32_bitwise`) for
 #1521 (test failures) and presumably helps #1519 (SpiderMonkey
integration).
pull/1582/head
Chris Fallin 5 years ago
parent
commit
8f462db645
  1. 2
      build.rs
  2. 14
      cranelift/codegen/src/isa/aarch64/inst/args.rs
  3. 50
      cranelift/codegen/src/isa/aarch64/inst/imms.rs
  4. 14
      cranelift/codegen/src/isa/aarch64/lower.rs

2
build.rs

@ -243,8 +243,6 @@ fn should_panic(testsuite: &str, testname: &str) -> bool {
| ("multi_value", "call")
| ("spec_testsuite", "call")
| ("spec_testsuite", "conversions")
| ("spec_testsuite", "f32_bitwise")
| ("spec_testsuite", "float_misc")
| ("spec_testsuite", "i32")
| ("spec_testsuite", "i64")
| ("spec_testsuite", "int_exprs")

14
cranelift/codegen/src/isa/aarch64/inst/args.rs

@ -6,6 +6,7 @@
use crate::binemit::CodeOffset;
use crate::ir::Type;
use crate::isa::aarch64::inst::*;
use crate::isa::aarch64::lower::ty_bits;
use regalloc::{RealRegUniverse, Reg, Writable};
@ -526,6 +527,19 @@ impl InstSize {
}
}
/// Convert from an integer type into the smallest size that fits.
pub fn from_ty(ty: Type) -> InstSize {
Self::from_bits(ty_bits(ty))
}
/// Convert to I32 or I64.
pub fn to_ty(self) -> Type {
match self {
InstSize::Size32 => I32,
InstSize::Size64 => I64,
}
}
pub fn sf_bit(&self) -> u32 {
match self {
InstSize::Size32 => 0,

50
cranelift/codegen/src/isa/aarch64/inst/imms.rs

@ -4,6 +4,7 @@
#[allow(dead_code)]
use crate::ir::types::*;
use crate::ir::Type;
use crate::isa::aarch64::inst::InstSize;
use crate::machinst::*;
use regalloc::RealRegUniverse;
@ -233,6 +234,8 @@ pub struct ImmLogic {
pub r: u8,
/// `R` field: rotate amount.
pub s: u8,
/// Was this constructed for a 32-bit or 64-bit instruction?
pub size: InstSize,
}
impl ImmLogic {
@ -243,6 +246,7 @@ impl ImmLogic {
if ty != I64 && ty != I32 {
return None;
}
let inst_size = InstSize::from_ty(ty);
let original_value = value;
@ -423,13 +427,10 @@ impl ImmLogic {
n: out_n != 0,
r: r as u8,
s: s as u8,
size: inst_size,
})
}
pub fn from_raw(value: u64, n: bool, r: u8, s: u8) -> ImmLogic {
ImmLogic { n, r, s, value }
}
/// Returns bits ready for encoding: (N:1, R:6, S:6)
pub fn enc_bits(&self) -> u32 {
((self.n as u32) << 12) | ((self.r as u32) << 6) | (self.s as u32)
@ -443,7 +444,7 @@ impl ImmLogic {
/// Return an immediate for the bitwise-inverted value.
pub fn invert(&self) -> ImmLogic {
// For every ImmLogical immediate, the inverse can also be encoded.
Self::maybe_from_u64(!self.value, I64).unwrap()
Self::maybe_from_u64(!self.value, self.size.to_ty()).unwrap()
}
}
@ -613,7 +614,8 @@ mod test {
value: 1,
n: true,
r: 0,
s: 0
s: 0,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(1, I64)
);
@ -623,7 +625,8 @@ mod test {
value: 2,
n: true,
r: 63,
s: 0
s: 0,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(2, I64)
);
@ -637,7 +640,8 @@ mod test {
value: 248,
n: true,
r: 61,
s: 4
s: 4,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(248, I64)
);
@ -649,7 +653,8 @@ mod test {
value: 1920,
n: true,
r: 57,
s: 3
s: 3,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(1920, I64)
);
@ -659,7 +664,8 @@ mod test {
value: 0x7ffe,
n: true,
r: 63,
s: 13
s: 13,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(0x7ffe, I64)
);
@ -669,7 +675,8 @@ mod test {
value: 0x30000,
n: true,
r: 48,
s: 1
s: 1,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(0x30000, I64)
);
@ -679,7 +686,8 @@ mod test {
value: 0x100000,
n: true,
r: 44,
s: 0
s: 0,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(0x100000, I64)
);
@ -689,7 +697,8 @@ mod test {
value: u64::max_value() - 1,
n: true,
r: 63,
s: 62
s: 62,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(u64::max_value() - 1, I64)
);
@ -699,7 +708,8 @@ mod test {
value: 0xaaaaaaaaaaaaaaaa,
n: false,
r: 1,
s: 60
s: 60,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(0xaaaaaaaaaaaaaaaa, I64)
);
@ -709,7 +719,8 @@ mod test {
value: 0x8181818181818181,
n: false,
r: 1,
s: 49
s: 49,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(0x8181818181818181, I64)
);
@ -719,7 +730,8 @@ mod test {
value: 0xffc3ffc3ffc3ffc3,
n: false,
r: 10,
s: 43
s: 43,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(0xffc3ffc3ffc3ffc3, I64)
);
@ -729,7 +741,8 @@ mod test {
value: 0x100000001,
n: false,
r: 0,
s: 0
s: 0,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(0x100000001, I64)
);
@ -739,7 +752,8 @@ mod test {
value: 0x1111111111111111,
n: false,
r: 0,
s: 56
s: 56,
size: InstSize::Size64,
}),
ImmLogic::maybe_from_u64(0x1111111111111111, I64)
);

14
cranelift/codegen/src/isa/aarch64/lower.rs

@ -2105,19 +2105,9 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(ctx: &mut C, insn: IRInst) {
ctx.emit(Inst::MovFromVec64 { rd: tmp1, rn: rn });
ctx.emit(Inst::MovFromVec64 { rd: tmp2, rn: rm });
let imml = if bits == 32 {
ImmLogic::from_raw(
/* value = */ 0x8000_0000,
/* n = */ false,
/* r = */ 1,
/* s = */ 0,
)
ImmLogic::maybe_from_u64(0x8000_0000, I32).unwrap()
} else {
ImmLogic::from_raw(
/* value = */ 0x8000_0000_0000_0000,
/* n = */ true,
/* r = */ 1,
/* s = */ 0,
)
ImmLogic::maybe_from_u64(0x8000_0000_0000_0000, I64).unwrap()
};
let alu_op = choose_32_64(ty, ALUOp::And32, ALUOp::And64);
ctx.emit(Inst::AluRRImmLogic {

Loading…
Cancel
Save