[前][次][番号順一覧][スレッド一覧]

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/

[前][次][番号順一覧][スレッド一覧]