Skip to content

Commit

Permalink
Differentiate T_ARRAY and array subclasses (ruby#7091)
Browse files Browse the repository at this point in the history
* Differentiate T_ARRAY and array subclasses

This commit teaches the YJIT context the difference between Arrays
(objects with type T_ARRAY and class rb_cArray) vs Array subclasses
(objects with type T_ARRAY but _not_ class rb_cArray).  It uses this
information to reduce the number of guards emitted when using
`jit_guard_known_klass` with rb_cArray, notably opt_aref

* Update yjit/src/core.rs

Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
  • Loading branch information
tenderlove and maximecb authored Jan 10, 2023
1 parent aeddc19 commit 5bf7218
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
10 changes: 6 additions & 4 deletions yjit/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,7 @@ fn gen_newarray(
);

ctx.stack_pop(n.as_usize());
let stack_ret = ctx.stack_push(Type::Array);
let stack_ret = ctx.stack_push(Type::CArray);
asm.mov(stack_ret, new_ary);

KeepCompiling
Expand All @@ -1185,7 +1185,7 @@ fn gen_duparray(
vec![ary.into()],
);

let stack_ret = ctx.stack_push(Type::Array);
let stack_ret = ctx.stack_push(Type::CArray);
asm.mov(stack_ret, new_ary);

KeepCompiling
Expand Down Expand Up @@ -1231,7 +1231,7 @@ fn gen_splatarray(
// Call rb_vm_splat_array(flag, ary)
let ary = asm.ccall(rb_vm_splat_array as *const u8, vec![flag.into(), ary_opnd]);

let stack_ret = ctx.stack_push(Type::Array);
let stack_ret = ctx.stack_push(Type::TArray);
asm.mov(stack_ret, ary);

KeepCompiling
Expand All @@ -1255,7 +1255,7 @@ fn gen_concatarray(
// Call rb_vm_concat_array(ary1, ary2st)
let ary = asm.ccall(rb_vm_concat_array as *const u8, vec![ary1_opnd, ary2st_opnd]);

let stack_ret = ctx.stack_push(Type::Array);
let stack_ret = ctx.stack_push(Type::TArray);
asm.mov(stack_ret, ary);

KeepCompiling
Expand Down Expand Up @@ -3886,6 +3886,8 @@ fn jit_guard_known_klass(

if known_klass == unsafe { rb_cString } {
ctx.upgrade_opnd_type(insn_opnd, Type::CString);
} else if known_klass == unsafe { rb_cArray } {
ctx.upgrade_opnd_type(insn_opnd, Type::CArray);
}
}
}
Expand Down
20 changes: 16 additions & 4 deletions yjit/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ pub enum Type {
False,
Fixnum,
Flonum,
Array,
Hash,
ImmSymbol,

Expand All @@ -44,6 +43,8 @@ pub enum Type {

TString, // An object with the T_STRING flag set, possibly an rb_cString
CString, // An un-subclassed string of type rb_cString (can have instance vars in some cases)
TArray, // An object with the T_ARRAY flag set, possibly an rb_cArray
CArray, // An un-subclassed string of type rb_cArray (can have instance vars in some cases)

BlockParamProxy, // A special sentinel value indicating the block parameter should be read from
// the current surrounding cfp
Expand Down Expand Up @@ -82,14 +83,18 @@ impl Type {
if val.class_of() == unsafe { rb_cString } {
return Type::CString;
}
#[cfg(not(test))]
if val.class_of() == unsafe { rb_cArray } {
return Type::CArray;
}
// We likewise can't reference rb_block_param_proxy, but it's again an optimisation;
// we can just treat it as a normal Object.
#[cfg(not(test))]
if val == unsafe { rb_block_param_proxy } {
return Type::BlockParamProxy;
}
match val.builtin_type() {
RUBY_T_ARRAY => Type::Array,
RUBY_T_ARRAY => Type::TArray,
RUBY_T_HASH => Type::Hash,
RUBY_T_STRING => Type::TString,
_ => Type::UnknownHeap,
Expand Down Expand Up @@ -130,7 +135,8 @@ impl Type {
pub fn is_heap(&self) -> bool {
match self {
Type::UnknownHeap => true,
Type::Array => true,
Type::TArray => true,
Type::CArray => true,
Type::Hash => true,
Type::HeapSymbol => true,
Type::TString => true,
Expand All @@ -147,7 +153,7 @@ impl Type {
Type::False => Some(RUBY_T_FALSE),
Type::Fixnum => Some(RUBY_T_FIXNUM),
Type::Flonum => Some(RUBY_T_FLOAT),
Type::Array => Some(RUBY_T_ARRAY),
Type::TArray | Type::CArray => Some(RUBY_T_ARRAY),
Type::Hash => Some(RUBY_T_HASH),
Type::ImmSymbol | Type::HeapSymbol => Some(RUBY_T_SYMBOL),
Type::TString | Type::CString => Some(RUBY_T_STRING),
Expand All @@ -167,6 +173,7 @@ impl Type {
Type::Flonum => Some(rb_cFloat),
Type::ImmSymbol | Type::HeapSymbol => Some(rb_cSymbol),
Type::CString => Some(rb_cString),
Type::CArray => Some(rb_cArray),
_ => None,
}
}
Expand Down Expand Up @@ -224,6 +231,11 @@ impl Type {
return 1;
}

// A CArray is also a TArray.
if self == Type::CArray && dst == Type::TArray {
return 1;
}

// Specific heap type into unknown heap type is imperfect but valid
if self.is_heap() && dst == Type::UnknownHeap {
return 1;
Expand Down

0 comments on commit 5bf7218

Please sign in to comment.