From 58c1ebb634c8469026e1bdd0f7e65bfe72b9d9fc Mon Sep 17 00:00:00 2001
From: Aaron Patterson <tenderlove@ruby-lang.org>
Date: Wed, 23 Aug 2023 07:34:03 -0700
Subject: [PATCH] Fix guard-heap upgrades (#8264)

* Fix guard-heap upgrades

`getinstancevariable` was generating more heap guards than I thought.
It turns out that the upgrade code has a bug in it.

Given the following Ruby code:

```ruby
class Foo
  def initialize
    @a = 1
    @b = 1
  end

  def foo
    [@a, @b]
  end
end

foo = Foo.new
10.times { foo.foo }

puts RubyVM::YJIT.disasm Foo.instance_method(:foo)
```

Before this commit, the machine code was like this:

```
== BLOCK 1/4, ISEQ RANGE [0,3), 36 bytes ======================
  # Insn: 0000 getinstancevariable (stack_size: 0)
  0x5562fb831023: mov rax, qword ptr [r13 + 0x18]
  # guard object is heap
  0x5562fb831027: test al, 7
  0x5562fb83102a: jne 0x5562fb833080
  0x5562fb831030: test rax, rax
  0x5562fb831033: je 0x5562fb833080
  # guard shape
  0x5562fb831039: cmp dword ptr [rax + 4], 0x18
  0x5562fb83103d: jne 0x5562fb833062
  # reg_temps: 00000000 -> 00000001
  0x5562fb831043: mov rsi, qword ptr [rax + 0x10]

== BLOCK 2/4, ISEQ RANGE [3,6), 0 bytes =======================
== BLOCK 3/4, ISEQ RANGE [3,6), 36 bytes ======================
  # regenerate_branch
  # Insn: 0003 getinstancevariable (stack_size: 1)
  # regenerate_branch
  0x5562fb831047: mov rax, qword ptr [r13 + 0x18]
  # guard object is heap
  0x5562fb83104b: test al, 7
  0x5562fb83104e: jne 0x5562fb8330db
  0x5562fb831054: test rax, rax
  0x5562fb831057: je 0x5562fb8330db
  # guard shape
  0x5562fb83105d: cmp dword ptr [rax + 4], 0x18
  0x5562fb831061: jne 0x5562fb8330ba
  # reg_temps: 00000001 -> 00000011
  0x5562fb831067: mov rdi, qword ptr [rax + 0x18]
```

After this commit, the machine code has fewer guards for `self`:

```
== BLOCK 1/4, ISEQ RANGE [0,3), 36 bytes ======================
  # Insn: 0000 getinstancevariable (stack_size: 0)
  0x55cb5db5f023: mov rax, qword ptr [r13 + 0x18]
  # guard object is heap
  0x55cb5db5f027: test al, 7
  0x55cb5db5f02a: jne 0x55cb5db61080
  0x55cb5db5f030: test rax, rax
  0x55cb5db5f033: je 0x55cb5db61080
  # guard shape
  0x55cb5db5f039: cmp dword ptr [rax + 4], 0x18
  0x55cb5db5f03d: jne 0x55cb5db61062
  # reg_temps: 00000000 -> 00000001
  0x55cb5db5f043: mov rsi, qword ptr [rax + 0x10]

== BLOCK 2/4, ISEQ RANGE [3,6), 0 bytes =======================
== BLOCK 3/4, ISEQ RANGE [3,6), 18 bytes ======================
  # regenerate_branch
  # Insn: 0003 getinstancevariable (stack_size: 1)
  # regenerate_branch
  0x55cb5db5f047: mov rax, qword ptr [r13 + 0x18]
  # guard shape
  0x55cb5db5f04b: cmp dword ptr [rax + 4], 0x18
  0x55cb5db5f04f: jne 0x55cb5db610ba
  # reg_temps: 00000001 -> 00000011
  0x55cb5db5f055: mov rdi, qword ptr [rax + 0x18]
```

Co-Authored-By: Takashi Kokubun <takashikkbn@gmail.com>

* Fix array/string guards as well

---------

Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
---
 yjit/src/codegen.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs
index 8e83f128af..77b8747e3c 100644
--- a/yjit/src/codegen.rs
+++ b/yjit/src/codegen.rs
@@ -1417,7 +1417,7 @@ fn guard_object_is_heap(
     asm.cmp(object, Qfalse.into());
     asm.je(Target::side_exit(counter));
 
-    if object_type.diff(Type::UnknownHeap) != TypeDiff::Incompatible {
+    if Type::UnknownHeap.diff(object_type) != TypeDiff::Incompatible {
         asm.ctx.upgrade_opnd_type(object_opnd, Type::UnknownHeap);
     }
 }
@@ -1449,7 +1449,7 @@ fn guard_object_is_array(
     asm.cmp(flags_opnd, (RUBY_T_ARRAY as u64).into());
     asm.jne(Target::side_exit(counter));
 
-    if object_type.diff(Type::TArray) != TypeDiff::Incompatible {
+    if Type::UnknownHeap.diff(object_type) != TypeDiff::Incompatible {
         asm.ctx.upgrade_opnd_type(object_opnd, Type::TArray);
     }
 }
@@ -1481,7 +1481,7 @@ fn guard_object_is_string(
     asm.cmp(flags_reg, Opnd::UImm(RUBY_T_STRING as u64));
     asm.jne(Target::side_exit(counter));
 
-    if object_type.diff(Type::TString) != TypeDiff::Incompatible {
+    if Type::UnknownHeap.diff(object_type) != TypeDiff::Incompatible {
         asm.ctx.upgrade_opnd_type(object_opnd, Type::TString);
     }
 }