diff options
author | Aaron Patterson <tenderlove@ruby-lang.org> | 2023-08-23 07:34:03 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-08-23 10:34:03 -0400 |
commit | 58c1ebb634c8469026e1bdd0f7e65bfe72b9d9fc (patch) | |
tree | 6ebf0340fd4fa6bb2d86b48bad3f43b54b9f7dc0 /yjit | |
parent | 448ff162c49bb5b27060d4551c5ded6ba768b5dd (diff) |
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>
Notes
Notes:
Merged-By: maximecb <maximecb@ruby-lang.org>
Diffstat (limited to 'yjit')
-rw-r--r-- | yjit/src/codegen.rs | 6 |
1 files 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); } } |