Skip to content

Commit d265349

Browse files
committed
ZJIT: Guard T_* in addition to shape in polymorphic getivar
This is a8f3c34 ("ZJIT: Add missing guard on ivar access on T_{DATA,CLASS,MODULE}") but for the polymorphic implementation in HIR build.
1 parent a2a69b4 commit d265349

8 files changed

Lines changed: 209 additions & 80 deletions

File tree

jit.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ enum jit_bindgen_constants {
4040
// Field offsets for the RString struct
4141
RUBY_OFFSET_RSTRING_LEN = offsetof(struct RString, len),
4242

43+
// Shape constant related to RBasic::flags. (See RBASIC_SET_SHAPE_ID())
44+
RB_SHAPE_FLAG_SHIFT = SHAPE_FLAG_SHIFT,
45+
4346
// Field offsets for rb_execution_context_t
4447
RUBY_OFFSET_EC_CFP = offsetof(rb_execution_context_t, cfp),
4548
RUBY_OFFSET_EC_INTERRUPT_FLAG = offsetof(rb_execution_context_t, interrupt_flag),

yjit/src/cruby_bindings.inc.rs

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

zjit/src/codegen.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
617617
&Insn::Const { val: Const::CInt64(val) } => gen_const_long(val),
618618
&Insn::Const { val: Const::CUInt16(val) } => gen_const_uint16(val),
619619
&Insn::Const { val: Const::CUInt32(val) } => gen_const_uint32(val),
620+
&Insn::Const { val: Const::CUInt64(val) } => Opnd::UImm(val),
620621
&Insn::Const { val: Const::CShape(val) } => {
621622
assert_eq!(SHAPE_ID_NUM_BITS, 32);
622623
gen_const_uint32(val.0)

zjit/src/codegen_tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3714,6 +3714,7 @@ fn test_getivar_t_class_then_string() {
37143714
assert_snapshot!(assert_compiles("[STR.test, STR.test]"), @"[1000, 1000]");
37153715
}
37163716

3717+
37173718
#[test]
37183719
fn test_attr_accessor_setivar() {
37193720
eval("

zjit/src/cruby_bindings.inc.rs

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

zjit/src/hir.rs

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6342,8 +6342,16 @@ impl Function {
63426342
Insn::IntAnd { left, right }
63436343
| Insn::IntOr { left, right } => {
63446344
// TODO: Expand this to other matching C integer sizes when we need them.
6345-
self.assert_subtype(insn_id, left, types::CInt64)?;
6346-
self.assert_subtype(insn_id, right, types::CInt64)
6345+
let left_type = self.type_of(left);
6346+
if left_type.is_subtype(types::CInt64) {
6347+
self.assert_subtype(insn_id, right, types::CInt64)
6348+
} else if left_type.is_subtype(types::CUInt64) {
6349+
self.assert_subtype(insn_id, right, types::CUInt64)
6350+
} else {
6351+
let all_ints = types::CInt64.union(types::CUInt64);
6352+
self.assert_subtype(insn_id, left, all_ints)?;
6353+
self.assert_subtype(insn_id, right, all_ints)
6354+
}
63476355
}
63486356
Insn::BoxBool { val }
63496357
| Insn::IfTrue { val, .. }
@@ -8185,31 +8193,37 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
81858193
}
81868194
if let Some(summary) = fun.polymorphic_summary(&profiles, self_param, exit_state.insn_idx) {
81878195
self_param = fun.push_insn(block, Insn::GuardType { val: self_param, guard_type: types::HeapBasicObject, state: exit_id });
8188-
let shape = fun.load_shape(block, self_param);
8196+
let rbasic_flags = fun.load_rbasic_flags(block, self_param);
81898197
let join_block = insn_idx_to_block.get(&insn_idx).copied().unwrap_or_else(|| fun.new_block(insn_idx));
81908198
let join_param = fun.push_insn(join_block, Insn::Param);
81918199
// Dedup by expected shape so objects with different classes but the same shape can share code
81928200
// TODO(max): De-duplicate further by checking ivar offsets to allow
81938201
// different shapes with the same ivar layout to share code
8194-
let mut seen_shapes = Vec::with_capacity(summary.buckets().len());
8202+
let mut seen_shape_and_flags = Vec::with_capacity(summary.buckets().len());
81958203
for &profiled_type in summary.buckets() {
81968204
// End of the buckets
81978205
if profiled_type.is_empty() { break; }
81988206
// Instance variable lookups on immediate values are always nil; don't bother
81998207
if profiled_type.flags().is_immediate() { continue; }
82008208
let expected_shape = profiled_type.shape();
8209+
let (expected_rbasic_flags, rbasic_flags_mask) = profiled_type.rbasic_flags_and_mask();
82018210
assert!(expected_shape.is_valid());
82028211
// Too-complex shapes use hash tables for ivars;
82038212
// rb_shape_get_iv_index doesn't work for them.
82048213
// Let the fallthrough GetIvar handle these.
82058214
if expected_shape.is_too_complex() { continue; }
8206-
if seen_shapes.contains(&expected_shape) { continue; }
8207-
seen_shapes.push(expected_shape);
8208-
let expected_shape_const = fun.push_insn(block, Insn::Const { val: Const::CShape(expected_shape) });
8209-
let has_shape = fun.push_insn(block, Insn::IsBitEqual { left: shape, right: expected_shape_const });
8215+
if seen_shape_and_flags.contains(&expected_rbasic_flags) { continue; }
8216+
seen_shape_and_flags.push(expected_rbasic_flags);
8217+
let rbasic_flags_mask = fun.push_insn(block, Insn::Const { val: Const::CUInt64(rbasic_flags_mask) });
8218+
// The expected shape can change over run, so we put it
8219+
// as a pointer to keep it stable in snapshot tests.
8220+
let expected_rbasic_flags = fun.push_insn(block, Insn::Const { val: Const::CPtr(ptr::without_provenance(expected_rbasic_flags.to_usize())) });
8221+
let expected_rbasic_flags = fun.push_insn(block, Insn::RefineType { val: expected_rbasic_flags, new_type: types::CUInt64 });
8222+
let masked = fun.push_insn(block, Insn::IntAnd { left: rbasic_flags, right: rbasic_flags_mask});
8223+
let has_shape_and_type = fun.push_insn(block, Insn::IsBitEqual { left: masked, right: expected_rbasic_flags });
82108224
let iftrue_block = fun.new_block(insn_idx);
82118225
let target = BranchEdge { target: iftrue_block, args: vec![] };
8212-
fun.push_insn(block, Insn::IfTrue { val: has_shape, target });
8226+
fun.push_insn(block, Insn::IfTrue { val: has_shape_and_type, target });
82138227
let result = fun.load_ivar(iftrue_block, self_param, profiled_type, id, exit_id);
82148228
fun.push_insn(iftrue_block, Insn::Jump(BranchEdge { target: join_block, args: vec![result] }));
82158229
}

zjit/src/hir/opt_tests.rs

Lines changed: 160 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -7355,21 +7355,27 @@ mod hir_opt_tests {
73557355
bb3(v6:BasicObject):
73567356
PatchPoint SingleRactorMode
73577357
v11:HeapBasicObject = GuardType v6, HeapBasicObject
7358-
v12:CShape = LoadField v11, :_shape_id@0x1000
7359-
v14:CShape[0x1001] = Const CShape(0x1001)
7360-
v15:CBool = IsBitEqual v12, v14
7361-
IfTrue v15, bb5()
7362-
v19:CShape[0x1002] = Const CShape(0x1002)
7363-
v20:CBool = IsBitEqual v12, v19
7364-
IfTrue v20, bb6()
7365-
v24:BasicObject = GetIvar v11, :@foo
7366-
Jump bb4(v24)
7358+
v12:CUInt64 = LoadField v11, :_rbasic_flags@0x1000
7359+
v14:CUInt64[0xffffffff0000001f] = Const CUInt64(0xffffffff0000001f)
7360+
v15:CPtr[CPtr(0x1001)] = Const CPtr(0x1001)
7361+
v16 = RefineType v15, CUInt64
7362+
v17:CInt64 = IntAnd v12, v14
7363+
v18:CBool = IsBitEqual v17, v16
7364+
IfTrue v18, bb5()
7365+
v22:CUInt64[0xffffffff0000001f] = Const CUInt64(0xffffffff0000001f)
7366+
v23:CPtr[CPtr(0x1002)] = Const CPtr(0x1002)
7367+
v24 = RefineType v23, CUInt64
7368+
v25:CInt64 = IntAnd v12, v22
7369+
v26:CBool = IsBitEqual v25, v24
7370+
IfTrue v26, bb6()
7371+
v30:BasicObject = GetIvar v11, :@foo
7372+
Jump bb4(v30)
73677373
bb5():
7368-
v17:BasicObject = LoadField v11, :@foo@0x1003
7369-
Jump bb4(v17)
7374+
v20:BasicObject = LoadField v11, :@foo@0x1003
7375+
Jump bb4(v20)
73707376
bb6():
7371-
v22:BasicObject = LoadField v11, :@foo@0x1004
7372-
Jump bb4(v22)
7377+
v28:BasicObject = LoadField v11, :@foo@0x1004
7378+
Jump bb4(v28)
73737379
bb4(v13:BasicObject):
73747380
CheckInterrupts
73757381
Return v13
@@ -7712,29 +7718,35 @@ mod hir_opt_tests {
77127718
bb3(v6:BasicObject):
77137719
PatchPoint SingleRactorMode
77147720
v11:HeapBasicObject = GuardType v6, HeapBasicObject
7715-
v12:CShape = LoadField v11, :_shape_id@0x1000
7716-
v14:CShape[0x1001] = Const CShape(0x1001)
7717-
v15:CBool = IsBitEqual v12, v14
7718-
IfTrue v15, bb5()
7719-
v20:CShape[0x1002] = Const CShape(0x1002)
7720-
v21:CBool = IsBitEqual v12, v20
7721-
IfTrue v21, bb6()
7722-
v25:BasicObject = GetIvar v11, :@foo
7723-
Jump bb4(v25)
7721+
v12:CUInt64 = LoadField v11, :_rbasic_flags@0x1000
7722+
v14:CUInt64[18446744069414584351] = Const CUInt64(18446744069414584351)
7723+
v15:CPtr[CPtr(0x1008)] = Const CPtr(0x1010)
7724+
v16 = RefineType v15, CUInt64
7725+
v17:CInt64 = IntAnd v12, v14
7726+
v18:CBool = IsBitEqual v17, v16
7727+
IfTrue v18, bb5()
7728+
v23:CUInt64[18446744069414584351] = Const CUInt64(18446744069414584351)
7729+
v24:CPtr[CPtr(0x1008)] = Const CPtr(0x1010)
7730+
v25 = RefineType v24, CUInt64
7731+
v26:CInt64 = IntAnd v12, v23
7732+
v27:CBool = IsBitEqual v26, v25
7733+
IfTrue v27, bb6()
7734+
v31:BasicObject = GetIvar v11, :@foo
7735+
Jump bb4(v31)
77247736
bb5():
7725-
v17:CPtr = LoadField v11, :_as_heap@0x1003
7726-
v18:BasicObject = LoadField v17, :@foo@0x1004
7727-
Jump bb4(v18)
7737+
v20:CPtr = LoadField v11, :_as_heap@0x1018
7738+
v21:BasicObject = LoadField v20, :@foo@0x1019
7739+
Jump bb4(v21)
77287740
bb6():
7729-
v23:BasicObject = LoadField v11, :@foo@0x1003
7730-
Jump bb4(v23)
7741+
v29:BasicObject = LoadField v11, :@foo@0x1018
7742+
Jump bb4(v29)
77317743
bb4(v13:BasicObject):
7732-
v28:Fixnum[1] = Const Value(1)
7733-
PatchPoint MethodRedefined(Integer@0x1008, +@0x1010, cme:0x1018)
7734-
v39:Fixnum = GuardType v13, Fixnum
7735-
v40:Fixnum = FixnumAdd v39, v28
7744+
v34:Fixnum[1] = Const Value(1)
7745+
PatchPoint MethodRedefined(Integer@0x1020, +@0x1028, cme:0x1030)
7746+
v45:Fixnum = GuardType v13, Fixnum
7747+
v46:Fixnum = FixnumAdd v45, v34
77367748
CheckInterrupts
7737-
Return v40
7749+
Return v46
77387750
");
77397751
}
77407752

@@ -7782,31 +7794,37 @@ mod hir_opt_tests {
77827794
bb3(v6:BasicObject):
77837795
PatchPoint SingleRactorMode
77847796
v11:HeapBasicObject = GuardType v6, HeapBasicObject
7785-
v12:CShape = LoadField v11, :_shape_id@0x1000
7786-
v14:CShape[0x1001] = Const CShape(0x1001)
7787-
v15:CBool = IsBitEqual v12, v14
7788-
IfTrue v15, bb5()
7789-
v19:CShape[0x1002] = Const CShape(0x1002)
7790-
v20:CBool = IsBitEqual v12, v19
7791-
IfTrue v20, bb6()
7792-
v38:CShape = LoadField v11, :_shape_id@0x1000
7793-
v39:CShape[0x1001] = GuardBitEquals v38, CShape(0x1001)
7794-
v40:BasicObject = LoadField v11, :@foo@0x1003
7795-
Jump bb4(v40)
7797+
v12:CUInt64 = LoadField v11, :_rbasic_flags@0x1000
7798+
v14:CUInt64[18446744069414584351] = Const CUInt64(18446744069414584351)
7799+
v15:CPtr[CPtr(0x1008)] = Const CPtr(0x1010)
7800+
v16 = RefineType v15, CUInt64
7801+
v17:CInt64 = IntAnd v12, v14
7802+
v18:CBool = IsBitEqual v17, v16
7803+
IfTrue v18, bb5()
7804+
v22:CUInt64[18446744069414584351] = Const CUInt64(18446744069414584351)
7805+
v23:CPtr[CPtr(0x1008)] = Const CPtr(0x1010)
7806+
v24 = RefineType v23, CUInt64
7807+
v25:CInt64 = IntAnd v12, v22
7808+
v26:CBool = IsBitEqual v25, v24
7809+
IfTrue v26, bb6()
7810+
v44:CShape = LoadField v11, :_shape_id@0x1018
7811+
v45:CShape[0x1019] = GuardBitEquals v44, CShape(0x1019)
7812+
v46:BasicObject = LoadField v11, :@foo@0x101a
7813+
Jump bb4(v46)
77967814
bb5():
7797-
v17:BasicObject = LoadField v11, :@foo@0x1003
7798-
Jump bb4(v17)
7815+
v20:BasicObject = LoadField v11, :@foo@0x101a
7816+
Jump bb4(v20)
77997817
bb6():
7800-
v22:CPtr = LoadField v11, :_as_heap@0x1003
7801-
v23:BasicObject = LoadField v22, :@foo@0x1004
7802-
Jump bb4(v23)
7818+
v28:CPtr = LoadField v11, :_as_heap@0x101a
7819+
v29:BasicObject = LoadField v28, :@foo@0x101b
7820+
Jump bb4(v29)
78037821
bb4(v13:BasicObject):
7804-
v28:Fixnum[1] = Const Value(1)
7805-
PatchPoint MethodRedefined(Integer@0x1008, +@0x1010, cme:0x1018)
7806-
v43:Fixnum = GuardType v13, Fixnum
7807-
v44:Fixnum = FixnumAdd v43, v28
7822+
v34:Fixnum[1] = Const Value(1)
7823+
PatchPoint MethodRedefined(Integer@0x1020, +@0x1028, cme:0x1030)
7824+
v49:Fixnum = GuardType v13, Fixnum
7825+
v50:Fixnum = FixnumAdd v49, v34
78087826
CheckInterrupts
7809-
Return v44
7827+
Return v50
78107828
");
78117829
}
78127830

@@ -7847,28 +7865,99 @@ mod hir_opt_tests {
78477865
bb3(v6:BasicObject):
78487866
PatchPoint SingleRactorMode
78497867
v11:HeapBasicObject = GuardType v6, HeapBasicObject
7850-
v12:CShape = LoadField v11, :_shape_id@0x1000
7851-
v14:CShape[0x1001] = Const CShape(0x1001)
7852-
v15:CBool = IsBitEqual v12, v14
7853-
IfTrue v15, bb5()
7854-
v19:CShape[0x1002] = Const CShape(0x1002)
7855-
v20:CBool = IsBitEqual v12, v19
7856-
IfTrue v20, bb6()
7857-
v24:BasicObject = GetIvar v11, :@foo
7858-
Jump bb4(v24)
7868+
v12:CUInt64 = LoadField v11, :_rbasic_flags@0x1000
7869+
v14:CUInt64[18446744069414584351] = Const CUInt64(18446744069414584351)
7870+
v15:CPtr[CPtr(0x1008)] = Const CPtr(0x1010)
7871+
v16 = RefineType v15, CUInt64
7872+
v17:CInt64 = IntAnd v12, v14
7873+
v18:CBool = IsBitEqual v17, v16
7874+
IfTrue v18, bb5()
7875+
v22:CUInt64[18446744069414584351] = Const CUInt64(18446744069414584351)
7876+
v23:CPtr[CPtr(0x1008)] = Const CPtr(0x1010)
7877+
v24 = RefineType v23, CUInt64
7878+
v25:CInt64 = IntAnd v12, v22
7879+
v26:CBool = IsBitEqual v25, v24
7880+
IfTrue v26, bb6()
7881+
v30:BasicObject = GetIvar v11, :@foo
7882+
Jump bb4(v30)
78597883
bb5():
7860-
v17:BasicObject = LoadField v11, :@foo@0x1003
7861-
Jump bb4(v17)
7884+
v20:BasicObject = LoadField v11, :@foo@0x1018
7885+
Jump bb4(v20)
78627886
bb6():
7863-
v22:BasicObject = LoadField v11, :@foo@0x1003
7864-
Jump bb4(v22)
7887+
v28:BasicObject = LoadField v11, :@foo@0x1018
7888+
Jump bb4(v28)
78657889
bb4(v13:BasicObject):
7866-
v27:Fixnum[1] = Const Value(1)
7867-
PatchPoint MethodRedefined(Integer@0x1008, +@0x1010, cme:0x1018)
7868-
v38:Fixnum = GuardType v13, Fixnum
7869-
v39:Fixnum = FixnumAdd v38, v27
7890+
v33:Fixnum[1] = Const Value(1)
7891+
PatchPoint MethodRedefined(Integer@0x1020, +@0x1028, cme:0x1030)
7892+
v44:Fixnum = GuardType v13, Fixnum
7893+
v45:Fixnum = FixnumAdd v44, v33
78707894
CheckInterrupts
7871-
Return v39
7895+
Return v45
7896+
");
7897+
}
7898+
7899+
#[test]
7900+
fn test_getivar_polymorphic_t_class_and_typed_data() {
7901+
set_call_threshold(3);
7902+
eval(r#"
7903+
module Reader
7904+
def test = @a
7905+
end
7906+
7907+
class A
7908+
extend Reader
7909+
@a = 0
7910+
end
7911+
7912+
ARGF.instance_eval do
7913+
extend Reader
7914+
@a = :a
7915+
end
7916+
7917+
A.test
7918+
ARGF.test
7919+
"#);
7920+
assert_snapshot!(assert_compiles("[A.test, ARGF.test]"), @"[0, :a]");
7921+
assert_snapshot!(hir_string_proc("Reader.instance_method(:test)"), @"
7922+
fn test@<compiled>:3:
7923+
bb1():
7924+
EntryPoint interpreter
7925+
v1:BasicObject = LoadSelf
7926+
Jump bb3(v1)
7927+
bb2():
7928+
EntryPoint JIT(0)
7929+
v4:BasicObject = LoadArg :self@0
7930+
Jump bb3(v4)
7931+
bb3(v6:BasicObject):
7932+
PatchPoint SingleRactorMode
7933+
v11:HeapBasicObject = GuardType v6, HeapBasicObject
7934+
v12:CUInt64 = LoadField v11, :_rbasic_flags@0x1000
7935+
v14:CUInt64[0xffffffff0000005f] = Const CUInt64(0xffffffff0000005f)
7936+
v15:CPtr[CPtr(0x1001)] = Const CPtr(0x1001)
7937+
v16 = RefineType v15, CUInt64
7938+
v17:CInt64 = IntAnd v12, v14
7939+
v18:CBool = IsBitEqual v17, v16
7940+
IfTrue v18, bb5()
7941+
v23:CUInt64[0xffffffff0000001f] = Const CUInt64(0xffffffff0000001f)
7942+
v24:CPtr[CPtr(0x1002)] = Const CPtr(0x1002)
7943+
v25 = RefineType v24, CUInt64
7944+
v26:CInt64 = IntAnd v12, v23
7945+
v27:CBool = IsBitEqual v26, v25
7946+
IfTrue v27, bb6()
7947+
v33:BasicObject = GetIvar v11, :@a
7948+
Jump bb4(v33)
7949+
bb5():
7950+
v20:RubyValue = LoadField v11, :_fields_obj@0x1003
7951+
v21:BasicObject = LoadField v20, :@a@0x1003
7952+
Jump bb4(v21)
7953+
bb6():
7954+
PatchPoint RootBoxOnly
7955+
v30:RubyValue = LoadField v11, :_fields_obj@0x1004
7956+
v31:BasicObject = LoadField v30, :@a@0x1003
7957+
Jump bb4(v31)
7958+
bb4(v13:BasicObject):
7959+
CheckInterrupts
7960+
Return v13
78727961
");
78737962
}
78747963

0 commit comments

Comments
 (0)