ruby-changes:73501
From: Kevin <ko1@a...>
Date: Sat, 10 Sep 2022 00:38:10 +0900 (JST)
Subject: [ruby-changes:73501] 848037cadd (master): Better offsets (#6315)
https://git.ruby-lang.org/ruby.git/commit/?id=848037cadd From 848037cadd63091d5c39d06cd5d49aeee2258b4e Mon Sep 17 00:00:00 2001 From: Kevin Newton <kddnewton@g...> Date: Fri, 9 Sep 2022 11:37:41 -0400 Subject: Better offsets (#6315) * Introduce InstructionOffset for AArch64 There are a lot of instructions on AArch64 where we take an offset from PC in terms of the number of instructions. This is for loading a value relative to the PC or for jumping. We were usually accepting an A64Opnd or an i32. It can get confusing and inconsistent though because sometimes you would divide by 4 to get the number of instructions or multiply by 4 to get the number of bytes. This commit adds a struct that wraps an i32 in order to keep all of that logic in one place. It makes it much easier to read and reason about how these offsets are getting used. * Use b instruction when the offset fits on AArch64 --- yjit/src/asm/arm64/arg/inst_offset.rs | 47 ++++++++++++++++++++++++ yjit/src/asm/arm64/arg/mod.rs | 2 ++ yjit/src/asm/arm64/inst/branch_cond.rs | 18 +++++----- yjit/src/asm/arm64/inst/call.rs | 28 +++++++-------- yjit/src/asm/arm64/inst/load_literal.rs | 14 ++++---- yjit/src/asm/arm64/mod.rs | 63 ++++++++++++++------------------- yjit/src/backend/arm64/mod.rs | 52 ++++++++++++++------------- 7 files changed, 133 insertions(+), 91 deletions(-) create mode 100644 yjit/src/asm/arm64/arg/inst_offset.rs diff --git a/yjit/src/asm/arm64/arg/inst_offset.rs b/yjit/src/asm/arm64/arg/inst_offset.rs new file mode 100644 index 0000000000..f4a6bc73a0 --- /dev/null +++ b/yjit/src/asm/arm64/arg/inst_offset.rs @@ -0,0 +1,47 @@ https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/arg/inst_offset.rs#L1 +/// There are a lot of instructions in the AArch64 architectrue that take an +/// offset in terms of number of instructions. Usually they are jump +/// instructions or instructions that load a value relative to the current PC. +/// +/// This struct is used to mark those locations instead of a generic operand in +/// order to give better clarity to the developer when reading the AArch64 +/// backend code. It also helps to clarify that everything is in terms of a +/// number of instructions and not a number of bytes (i.e., the offset is the +/// number of bytes divided by 4). +#[derive(Copy, Clone)] +pub struct InstructionOffset(i32); + +impl InstructionOffset { + /// Create a new instruction offset. + pub fn from_insns(insns: i32) -> Self { + InstructionOffset(insns) + } + + /// Create a new instruction offset from a number of bytes. + pub fn from_bytes(bytes: i32) -> Self { + assert_eq!(bytes % 4, 0, "Byte offset must be a multiple of 4"); + InstructionOffset(bytes / 4) + } +} + +impl From<i32> for InstructionOffset { + /// Convert an i64 into an instruction offset. + fn from(value: i32) -> Self { + InstructionOffset(value) + } +} + +impl From<InstructionOffset> for i32 { + /// Convert an instruction offset into a number of instructions as an i32. + fn from(offset: InstructionOffset) -> Self { + offset.0 + } +} + +impl From<InstructionOffset> for i64 { + /// Convert an instruction offset into a number of instructions as an i64. + /// This is useful for when we're checking how many bits this offset fits + /// into. + fn from(offset: InstructionOffset) -> Self { + offset.0.into() + } +} diff --git a/yjit/src/asm/arm64/arg/mod.rs b/yjit/src/asm/arm64/arg/mod.rs index 9bf4a8ea13..7eb37834f9 100644 --- a/yjit/src/asm/arm64/arg/mod.rs +++ b/yjit/src/asm/arm64/arg/mod.rs @@ -3,6 +3,7 @@ https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/arg/mod.rs#L3 mod bitmask_imm; mod condition; +mod inst_offset; mod sf; mod shifted_imm; mod sys_reg; @@ -10,6 +11,7 @@ mod truncate; https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/arg/mod.rs#L11 pub use bitmask_imm::BitmaskImmediate; pub use condition::Condition; +pub use inst_offset::InstructionOffset; pub use sf::Sf; pub use shifted_imm::ShiftedImmediate; pub use sys_reg::SystemRegister; diff --git a/yjit/src/asm/arm64/inst/branch_cond.rs b/yjit/src/asm/arm64/inst/branch_cond.rs index c489bacef0..4338cf0f4f 100644 --- a/yjit/src/asm/arm64/inst/branch_cond.rs +++ b/yjit/src/asm/arm64/inst/branch_cond.rs @@ -1,4 +1,4 @@ https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/branch_cond.rs#L1 -use super::super::arg::{Condition, truncate_imm}; +use super::super::arg::{Condition, InstructionOffset, truncate_imm}; /// The struct that represents an A64 conditional branch instruction that can be /// encoded. @@ -14,14 +14,14 @@ pub struct BranchCond { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/branch_cond.rs#L14 cond: u8, /// The instruction offset from this instruction to branch to. - imm19: i32 + offset: InstructionOffset } impl BranchCond { /// B.cond /// https://developer.arm.com/documentation/ddi0596/2020-12/Base-Instructions/B-cond--Branch-conditionally- - pub fn bcond(cond: u8, imm19: i32) -> Self { - Self { cond, imm19 } + pub fn bcond(cond: u8, offset: InstructionOffset) -> Self { + Self { cond, offset } } } @@ -34,7 +34,7 @@ impl From<BranchCond> for u32 { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/branch_cond.rs#L34 0 | (1 << 30) | (FAMILY << 26) - | (truncate_imm::<_, 19>(inst.imm19) << 5) + | (truncate_imm::<_, 19>(inst.offset) << 5) | (inst.cond as u32) } } @@ -53,25 +53,25 @@ mod tests { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/branch_cond.rs#L53 #[test] fn test_b_eq() { - let result: u32 = BranchCond::bcond(Condition::EQ, 32).into(); + let result: u32 = BranchCond::bcond(Condition::EQ, 32.into()).into(); assert_eq!(0x54000400, result); } #[test] fn test_b_vs() { - let result: u32 = BranchCond::bcond(Condition::VS, 32).into(); + let result: u32 = BranchCond::bcond(Condition::VS, 32.into()).into(); assert_eq!(0x54000406, result); } #[test] fn test_b_eq_max() { - let result: u32 = BranchCond::bcond(Condition::EQ, (1 << 18) - 1).into(); + let result: u32 = BranchCond::bcond(Condition::EQ, ((1 << 18) - 1).into()).into(); assert_eq!(0x547fffe0, result); } #[test] fn test_b_eq_min() { - let result: u32 = BranchCond::bcond(Condition::EQ, -(1 << 18)).into(); + let result: u32 = BranchCond::bcond(Condition::EQ, (-(1 << 18)).into()).into(); assert_eq!(0x54800000, result); } } diff --git a/yjit/src/asm/arm64/inst/call.rs b/yjit/src/asm/arm64/inst/call.rs index 32d924f799..74debac7f7 100644 --- a/yjit/src/asm/arm64/inst/call.rs +++ b/yjit/src/asm/arm64/inst/call.rs @@ -1,4 +1,4 @@ https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/call.rs#L1 -use super::super::arg::truncate_imm; +use super::super::arg::{InstructionOffset, truncate_imm}; /// The operation to perform for this instruction. enum Op { @@ -20,8 +20,8 @@ enum Op { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/call.rs#L20 /// +-------------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+ /// pub struct Call { - /// The PC-relative offset to jump to (which will be multiplied by 4). - imm26: i32, + /// The PC-relative offset to jump to in terms of number of instructions. + offset: InstructionOffset, /// The operation to perform for this instruction. op: Op @@ -30,14 +30,14 @@ pub struct Call { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/call.rs#L30 impl Call { /// B /// https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/B--Branch- - pub fn b(imm26: i32) -> Self { - Self { imm26, op: Op::Branch } + pub fn b(offset: InstructionOffset) -> Self { + Self { offset, op: Op::Branch } } /// BL /// https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/BL--Branch-with-Link-?lang=en - pub fn bl(imm26: i32) -> Self { - Self { imm26, op: Op::BranchWithLink } + pub fn bl(offset: InstructionOffset) -> Self { + Self { offset, op: Op::BranchWithLink } } } @@ -50,7 +50,7 @@ impl From<Call> for u32 { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/call.rs#L50 0 | ((inst.op as u32) << 31) | (FAMILY << 26) - | truncate_imm::<_, 26>(inst.imm26) + | truncate_imm::<_, 26>(inst.offset) } } @@ -68,37 +68,37 @@ mod tests { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/inst/call.rs#L68 #[test] fn test_bl() { - let result: u32 = Call::bl(0).into(); + let result: u32 = Call::bl(0.into()).into(); assert_eq!(0x94000000, result); } #[test] fn test_bl_positive() { - let result: u32 = Call::bl(256).into(); + let result: u32 = Call::bl(256.into()).into(); assert_eq!(0x94000100, result); } #[test] fn test_bl_negative() { - let result: u32 = Call::bl(-256).into(); + let result: u32 = Call::bl((-256).into()).into(); assert_eq!(0x97ffff00, result); } #[test] fn test_b() { - let result: u32 = Call::b(0).into(); + let result: u32 = Call::b(0.into()).into(); assert_eq!(0x14000000, result); } #[test] fn test_b_positive() { - let result: u32 = Call::b((1 << 25) - 1).into(); + let result: u32 = Call::b(((1 << 25) - 1).into()).into(); assert_eq!(0x15ffffff, result); } #[test] fn test_b_negative() { - let result: u32 = Call::b(-(1 << 25)).into(); + let result: u32 = Call::b((-(1 << 25)).into()).into(); assert_eq!(0x16000000, result); } } diff --git a/yjit/src/asm/arm6 (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/