ruby-changes:73219
From: Kevin <ko1@a...>
Date: Tue, 30 Aug 2022 01:03:05 +0900 (JST)
Subject: [ruby-changes:73219] f593b2c6db (master): Fixes for AArch64 (https://github.com/Shopify/ruby/pull/338)
https://git.ruby-lang.org/ruby.git/commit/?id=f593b2c6db From f593b2c6db622de6f973e4e847e959855c341a25 Mon Sep 17 00:00:00 2001 From: Kevin Newton <kddnewton@g...> Date: Fri, 22 Jul 2022 14:01:21 -0400 Subject: Fixes for AArch64 (https://github.com/Shopify/ruby/pull/338) * 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 --- .cirrus.yml | 3 + yjit/src/asm/arm64/arg/mod.rs | 2 + yjit/src/asm/arm64/arg/shifted_imm.rs | 75 +++++++++++++++++++++++ yjit/src/asm/arm64/inst/data_imm.rs | 80 ++++++------------------- yjit/src/asm/arm64/mod.rs | 40 +++++-------- yjit/src/backend/arm64/mod.rs | 109 +++++++++++++++++++++++++++------- 6 files changed, 200 insertions(+), 109 deletions(-) create mode 100644 yjit/src/asm/arm64/arg/shifted_imm.rs diff --git a/.cirrus.yml b/.cirrus.yml index d71ac851e3..74ab6bd979 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -140,6 +140,7 @@ yjit_task: https://github.com/ruby/ruby/blob/trunk/.cirrus.yml#L140 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 \ @@ -147,7 +148,9 @@ yjit_task: https://github.com/ruby/ruby/blob/trunk/.cirrus.yml#L148 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 diff --git a/yjit/src/asm/arm64/arg/mod.rs b/yjit/src/asm/arm64/arg/mod.rs index bb779ab6df..30f3cc3dfe 100644 --- a/yjit/src/asm/arm64/arg/mod.rs +++ b/yjit/src/asm/arm64/arg/mod.rs @@ -4,9 +4,11 @@ https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/arg/mod.rs#L4 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; diff --git a/yjit/src/asm/arm64/arg/shifted_imm.rs b/yjit/src/asm/arm64/arg/shifted_imm.rs new file mode 100644 index 0000000000..5d1eeaf26d --- /dev/null +++ b/yjit/src/asm/arm64/arg/shifted_imm.rs @@ -0,0 +1,75 @@ https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/arg/shifted_imm.rs#L1 +/// 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(()))); + } +} diff --git a/yjit/src/asm/arm64/inst/data_imm.rs b/yjit/src/asm/arm64/inst/data_imm.rs index 19e2bfa199..b474b00a52 100644 --- a/yjit/src/asm/arm64/inst/data_imm.rs +++ b/yjit/src/asm/arm64/inst/data_imm.rs @@ -1,4 +1,4 @@ https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/data_imm.rs#L1 -use super::super::arg::Sf; +use super::super::arg::{Sf, ShiftedImmediate}; /// The operation being performed by this instruction. enum Op { @@ -12,12 +12,6 @@ enum S { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/data_imm.rs#L12 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. /// @@ -35,11 +29,8 @@ pub struct DataImm { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/data_imm.rs#L29 /// 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, @@ -54,64 +45,32 @@ pub struct DataImm { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/data_imm.rs#L45 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() } } } @@ -121,7 +80,7 @@ const FAMILY: u32 = 0b1000; https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/data_imm.rs#L80 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) @@ -129,8 +88,7 @@ impl From<DataImm> for u32 { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/data_imm.rs#L88 | ((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 } @@ -150,35 +108,35 @@ mod tests { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/data_imm.rs#L108 #[test] fn test_add() { - let inst = DataImm::add(0, 1, (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/