ruby-changes:73235
From: Alan <ko1@a...>
Date: Tue, 30 Aug 2022 01:03:28 +0900 (JST)
Subject: [ruby-changes:73235] b2d255ad3c (master): A64: Fix off by one in offset encoding for BL (https://github.com/Shopify/ruby/pull/344)
https://git.ruby-lang.org/ruby.git/commit/?id=b2d255ad3c From b2d255ad3cb34262494df3c55352215dcbd4d881 Mon Sep 17 00:00:00 2001 From: Alan Wu <XrXr@u...> Date: Tue, 26 Jul 2022 13:14:06 -0400 Subject: A64: Fix off by one in offset encoding for BL (https://github.com/Shopify/ruby/pull/344) * A64: Fix off by one in offset encoding for BL It's relative to the address of the instruction not the end of it. * A64: Fix off by one when encoding B It's relative to the start of the instruction not the end. * A64: Add some tests for boundary offsets --- yjit/src/asm/arm64/mod.rs | 32 ++++++++++++++++++++++++++++++-- yjit/src/backend/arm64/mod.rs | 22 ++++++++++------------ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/yjit/src/asm/arm64/mod.rs b/yjit/src/asm/arm64/mod.rs index 0eba37ee15..d114f64a22 100644 --- a/yjit/src/asm/arm64/mod.rs +++ b/yjit/src/asm/arm64/mod.rs @@ -910,12 +910,40 @@ mod tests { https://github.com/ruby/ruby/blob/trunk/yjit/src/asm/arm64/mod.rs#L910 #[test] fn test_b() { - check_bytes("00040014", |cb| b(cb, A64Opnd::new_imm(1024))); + check_bytes("ffffff15", |cb| b(cb, A64Opnd::new_imm((1 << 25) - 1))); + } + + #[test] + #[should_panic] + fn test_b_too_big() { + // There are 26 bits available + check_bytes("", |cb| b(cb, A64Opnd::new_imm(1 << 25))); + } + + #[test] + #[should_panic] + fn test_b_too_small() { + // There are 26 bits available + check_bytes("", |cb| b(cb, A64Opnd::new_imm(-(1 << 25) - 1))); } #[test] fn test_bl() { - check_bytes("00040094", |cb| bl(cb, A64Opnd::new_imm(1024))); + check_bytes("00000096", |cb| bl(cb, A64Opnd::new_imm(-(1 << 25)))); + } + + #[test] + #[should_panic] + fn test_bl_too_big() { + // There are 26 bits available + check_bytes("", |cb| bl(cb, A64Opnd::new_imm(1 << 25))); + } + + #[test] + #[should_panic] + fn test_bl_too_small() { + // There are 26 bits available + check_bytes("", |cb| bl(cb, A64Opnd::new_imm(-(1 << 25) - 1))); } #[test] diff --git a/yjit/src/backend/arm64/mod.rs b/yjit/src/backend/arm64/mod.rs index 12cd267245..018dd79ae4 100644 --- a/yjit/src/backend/arm64/mod.rs +++ b/yjit/src/backend/arm64/mod.rs @@ -683,20 +683,17 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L683 } }, Op::CCall => { - // The offset between the two instructions in bytes. Note - // that when we encode this into a bl instruction, we'll - // divide by 4 because it accepts the number of instructions - // to jump over. - let src_addr = cb.get_write_ptr().into_i64() + 4; + // The offset to the call target in bytes + let src_addr = cb.get_write_ptr().into_i64(); let dst_addr = insn.target.unwrap().unwrap_fun_ptr() as i64; let offset = dst_addr - src_addr; + // The offset in instruction count for BL's immediate + let offset = offset / 4; - // If the offset is short enough, then we'll use the branch - // link instruction. Otherwise, we'll move the destination - // and return address into appropriate registers and use the - // branch register instruction. + // Use BL if the offset is short enough to encode as an immediate. + // Otherwise, use BLR with a register. if b_offset_fits_bits(offset) { - bl(cb, A64Opnd::new_imm(offset / 4)); + bl(cb, A64Opnd::new_imm(offset)); } else { emit_load_value(cb, Self::SCRATCH0, dst_addr as u64); blr(cb, Self::SCRATCH0); @@ -717,7 +714,7 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L714 Op::Jmp => { match insn.target.unwrap() { Target::CodePtr(dst_ptr) => { - let src_addr = cb.get_write_ptr().into_i64() + 4; + let src_addr = cb.get_write_ptr().into_i64(); let dst_addr = dst_ptr.into_i64(); // The offset between the two instructions in bytes. @@ -725,13 +722,14 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L722 // instruction, we'll divide by 4 because it accepts // the number of instructions to jump over. let offset = dst_addr - src_addr; + let offset = offset / 4; // If the offset is short enough, then we'll use the // branch instruction. Otherwise, we'll move the // destination into a register and use the branch // register instruction. if b_offset_fits_bits(offset) { - b(cb, A64Opnd::new_imm(offset / 4)); + b(cb, A64Opnd::new_imm(offset)); } else { emit_load_value(cb, Self::SCRATCH0, dst_addr as u64); br(cb, Self::SCRATCH0); -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/