From a0f12a0258e4020bd657ee80b7d8f22bd33ea223 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Fri, 3 Jul 2020 16:56:44 +0900 Subject: [PATCH] Use ID instead of GENTRY for gvars. (#3278) Use ID instead of GENTRY for gvars. Global variables are compiled into GENTRY (a pointer to struct rb_global_entry). This patch replace this GENTRY to ID and make the code simple. We need to search GENTRY from ID every time (st_lookup), so additional overhead will be introduced. However, the performance of accessing global variables is not important now a day and this simplicity helps Ractor development. --- compile.c | 50 ++--------------- insns.def | 14 +++-- internal/variable.h | 20 ++----- iseq.c | 12 ----- node.c | 2 +- node.h | 7 ++- tool/ruby_vm/models/typemap.rb | 1 - tool/ruby_vm/views/_leaf_helpers.erb | 57 -------------------- variable.c | 81 ++++++++++++++++------------ vm_insnhelper.c | 2 +- 10 files changed, 68 insertions(+), 178 deletions(-) diff --git a/compile.c b/compile.c index ccb079c5e0c972..dfa38025796dd0 100644 --- a/compile.c +++ b/compile.c @@ -2318,13 +2318,6 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor) case TS_ID: /* ID */ generated_iseq[code_index + 1 + j] = SYM2ID(operands[j]); break; - case TS_GENTRY: - { - struct rb_global_entry *entry = - (struct rb_global_entry *)(operands[j] & (~1)); - generated_iseq[code_index + 1 + j] = (VALUE)entry; - } - break; case TS_FUNCPTR: generated_iseq[code_index + 1 + j] = operands[j]; break; @@ -4875,7 +4868,7 @@ defined_expr0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, case NODE_GVAR: ADD_INSN(ret, line, putnil); ADD_INSN3(ret, line, defined, INT2FIX(DEFINED_GVAR), - ID2SYM(node->nd_entry->id), needstr); + ID2SYM(node->nd_entry), needstr); return; case NODE_CVAR: @@ -5262,7 +5255,7 @@ compile_named_capture_assign(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE LABEL *fail_label = NEW_LABEL(line), *end_label = NEW_LABEL(line); #if !(defined(NAMED_CAPTURE_BY_SVAR) && NAMED_CAPTURE_BY_SVAR-0) - ADD_INSN1(ret, line, getglobal, ((VALUE)rb_global_entry(idBACKREF) | 1)); + ADD_INSN1(ret, line, getglobal, ID2SYM(idBACKREF)); #else ADD_INSN2(ret, line, getspecial, INT2FIX(1) /* '~' */, INT2FIX(0)); #endif @@ -7599,8 +7592,7 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, in if (!popped) { ADD_INSN(ret, line, dup); } - ADD_INSN1(ret, line, setglobal, - ((VALUE)node->nd_entry | 1)); + ADD_INSN1(ret, line, setglobal, ID2SYM(node->nd_entry)); break; } case NODE_IASGN:{ @@ -8212,8 +8204,7 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, in break; } case NODE_GVAR:{ - ADD_INSN1(ret, line, getglobal, - ((VALUE)node->nd_entry | 1)); + ADD_INSN1(ret, line, getglobal, ID2SYM(node->nd_entry)); if (popped) { ADD_INSN(ret, line, pop); } @@ -8980,13 +8971,6 @@ insn_data_to_s_detail(INSN *iobj) case TS_ID: /* ID */ rb_str_concat(str, opobj_inspect(OPERAND_AT(iobj, j))); break; - case TS_GENTRY: - { - struct rb_global_entry *entry = (struct rb_global_entry *) - (OPERAND_AT(iobj, j) & (~1)); - rb_str_append(str, rb_id2str(entry->id)); - break; - } case TS_IC: /* inline cache */ case TS_IVC: /* inline ivar cache */ case TS_ISE: /* inline storage entry */ @@ -9376,10 +9360,6 @@ iseq_build_from_ary_body(rb_iseq_t *iseq, LINK_ANCHOR *const anchor, } } break; - case TS_GENTRY: - op = rb_to_symbol_type(op); - argv[j] = (VALUE)rb_global_entry(SYM2ID(op)); - break; case TS_ISE: FL_SET((VALUE)iseq, ISEQ_MARKABLE_ISEQ); /* fall through */ @@ -10136,19 +10116,6 @@ ibf_dump_iseq(struct ibf_dump *dump, const rb_iseq_t *iseq) } } -static VALUE -ibf_dump_gentry(struct ibf_dump *dump, const struct rb_global_entry *entry) -{ - return (VALUE)ibf_dump_id(dump, entry->id); -} - -static VALUE -ibf_load_gentry(const struct ibf_load *load, const struct rb_global_entry *entry) -{ - ID gid = ibf_load_id(load, (ID)(VALUE)entry); - return (VALUE)rb_global_entry(gid); -} - static unsigned char ibf_load_byte(const struct ibf_load *load, ibf_offset_t *offset) { @@ -10316,9 +10283,6 @@ ibf_dump_code(struct ibf_dump *dump, const rb_iseq_t *iseq) case TS_ID: wv = ibf_dump_id(dump, (ID)op); break; - case TS_GENTRY: - wv = ibf_dump_gentry(dump, (const struct rb_global_entry *)op); - break; case TS_FUNCPTR: rb_raise(rb_eRuntimeError, "TS_FUNCPTR is not supported"); goto skip_wv; @@ -10403,12 +10367,6 @@ ibf_load_code(const struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t bytecod code[code_index] = ibf_load_id(load, (ID)(VALUE)op); } break; - case TS_GENTRY: - { - VALUE op = ibf_load_small_value(load, &reading_pos); - code[code_index] = ibf_load_gentry(load, (const struct rb_global_entry *)(VALUE)op); - } - break; case TS_FUNCPTR: rb_raise(rb_eRuntimeError, "TS_FUNCPTR is not supported"); break; diff --git a/insns.def b/insns.def index 95f13915a56819..c4e80040b6b2c3 100644 --- a/insns.def +++ b/insns.def @@ -288,25 +288,23 @@ setconstant /* get global variable id. */ DEFINE_INSN getglobal -(GENTRY entry) +(ID gid) () (VALUE val) -// attr bool leaf = leafness_of_getglobal(entry); +// attr bool leaf = false; { - struct rb_global_entry *gentry = (void *)entry; - val = rb_gvar_get(gentry); + val = rb_gvar_get(gid); } /* set global variable id as val. */ DEFINE_INSN setglobal -(GENTRY entry) +(ID gid) (VALUE val) () -// attr bool leaf = leafness_of_setglobal(entry); +// attr bool leaf = false; { - struct rb_global_entry *gentry = (void *)entry; - rb_gvar_set(gentry, val); + rb_gvar_set(gid, val); } /**********************************************************/ diff --git a/internal/variable.h b/internal/variable.h index f0b7b215a465ac..6ed728076e37c3 100644 --- a/internal/variable.h +++ b/internal/variable.h @@ -19,13 +19,6 @@ #define ROBJECT_TRANSIENT_FLAG FL_USER13 -struct rb_global_variable; /* defined in variable.c */ - -struct rb_global_entry { - struct rb_global_variable *var; - ID id; -}; - /* variable.c */ void rb_gc_mark_global_tbl(void); void rb_gc_update_global_tbl(void); @@ -36,9 +29,8 @@ VALUE rb_ivar_lookup(VALUE obj, ID id, VALUE undef); void rb_autoload_str(VALUE mod, ID id, VALUE file); VALUE rb_autoload_at_p(VALUE, ID, int); NORETURN(VALUE rb_mod_const_missing(VALUE,VALUE)); -rb_gvar_getter_t *rb_gvar_getter_function_of(const struct rb_global_entry *); -rb_gvar_setter_t *rb_gvar_setter_function_of(const struct rb_global_entry *); -bool rb_gvar_is_traced(const struct rb_global_entry *); +rb_gvar_getter_t *rb_gvar_getter_function_of(ID); +rb_gvar_setter_t *rb_gvar_setter_function_of(ID); void rb_gvar_readonly_setter(VALUE v, ID id, VALUE *_); static inline bool ROBJ_TRANSIENT_P(VALUE obj); static inline void ROBJ_TRANSIENT_SET(VALUE obj); @@ -55,11 +47,9 @@ void rb_deprecate_constant(VALUE mod, const char *name); RUBY_SYMBOL_EXPORT_END MJIT_SYMBOL_EXPORT_BEGIN -struct rb_global_entry *rb_global_entry(ID); -VALUE rb_gvar_get(struct rb_global_entry *); -VALUE rb_gvar_set(struct rb_global_entry *, VALUE); -VALUE rb_gvar_defined(struct rb_global_entry *); -struct st_table *rb_ivar_generic_ivtbl(void); +VALUE rb_gvar_get(ID); +VALUE rb_gvar_set(ID, VALUE); +VALUE rb_gvar_defined(ID); void rb_const_warn_if_deprecated(const rb_const_entry_t *, VALUE, ID); MJIT_SYMBOL_EXPORT_END diff --git a/iseq.c b/iseq.c index 956bd2a6971a62..e6568a6a2e42f0 100644 --- a/iseq.c +++ b/iseq.c @@ -1946,12 +1946,6 @@ rb_insn_operand_intern(const rb_iseq_t *iseq, } break; } - case TS_GENTRY: - { - struct rb_global_entry *entry = (struct rb_global_entry *)op; - ret = rb_str_dup(rb_id2str(entry->id)); - } - break; case TS_IC: case TS_IVC: @@ -2776,12 +2770,6 @@ iseq_data_to_ary(const rb_iseq_t *iseq) } } break; - case TS_GENTRY: - { - struct rb_global_entry *entry = (struct rb_global_entry *)*seq; - rb_ary_push(ary, ID2SYM(entry->id)); - } - break; case TS_IC: case TS_IVC: case TS_ISE: diff --git a/node.c b/node.c index 4ef18db544b239..7a2fce199753f3 100644 --- a/node.c +++ b/node.c @@ -63,7 +63,7 @@ #define SIMPLE_FIELD1(name, ann) SIMPLE_FIELD(FIELD_NAME_LEN(name, ann), FIELD_NAME_DESC(name, ann)) #define F_CUSTOM1(name, ann) SIMPLE_FIELD1(#name, ann) #define F_ID(name, ann) SIMPLE_FIELD1(#name, ann) A_ID(node->name) -#define F_GENTRY(name, ann) SIMPLE_FIELD1(#name, ann) A_ID((node->name)->id) +#define F_GENTRY(name, ann) SIMPLE_FIELD1(#name, ann) A_ID(node->name) #define F_INT(name, ann) SIMPLE_FIELD1(#name, ann) A_INT(node->name) #define F_LONG(name, ann) SIMPLE_FIELD1(#name, ann) A_LONG(node->name) #define F_LIT(name, ann) SIMPLE_FIELD1(#name, ann) A_LIT(node->name) diff --git a/node.h b/node.h index 60e9604f463334..84c5d09b832e3d 100644 --- a/node.h +++ b/node.h @@ -164,7 +164,6 @@ typedef struct RNode { struct RNode *node; ID id; long state; - struct rb_global_entry *entry; struct rb_args_info *args; struct rb_ary_pattern_info *apinfo; struct rb_fnd_pattern_info *fpinfo; @@ -228,7 +227,7 @@ typedef struct RNode { #define nd_stts u1.node -#define nd_entry u3.entry +#define nd_entry u3.id #define nd_vid u1.id #define nd_cflag u2.id #define nd_cval u3.value @@ -316,7 +315,7 @@ typedef struct RNode { #define NEW_ZLIST(loc) NEW_NODE(NODE_ZLIST,0,0,0,loc) #define NEW_HASH(a,loc) NEW_NODE(NODE_HASH,a,0,0,loc) #define NEW_MASGN(l,r,loc) NEW_NODE(NODE_MASGN,l,0,r,loc) -#define NEW_GASGN(v,val,loc) NEW_NODE(NODE_GASGN,v,val,rb_global_entry(v),loc) +#define NEW_GASGN(v,val,loc) NEW_NODE(NODE_GASGN,v,val,v,loc) #define NEW_LASGN(v,val,loc) NEW_NODE(NODE_LASGN,v,val,0,loc) #define NEW_DASGN(v,val,loc) NEW_NODE(NODE_DASGN,v,val,0,loc) #define NEW_DASGN_CURR(v,val,loc) NEW_NODE(NODE_DASGN_CURR,v,val,0,loc) @@ -329,7 +328,7 @@ typedef struct RNode { #define NEW_OP_ASGN_OR(i,val,loc) NEW_NODE(NODE_OP_ASGN_OR,i,val,0,loc) #define NEW_OP_ASGN_AND(i,val,loc) NEW_NODE(NODE_OP_ASGN_AND,i,val,0,loc) #define NEW_OP_CDECL(v,op,val,loc) NEW_NODE(NODE_OP_CDECL,v,val,op,loc) -#define NEW_GVAR(v,loc) NEW_NODE(NODE_GVAR,v,0,rb_global_entry(v),loc) +#define NEW_GVAR(v,loc) NEW_NODE(NODE_GVAR,v,0,v,loc) #define NEW_LVAR(v,loc) NEW_NODE(NODE_LVAR,v,0,0,loc) #define NEW_DVAR(v,loc) NEW_NODE(NODE_DVAR,v,0,0,loc) #define NEW_IVAR(v,loc) NEW_NODE(NODE_IVAR,v,0,0,loc) diff --git a/tool/ruby_vm/models/typemap.rb b/tool/ruby_vm/models/typemap.rb index c4b13f67f992bd..ed3aea7d2e2503 100644 --- a/tool/ruby_vm/models/typemap.rb +++ b/tool/ruby_vm/models/typemap.rb @@ -14,7 +14,6 @@ "..." => %w[. TS_VARIABLE], "CALL_DATA" => %w[C TS_CALLDATA], "CDHASH" => %w[H TS_CDHASH], - "GENTRY" => %w[G TS_GENTRY], "IC" => %w[K TS_IC], "IVC" => %w[A TS_IVC], "ID" => %w[I TS_ID], diff --git a/tool/ruby_vm/views/_leaf_helpers.erb b/tool/ruby_vm/views/_leaf_helpers.erb index ac60f2dcbc909c..2637f8777dd7f2 100644 --- a/tool/ruby_vm/views/_leaf_helpers.erb +++ b/tool/ruby_vm/views/_leaf_helpers.erb @@ -8,63 +8,6 @@ %; #line <%= __LINE__ + 1 %> <%=cstr __FILE__ %> -static bool -leafness_of_getglobal(VALUE gentry) -{ - const struct rb_global_entry *e = (void *)gentry; - - if (UNLIKELY(rb_gvar_is_traced(e))) { - return false; - } - else { - /* We cannot write this function using a switch() because a - * case label cannot be a function pointer. */ - static rb_gvar_getter_t *const allowlist[] = { - rb_gvar_val_getter, - rb_gvar_var_getter, - /* rb_gvar_undef_getter issues rb_warning() */ - }; - rb_gvar_getter_t *f = rb_gvar_getter_function_of(e); - int i; - - for (i = 0; i < numberof(allowlist); i++) { - if (f == allowlist[i]) { - return true; - } - } - return false; - } -} - -static bool -leafness_of_setglobal(VALUE gentry) -{ - const struct rb_global_entry *e = (void *)gentry; - - if (UNLIKELY(rb_gvar_is_traced(e))) { - return false; - } - else { - /* We cannot write this function using a switch() because a - * case label cannot be a function pointer. */ - static rb_gvar_setter_t *const allowlist[] = { - rb_gvar_val_setter, - /* rb_gvar_readonly_setter issues rb_name_error() */ - rb_gvar_var_setter, - rb_gvar_undef_setter, - }; - rb_gvar_setter_t *f = rb_gvar_setter_function_of(e); - int i; - - for (i = 0; i < numberof(allowlist); i++) { - if (f == allowlist[i]) { - return true; - } - } - return false; - } -} - #include "iseq.h" static bool diff --git a/variable.c b/variable.c index 4d62361fd07fa7..5d3785b346926d 100644 --- a/variable.c +++ b/variable.c @@ -325,13 +325,24 @@ struct rb_global_variable { struct trace_var *trace; }; +struct rb_global_entry { + struct rb_global_variable *var; + ID id; +}; + +static struct rb_id_table * +global_tbl(void) +{ + return rb_global_tbl; +} + static struct rb_global_entry* rb_find_global_entry(ID id) { struct rb_global_entry *entry; VALUE data; - if (!rb_id_table_lookup(rb_global_tbl, id, &data)) { + if (!rb_id_table_lookup(global_tbl(), id, &data)) { return NULL; } entry = (struct rb_global_entry *)data; @@ -344,7 +355,7 @@ rb_gvar_undef_compactor(void *var) { } -MJIT_FUNC_EXPORTED struct rb_global_entry* +static struct rb_global_entry* rb_global_entry(ID id) { struct rb_global_entry *entry = rb_find_global_entry(id); @@ -363,7 +374,7 @@ rb_global_entry(ID id) var->block_trace = 0; var->trace = 0; - rb_id_table_insert(rb_global_tbl, id, (VALUE)entry); + rb_id_table_insert(global_tbl(), id, (VALUE)entry); } return entry; } @@ -472,8 +483,9 @@ mark_global_entry(VALUE v, void *ignored) void rb_gc_mark_global_tbl(void) { - if (rb_global_tbl) + if (rb_global_tbl) { rb_id_table_foreach_values(rb_global_tbl, mark_global_entry, 0); + } } static enum rb_id_table_iterator_result @@ -640,7 +652,7 @@ rb_f_untrace_var(int argc, const VALUE *argv) if (!id) { rb_name_error_str(var, "undefined global variable %"PRIsVALUE"", QUOTE(var)); } - if (!rb_id_table_lookup(rb_global_tbl, id, &data)) { + if (!rb_id_table_lookup(global_tbl(), id, &data)) { rb_name_error(id, "undefined global variable %"PRIsVALUE"", QUOTE_ID(id)); } @@ -671,13 +683,6 @@ rb_f_untrace_var(int argc, const VALUE *argv) return Qnil; } -MJIT_FUNC_EXPORTED VALUE -rb_gvar_get(struct rb_global_entry *entry) -{ - struct rb_global_variable *var = entry->var; - return (*var->getter)(entry->id, var->data); -} - struct trace_data { struct trace_var *trace; VALUE val; @@ -706,8 +711,8 @@ trace_en(VALUE v) return Qnil; /* not reached */ } -MJIT_FUNC_EXPORTED VALUE -rb_gvar_set(struct rb_global_entry *entry, VALUE val) +static VALUE +rb_gvar_set_entry(struct rb_global_entry *entry, VALUE val) { struct trace_data trace; struct rb_global_variable *var = entry->var; @@ -724,54 +729,63 @@ rb_gvar_set(struct rb_global_entry *entry, VALUE val) } VALUE -rb_gv_set(const char *name, VALUE val) +rb_gvar_set(ID id, VALUE val) { struct rb_global_entry *entry; + entry = rb_global_entry(id); + + return rb_gvar_set_entry(entry, val); +} - entry = rb_global_entry(global_id(name)); - return rb_gvar_set(entry, val); +VALUE +rb_gv_set(const char *name, VALUE val) +{ + return rb_gvar_set(global_id(name), val); +} + +VALUE +rb_gvar_get(ID id) +{ + struct rb_global_entry *entry = rb_global_entry(id); + struct rb_global_variable *var = entry->var; + return (*var->getter)(entry->id, var->data); } VALUE rb_gv_get(const char *name) { - struct rb_global_entry *entry; ID id = find_global_id(name); - + if (!id) { rb_warning("global variable `%s' not initialized", name); return Qnil; } - entry = rb_global_entry(id); - return rb_gvar_get(entry); + return rb_gvar_get(id); } MJIT_FUNC_EXPORTED VALUE -rb_gvar_defined(struct rb_global_entry *entry) +rb_gvar_defined(ID id) { + struct rb_global_entry *entry = rb_global_entry(id); if (entry->var->getter == rb_gvar_undef_getter) return Qfalse; return Qtrue; } rb_gvar_getter_t * -rb_gvar_getter_function_of(const struct rb_global_entry *entry) +rb_gvar_getter_function_of(ID id) { + const struct rb_global_entry *entry = rb_global_entry(id); return entry->var->getter; } rb_gvar_setter_t * -rb_gvar_setter_function_of(const struct rb_global_entry *entry) +rb_gvar_setter_function_of(ID id) { + const struct rb_global_entry *entry = rb_global_entry(id); return entry->var->setter; } -bool -rb_gvar_is_traced(const struct rb_global_entry *entry) -{ - return !!entry->var->trace; -} - static enum rb_id_table_iterator_result gvar_i(ID key, VALUE val, void *a) { @@ -786,7 +800,7 @@ rb_f_global_variables(void) VALUE ary = rb_ary_new(); VALUE sym, backref = rb_backref_get(); - rb_id_table_foreach(rb_global_tbl, gvar_i, (void *)ary); + rb_id_table_foreach(global_tbl(), gvar_i, (void *)ary); if (!NIL_P(backref)) { char buf[2]; int i, nmatch = rb_match_count(backref); @@ -813,12 +827,13 @@ rb_alias_variable(ID name1, ID name2) { struct rb_global_entry *entry1, *entry2; VALUE data1; + struct rb_id_table *gtbl = global_tbl(); entry2 = rb_global_entry(name2); - if (!rb_id_table_lookup(rb_global_tbl, name1, &data1)) { + if (!rb_id_table_lookup(gtbl, name1, &data1)) { entry1 = ALLOC(struct rb_global_entry); entry1->id = name1; - rb_id_table_insert(rb_global_tbl, name1, (VALUE)entry1); + rb_id_table_insert(gtbl, name1, (VALUE)entry1); } else if ((entry1 = (struct rb_global_entry *)data1)->var != entry2->var) { struct rb_global_variable *var = entry1->var; diff --git a/vm_insnhelper.c b/vm_insnhelper.c index cacddc2b0e59e5..f2cab2f4b51eb6 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -3661,7 +3661,7 @@ vm_defined(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t op_ klass = vm_get_cbase(GET_EP()); break; case DEFINED_GVAR: - if (rb_gvar_defined(rb_global_entry(SYM2ID(obj)))) { + if (rb_gvar_defined(SYM2ID(obj))) { expr_type = DEFINED_GVAR; } break;