Skip to content

Commit

Permalink
Fixes for AArch64 (#338)
Browse files Browse the repository at this point in the history
* Better splitting for Op::Add, Op::Sub, and Op::Cmp

* Split stores if the displacement is too large

* Use a shifted immediate argument

* Split all places where shifted immediates are used

* Add more tests to the cirrus workflow
  • Loading branch information
kddnewton authored and k0kubun committed Aug 25, 2022
1 parent d29d172 commit 4cda346
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 109 deletions.
3 changes: 3 additions & 0 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,17 @@ yjit_task:
bootstraptest/test_flow.rb \
bootstraptest/test_fork.rb \
bootstraptest/test_gc.rb \
bootstraptest/test_io.rb \
bootstraptest/test_jump.rb \
bootstraptest/test_literal_suffix.rb \
bootstraptest/test_load.rb \
bootstraptest/test_marshal.rb \
bootstraptest/test_massign.rb \
bootstraptest/test_method.rb \
bootstraptest/test_objectspace.rb \
bootstraptest/test_proc.rb \
bootstraptest/test_string.rb \
bootstraptest/test_struct.rb \
bootstraptest/test_yjit_new_backend.rb
bootstraptest/test_yjit_rust_port.rb
# full_build_script: make -j
2 changes: 2 additions & 0 deletions yjit/src/asm/arm64/arg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
mod bitmask_imm;
mod condition;
mod sf;
mod shifted_imm;
mod sys_reg;

pub use bitmask_imm::BitmaskImmediate;
pub use condition::Condition;
pub use sf::Sf;
pub use shifted_imm::ShiftedImmediate;
pub use sys_reg::SystemRegister;
75 changes: 75 additions & 0 deletions yjit/src/asm/arm64/arg/shifted_imm.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/// How much to shift the immediate by.
pub enum Shift {
LSL0 = 0b0, // no shift
LSL12 = 0b1 // logical shift left by 12 bits
}

/// Some instructions accept a 12-bit immediate that has an optional shift
/// attached to it. This allows encoding larger values than just fit into 12
/// bits. We attempt to encode those here. If the values are too large we have
/// to bail out.
pub struct ShiftedImmediate {
shift: Shift,
value: u16
}

impl TryFrom<u64> for ShiftedImmediate {
type Error = ();

/// Attempt to convert a u64 into a BitmaskImm.
fn try_from(value: u64) -> Result<Self, Self::Error> {
let mut current = value;
if current < 2_u64.pow(12) {
return Ok(ShiftedImmediate { shift: Shift::LSL0, value: current as u16 });
}

if (current & (2_u64.pow(12) - 1) == 0) && ((current >> 12) < 2_u64.pow(12)) {
return Ok(ShiftedImmediate { shift: Shift::LSL12, value: (current >> 12) as u16 });
}

Err(())
}
}

impl From<ShiftedImmediate> for u32 {
/// Encode a bitmask immediate into a 32-bit value.
fn from(imm: ShiftedImmediate) -> Self {
0
| (((imm.shift as u32) & 1) << 12)
| (imm.value as u32)
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_no_shift() {
let value = 256;
let result = ShiftedImmediate::try_from(value);

assert!(matches!(result, Ok(ShiftedImmediate { shift: Shift::LSL0, value })));
}

#[test]
fn test_maximum_no_shift() {
let value = (1 << 12) - 1;
let result = ShiftedImmediate::try_from(value);

assert!(matches!(result, Ok(ShiftedImmediate { shift: Shift::LSL0, value })));
}

#[test]
fn test_with_shift() {
let result = ShiftedImmediate::try_from(256 << 12);

assert!(matches!(result, Ok(ShiftedImmediate { shift: Shift::LSL12, value: 256 })));
}

#[test]
fn test_unencodable() {
let result = ShiftedImmediate::try_from((256 << 12) + 1);
assert!(matches!(result, Err(())));
}
}
80 changes: 19 additions & 61 deletions yjit/src/asm/arm64/inst/data_imm.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::super::arg::Sf;
use super::super::arg::{Sf, ShiftedImmediate};

/// The operation being performed by this instruction.
enum Op {
Expand All @@ -12,12 +12,6 @@ enum S {
UpdateFlags = 0b1
}

/// How much to shift the immediate by.
enum Shift {
LSL0 = 0b0, // no shift
LSL12 = 0b1 // logical shift left by 12 bits
}

/// The struct that represents an A64 data processing -- immediate instruction
/// that can be encoded.
///
Expand All @@ -35,11 +29,8 @@ pub struct DataImm {
/// The register number of the first operand register.
rn: u8,

/// The value of the immediate.
imm12: u16,

/// How much to shift the immediate by.
shift: Shift,
imm: ShiftedImmediate,

/// Whether or not to update the flags when this instruction is performed.
s: S,
Expand All @@ -54,64 +45,32 @@ pub struct DataImm {
impl DataImm {
/// ADD (immediate)
/// https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/ADD--immediate---Add--immediate--?lang=en
pub fn add(rd: u8, rn: u8, imm12: u16, num_bits: u8) -> Self {
Self {
rd,
rn,
imm12,
shift: Shift::LSL0,
s: S::LeaveFlags,
op: Op::Add,
sf: num_bits.into()
}
pub fn add(rd: u8, rn: u8, imm: ShiftedImmediate, num_bits: u8) -> Self {
Self { rd, rn, imm, s: S::LeaveFlags, op: Op::Add, sf: num_bits.into() }
}

/// ADDS (immediate, set flags)
/// https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/ADDS--immediate---Add--immediate---setting-flags-?lang=en
pub fn adds(rd: u8, rn: u8, imm12: u16, num_bits: u8) -> Self {
Self {
rd,
rn,
imm12,
shift: Shift::LSL0,
s: S::UpdateFlags,
op: Op::Add,
sf: num_bits.into()
}
pub fn adds(rd: u8, rn: u8, imm: ShiftedImmediate, num_bits: u8) -> Self {
Self { rd, rn, imm, s: S::UpdateFlags, op: Op::Add, sf: num_bits.into() }
}

/// CMP (immediate)
/// https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/CMP--immediate---Compare--immediate---an-alias-of-SUBS--immediate--?lang=en
pub fn cmp(rn: u8, imm12: u16, num_bits: u8) -> Self {
Self::subs(31, rn, imm12, num_bits)
pub fn cmp(rn: u8, imm: ShiftedImmediate, num_bits: u8) -> Self {
Self::subs(31, rn, imm, num_bits)
}

/// SUB (immediate)
/// https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/SUB--immediate---Subtract--immediate--?lang=en
pub fn sub(rd: u8, rn: u8, imm12: u16, num_bits: u8) -> Self {
Self {
rd,
rn,
imm12,
shift: Shift::LSL0,
s: S::LeaveFlags,
op: Op::Sub,
sf: num_bits.into()
}
pub fn sub(rd: u8, rn: u8, imm: ShiftedImmediate, num_bits: u8) -> Self {
Self { rd, rn, imm, s: S::LeaveFlags, op: Op::Sub, sf: num_bits.into() }
}

/// SUBS (immediate, set flags)
/// https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/SUBS--immediate---Subtract--immediate---setting-flags-?lang=en
pub fn subs(rd: u8, rn: u8, imm12: u16, num_bits: u8) -> Self {
Self {
rd,
rn,
imm12,
shift: Shift::LSL0,
s: S::UpdateFlags,
op: Op::Sub,
sf: num_bits.into()
}
pub fn subs(rd: u8, rn: u8, imm: ShiftedImmediate, num_bits: u8) -> Self {
Self { rd, rn, imm, s: S::UpdateFlags, op: Op::Sub, sf: num_bits.into() }
}
}

Expand All @@ -121,16 +80,15 @@ const FAMILY: u32 = 0b1000;
impl From<DataImm> for u32 {
/// Convert an instruction into a 32-bit value.
fn from(inst: DataImm) -> Self {
let imm12 = (inst.imm12 as u32) & ((1 << 12) - 1);
let imm: u32 = inst.imm.into();

0
| ((inst.sf as u32) << 31)
| ((inst.op as u32) << 30)
| ((inst.s as u32) << 29)
| (FAMILY << 25)
| (1 << 24)
| ((inst.shift as u32) << 22)
| (imm12 << 10)
| (imm << 10)
| ((inst.rn as u32) << 5)
| inst.rd as u32
}
Expand All @@ -150,35 +108,35 @@ mod tests {

#[test]
fn test_add() {
let inst = DataImm::add(0, 1, 7, 64);
let inst = DataImm::add(0, 1, 7.try_into().unwrap(), 64);
let result: u32 = inst.into();
assert_eq!(0x91001c20, result);
}

#[test]
fn test_adds() {
let inst = DataImm::adds(0, 1, 7, 64);
let inst = DataImm::adds(0, 1, 7.try_into().unwrap(), 64);
let result: u32 = inst.into();
assert_eq!(0xb1001c20, result);
}

#[test]
fn test_cmp() {
let inst = DataImm::cmp(0, 7, 64);
let inst = DataImm::cmp(0, 7.try_into().unwrap(), 64);
let result: u32 = inst.into();
assert_eq!(0xf1001c1f, result);
}

#[test]
fn test_sub() {
let inst = DataImm::sub(0, 1, 7, 64);
let inst = DataImm::sub(0, 1, 7.try_into().unwrap(), 64);
let result: u32 = inst.into();
assert_eq!(0xd1001c20, result);
}

#[test]
fn test_subs() {
let inst = DataImm::subs(0, 1, 7, 64);
let inst = DataImm::subs(0, 1, 7.try_into().unwrap(), 64);
let result: u32 = inst.into();
assert_eq!(0xf1001c20, result);
}
Expand Down
40 changes: 15 additions & 25 deletions yjit/src/asm/arm64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,16 @@ pub fn add(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, rm: A64Opnd) {
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::UImm(uimm12)) => {
assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size.");
assert!(uimm_fits_bits(uimm12, 12), "The immediate operand must be 12 bits or less.");

DataImm::add(rd.reg_no, rn.reg_no, uimm12 as u16, rd.num_bits).into()
DataImm::add(rd.reg_no, rn.reg_no, uimm12.try_into().unwrap(), rd.num_bits).into()
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::Imm(imm12)) => {
assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size.");
assert!(imm_fits_bits(imm12, 12), "The immediate operand must be 12 bits or less.");

if imm12 < 0 {
DataImm::sub(rd.reg_no, rn.reg_no, -imm12 as u16, rd.num_bits).into()
DataImm::sub(rd.reg_no, rn.reg_no, (-imm12 as u64).try_into().unwrap(), rd.num_bits).into()
} else {
DataImm::add(rd.reg_no, rn.reg_no, imm12 as u16, rd.num_bits).into()
DataImm::add(rd.reg_no, rn.reg_no, (imm12 as u64).try_into().unwrap(), rd.num_bits).into()
}
},
_ => panic!("Invalid operand combination to add instruction."),
Expand All @@ -74,18 +72,16 @@ pub fn adds(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, rm: A64Opnd) {
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::UImm(imm12)) => {
assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size.");
assert!(uimm_fits_bits(imm12, 12), "The immediate operand must be 12 bits or less.");

DataImm::adds(rd.reg_no, rn.reg_no, imm12 as u16, rd.num_bits).into()
DataImm::adds(rd.reg_no, rn.reg_no, imm12.try_into().unwrap(), rd.num_bits).into()
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::Imm(imm12)) => {
assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size.");
assert!(imm_fits_bits(imm12, 12), "The immediate operand must be 12 bits or less.");

if imm12 < 0 {
DataImm::subs(rd.reg_no, rn.reg_no, -imm12 as u16, rd.num_bits).into()
DataImm::subs(rd.reg_no, rn.reg_no, (-imm12 as u64).try_into().unwrap(), rd.num_bits).into()
} else {
DataImm::adds(rd.reg_no, rn.reg_no, imm12 as u16, rd.num_bits).into()
DataImm::adds(rd.reg_no, rn.reg_no, (imm12 as u64).try_into().unwrap(), rd.num_bits).into()
}
},
_ => panic!("Invalid operand combination to adds instruction."),
Expand Down Expand Up @@ -272,9 +268,7 @@ pub fn cmp(cb: &mut CodeBlock, rn: A64Opnd, rm: A64Opnd) {
DataReg::cmp(rn.reg_no, rm.reg_no, rn.num_bits).into()
},
(A64Opnd::Reg(rn), A64Opnd::UImm(imm12)) => {
assert!(uimm_fits_bits(imm12, 12), "The immediate operand must be 12 bits or less.");

DataImm::cmp(rn.reg_no, imm12 as u16, rn.num_bits).into()
DataImm::cmp(rn.reg_no, imm12.try_into().unwrap(), rn.num_bits).into()
},
_ => panic!("Invalid operand combination to cmp instruction."),
};
Expand Down Expand Up @@ -477,12 +471,12 @@ pub fn mov(cb: &mut CodeBlock, rd: A64Opnd, rm: A64Opnd) {
(A64Opnd::Reg(A64Reg { reg_no: 31, num_bits: 64 }), A64Opnd::Reg(rm)) => {
assert!(rm.num_bits == 64, "Expected rm to be 64 bits");

DataImm::add(31, rm.reg_no, 0, 64).into()
DataImm::add(31, rm.reg_no, 0.try_into().unwrap(), 64).into()
},
(A64Opnd::Reg(rd), A64Opnd::Reg(A64Reg { reg_no: 31, num_bits: 64 })) => {
assert!(rd.num_bits == 64, "Expected rd to be 64 bits");

DataImm::add(rd.reg_no, 31, 0, 64).into()
DataImm::add(rd.reg_no, 31, 0.try_into().unwrap(), 64).into()
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rm)) => {
assert!(rd.num_bits == rm.num_bits, "Expected registers to be the same size");
Expand Down Expand Up @@ -713,18 +707,16 @@ pub fn sub(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, rm: A64Opnd) {
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::UImm(uimm12)) => {
assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size.");
assert!(uimm_fits_bits(uimm12, 12), "The immediate operand must be 12 bits or less.");

DataImm::sub(rd.reg_no, rn.reg_no, uimm12 as u16, rd.num_bits).into()
DataImm::sub(rd.reg_no, rn.reg_no, uimm12.try_into().unwrap(), rd.num_bits).into()
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::Imm(imm12)) => {
assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size.");
assert!(imm_fits_bits(imm12, 12), "The immediate operand must be 12 bits or less.");

if imm12 < 0 {
DataImm::add(rd.reg_no, rn.reg_no, -imm12 as u16, rd.num_bits).into()
DataImm::add(rd.reg_no, rn.reg_no, (-imm12 as u64).try_into().unwrap(), rd.num_bits).into()
} else {
DataImm::sub(rd.reg_no, rn.reg_no, imm12 as u16, rd.num_bits).into()
DataImm::sub(rd.reg_no, rn.reg_no, (imm12 as u64).try_into().unwrap(), rd.num_bits).into()
}
},
_ => panic!("Invalid operand combination to sub instruction."),
Expand All @@ -746,18 +738,16 @@ pub fn subs(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, rm: A64Opnd) {
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::UImm(uimm12)) => {
assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size.");
assert!(uimm_fits_bits(uimm12, 12), "The immediate operand must be 12 bits or less.");

DataImm::subs(rd.reg_no, rn.reg_no, uimm12 as u16, rd.num_bits).into()
DataImm::subs(rd.reg_no, rn.reg_no, uimm12.try_into().unwrap(), rd.num_bits).into()
},
(A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::Imm(imm12)) => {
assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size.");
assert!(imm_fits_bits(imm12, 12), "The immediate operand must be 12 bits or less.");

if imm12 < 0 {
DataImm::adds(rd.reg_no, rn.reg_no, -imm12 as u16, rd.num_bits).into()
DataImm::adds(rd.reg_no, rn.reg_no, (-imm12 as u64).try_into().unwrap(), rd.num_bits).into()
} else {
DataImm::subs(rd.reg_no, rn.reg_no, imm12 as u16, rd.num_bits).into()
DataImm::subs(rd.reg_no, rn.reg_no, (imm12 as u64).try_into().unwrap(), rd.num_bits).into()
}
},
_ => panic!("Invalid operand combination to subs instruction."),
Expand Down
Loading

0 comments on commit 4cda346

Please sign in to comment.