10000 Merge pull request #181 from Shopify/getivar_always_mapped · jhawthorn/ruby@e0489a5 · GitHub
[go: up one dir, main page]
Skip to content

Commit e0489a5

Browse files
maximecbmaximecb
authored
Merge pull request ruby#181 from Shopify/getivar_always_mapped
Make sure that there is always an index table entry for getivars
2 parents 1125282 + 40a4021 commit e0489a5

File tree

2 files changed

+65
-63
lines changed

2 files changed

+65
-63
lines changed

‎yjit_codegen.c

Lines changed: 65 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,82 +1440,85 @@ gen_get_ivar(jitstate_t *jit, ctx_t *ctx, const int max_chain_depth, VALUE compt
14401440
struct rb_iv_index_tbl_entry *ent;
14411441
struct st_table *iv_index_tbl = ROBJECT_IV_INDEX_TBL(comptime_receiver);
14421442

1443-
// Lookup index for the ivar the instruction loads
1444-
if (iv_index_tbl && rb_iv_index_tbl_lookup(iv_index_tbl, id, &ent)) {
1445-
uint32_t ivar_index = ent->index;
1443+
// Make sure there is a mapping for this ivar in the index table
1444+
if (!iv_index_tbl || !rb_iv_index_tbl_lookup(iv_index_tbl, id, &ent)) {
1445+
rb_ivar_set(comptime_receiver, id, Qundef);
1446+
iv_index_tbl = ROBJECT_IV_INDEX_TBL(comptime_receiver);
1447+
RUBY_ASSERT(iv_index_tbl);
1448+
// Redo the lookup
1449+
RUBY_ASSERT_ALWAYS(rb_iv_index_tbl_lookup(iv_index_tbl, id, &ent));
1450+
}
14461451

1447-
// Pop receiver if it's on the temp stack
1448-
if (!reg0_opnd.is_self) {
1449-
(void)ctx_stack_pop(ctx, 1);
1450-
}
1452+
uint32_t ivar_index = ent->index;
14511453

1452-
// Compile time self is embedded and the ivar index lands within the object
1453-
if (RB_FL_TEST_RAW(comptime_receiver, ROBJECT_EMBED) && ivar_index < ROBJECT_EMBED_LEN_MAX) {
1454-
// See ROBJECT_IVPTR() from include/ruby/internal/core/robject.h
1454+
// Pop receiver if it's on the temp stack
1455+
if (!reg0_opnd.is_self) {
1456+
(void)ctx_stack_pop(ctx, 1);
1457+
}
14551458

1456-
// Guard that self is embedded
1457-
// TODO: BT and JC is shorter
1458-
ADD_COMMENT(cb, "guard embedded getivar");
1459-
x86opnd_t flags_opnd = member_opnd(REG0, struct RBasic, flags);
1460-
test(cb, flags_opnd, imm_opnd(ROBJECT_EMBED));
1461-
jit_chain_guard(JCC_JZ, jit, &starting_context, max_chain_depth, side_exit);
1459+
// Compile time self is embedded and the ivar index lands within the object
1460+
if (RB_FL_TEST_RAW(comptime_receiver, ROBJECT_EMBED) && ivar_index < ROBJECT_EMBED_LEN_MAX) {
1461+
// See ROBJECT_IVPTR() from include/ruby/internal/core/robject.h
14621462

1463-
// Load the variable
1464-
x86opnd_t ivar_opnd = mem_opnd(64, REG0, offsetof(struct RObject, as.ary) + ivar_index * SIZEOF_VALUE);
1465-
mov(cb, REG1, ivar_opnd);
1463+
// Guard that self is embedded
1464+
// TODO: BT and JC is shorter
1465+
ADD_COMMENT(cb, "guard embedded getivar");
1466+
x86opnd_t flags_opnd = member_opnd(REG0, struct RBasic, flags);
1467+
test(cb, flags_opnd, imm_opnd(ROBJECT_EMBED));
1468+
jit_chain_guard(JCC_JZ, jit, &starting_context, max_chain_depth, side_exit);
14661469

1467-
// Guard that the variable is not Qundef
1468-
cmp(cb, REG1, imm_opnd(Qundef));
1469-
mov(cb, REG0, imm_opnd(Qnil));
1470-
cmove(cb, REG1, REG0);
1470+
// Load the variable
1471+
x86opnd_t ivar_opnd = mem_opnd(64, REG0, offsetof(struct RObject, as.ary) + ivar_index * SIZEOF_VALUE);
1472+
mov(cb, REG1, ivar_opnd);
14711473

1472-
// Push the ivar on the stack
1473-
x86opnd_t out_opnd = ctx_stack_push(ctx, TYPE_UNKNOWN);
1474-
mov(cb, out_opnd, REG1);
1475-
}
1476-
else {
1477-
// Compile time value is *not* embeded.
1474+
// Guard that the variable is not Qundef
1475+
cmp(cb, REG1, imm_opnd(Qundef));
1476+
mov(cb, REG0, imm_opnd(Qnil));
1477+
cmove(cb, REG1, REG0);
14781478

1479-
// Guard that value is *not* embedded
1480-
// See ROBJECT_IVPTR() from include/ruby/internal/core/robject.h
1481-
ADD_COMMENT(cb, "guard extended getivar");
1482-
x86opnd_t flags_opnd = member_opnd(REG0, struct RBasic, flags);
1483-
test(cb, flags_opnd, imm_opnd(ROBJECT_EMBED));
1484-
jit_chain_guard(JCC_JNZ, jit, &starting_context, max_chain_depth, side_exit);
1485-
1486-
// check that the extended table is big enough
1487-
if (ivar_index >= ROBJECT_EMBED_LEN_MAX + 1) {
1488-
// Check that the slot is inside the extended table (num_slots > index)
1489-
x86opnd_t num_slots = mem_opnd(32, REG0, offsetof(struct RObject, as.heap.numiv));
1490-
cmp(cb, num_slots, imm_opnd(ivar_index));
1491-
jle_ptr(cb, COUNTED_EXIT(side_exit, getivar_idx_out_of_range));
1492-
}
1479+
// Push the ivar on the stack
1480+
x86opnd_t out_opnd = ctx_stack_push(ctx, TYPE_UNKNOWN);
1481+
mov(cb, out_opnd, REG1);
1482+
}
1483+
else {
1484+
// Compile time value is *not* embeded.
14931485

1494-
// Get a pointer to the extended table
1495-
x86opnd_t tbl_opnd = mem_opnd(64, REG0, offsetof(struct RObject, as.heap.ivptr));
1496-
mov(cb, REG0, tbl_opnd);
1486+
// Guard that value is *not* embedded
1487+
// See ROBJECT_IVPTR() from include/ruby/internal/core/robject.h
1488+
ADD_COMMENT(cb, "guard extended getivar");
1489+
x86opnd_t flags_opnd = member_opnd(REG0, struct RBasic, flags);
1490+
test(cb, flags_opnd, imm_opnd(ROBJECT_EMBED));
1491+
jit_chain_guard(JCC_JNZ, jit, &starting_context, max_chain_depth, side_exit);
1492+
1493+
// check that the extended table is big enough
1494+
if (ivar_index >= ROBJECT_EMBED_LEN_MAX + 1) {
1495+
// Check that the slot is inside the extended table (num_slots > index)
1496+
x86opnd_t num_slots = mem_opnd(32, REG0, offsetof(struct RObject, as.heap.numiv));
1497+
cmp(cb, num_slots, imm_opnd(ivar_index));
1498+
jle_ptr(cb, COUNTED_EXIT(side_exit, getivar_idx_out_of_range));
1499+
}
14971500

1498-
// Read the ivar from the extended table
1499-
x86opnd_t ivar_opnd = mem_opnd(64, REG0, sizeof(VALUE) * ivar_index);
1500-
mov(cb, REG0, ivar_opnd);
1501+
// Get a pointer to the extended table
1502+
x86opnd_t tbl_opnd = mem_opnd(64, REG0, offsetof(struct RObject, as.heap.ivptr));
1503+
mov(cb, REG0, tbl_opnd);
15011504

1502-
// Check that the ivar is not Qundef
1503-
cmp(cb, REG0, imm_opnd(Qundef));
1504-
mov(cb, REG1, imm_opnd(Qnil));
1505-
cmove(cb, REG0, REG1);
1505+
// Read the ivar from the extended table
1506+
x86opnd_t ivar_opnd = mem_opnd(64, REG0, sizeof(VALUE) * ivar_index);
1507+
mov(cb, REG0, ivar_opnd);
15061508

1507-
// Push the ivar on the stack
1508-
x86opnd_t out_opnd = ctx_stack_push(ctx, TYPE_UNKNOWN);
1509-
mov(cb, out_opnd, REG0);
1510-
}
1509+
// Check that the ivar is not Qundef
1510+
cmp(cb, REG0, imm_opnd(Qundef));
1511+
mov(cb, REG1, imm_opnd(Qnil));
1512+
cmove(cb, REG0, REG1);
15111513

1512-
// Jump to next instruction. This allows guard chains to share the same successor.
1513-
jit_jump_to_next_insn(jit, ctx);
1514-
return YJIT_END_BLOCK;
1514+
// Push the ivar on the stack
1515+
x86opnd_t out_opnd = ctx_stack_push(ctx, TYPE_UNKNOWN);
1516+
mov(cb, out_opnd, REG0);
15151517
}
15161518

1517-
GEN_COUNTER_INC(cb, getivar_name_not_mapped);
1518-
return YJIT_CANT_COMPILE;
1519+
// Jump to next instruction. This allows guard chains to share the same successor.
1520+
jit_jump_to_next_insn(jit, ctx);
1521+
return YJIT_END_BLOCK;
15191522
}
15201523

15211524
static codegen_status_t

‎yjit_iface.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ YJIT_DECLARE_COUNTERS(
6969

7070
getivar_se_self_not_heap,
7171
getivar_idx_out_of_range,
72-
getivar_name_not_mapped,
7372

7473
setivar_se_self_not_heap,
7574
setivar_idx_out_of_range,

0 commit comments

Comments
 (0)
0