ruby-changes:54563
From: k0kubun <ko1@a...>
Date: Thu, 10 Jan 2019 23:31:23 +0900 (JST)
Subject: [ruby-changes:54563] k0kubun:r66778 (trunk): mjit.c: use boolean type for boolean variables
k0kubun 2019-01-10 23:31:18 +0900 (Thu, 10 Jan 2019) New Revision: 66778 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=66778 Log: mjit.c: use boolean type for boolean variables and functions to clarify the intention and make sure it's not used in a surprising way (like using 2, 3, ... other than 0, 1 even while it seems to be a boolean). This is a retry of r66775. It included some typos... Modified files: trunk/eval.c trunk/internal.h trunk/mjit.c trunk/mjit.h trunk/mjit_compile.c trunk/mjit_worker.c trunk/process.c trunk/tool/ruby_vm/views/mjit_compile.inc.erb trunk/vm_insnhelper.c Index: tool/ruby_vm/views/mjit_compile.inc.erb =================================================================== --- tool/ruby_vm/views/mjit_compile.inc.erb (revision 66777) +++ tool/ruby_vm/views/mjit_compile.inc.erb (revision 66778) @@ -58,7 +58,7 @@ switch (insn) { https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/mjit_compile.inc.erb#L58 if (b->stack_size != 1) { if (mjit_opts.warnings || mjit_opts.verbose) fprintf(stderr, "MJIT warning: Unexpected JIT stack_size on leave: %d\n", b->stack_size); - status->success = FALSE; + status->success = false; } % end % @@ -72,6 +72,6 @@ switch (insn) { https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/mjit_compile.inc.erb#L72 default: if (mjit_opts.warnings || mjit_opts.verbose) fprintf(stderr, "MJIT warning: Skipped to compile unsupported instruction: %s\n", insn_name(insn)); - status->success = FALSE; + status->success = false; break; } Index: mjit_compile.c =================================================================== --- mjit_compile.c (revision 66777) +++ mjit_compile.c (revision 66778) @@ -29,12 +29,12 @@ https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L29 which is global during one `mjit_compile` call. Ones conditional in each branch should be stored in `compile_branch`. */ struct compile_status { - int success; /* has TRUE if compilation has had no issue */ - int *stack_size_for_pos; /* stack_size_for_pos[pos] has stack size for the position (otherwise -1) */ - /* If TRUE, JIT-ed code will use local variables to store pushed values instead of - using VM's stack and moving stack pointer. */ - int local_stack_p; - /* Safely-accessible cache entries copied from main thread. */ + bool success; // has true if compilation has had no issue + int *stack_size_for_pos; // stack_size_for_pos[pos] has stack size for the position (otherwise -1) + // If true, JIT-ed code will use local variables to store pushed values instead of + // using VM's stack and moving stack pointer. + bool local_stack_p; + // Safely-accessible cache entries copied from main thread. union iseq_inline_storage_entry *is_entries; struct rb_call_cache *cc_entries; }; @@ -43,8 +43,8 @@ struct compile_status { https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L43 This is created and used for one `compile_insns` call and its values should be copied for extra `compile_insns` call. */ struct compile_branch { - unsigned int stack_size; /* this simulates sp (stack pointer) of YARV */ - int finish_p; /* if TRUE, compilation in this branch should stop and let another branch to be compiled */ + unsigned int stack_size; // this simulates sp (stack pointer) of YARV + bool finish_p; // if true, compilation in this branch should stop and let another branch to be compiled }; struct case_dispatch_var { @@ -53,21 +53,21 @@ struct case_dispatch_var { https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L53 VALUE last_value; }; -/* Returns TRUE if call cache is still not obsoleted and cc->me->def->type is available. */ -static int +// Returns true if call cache is still not obsoleted and cc->me->def->type is available. +static bool has_valid_method_type(CALL_CACHE cc) { - extern int mjit_valid_class_serial_p(rb_serial_t class_serial); + extern bool mjit_valid_class_serial_p(rb_serial_t class_serial); return GET_GLOBAL_METHOD_STATE() == cc->method_state && mjit_valid_class_serial_p(cc->class_serial) && cc->me; } -/* Returns TRUE if iseq is inlinable, otherwise NULL. This becomes TRUE in the same condition - as CC_SET_FASTPATH (in vm_callee_setup_arg) is called from vm_call_iseq_setup. */ -static int +// Returns true if iseq is inlinable, otherwise NULL. This becomes true in the same condition +// as CC_SET_FASTPATH (in vm_callee_setup_arg) is called from vm_call_iseq_setup. +static bool inlinable_iseq_p(CALL_INFO ci, CALL_CACHE cc, const rb_iseq_t *iseq) { - extern int rb_simple_iseq_p(const rb_iseq_t *iseq); + extern bool rb_simple_iseq_p(const rb_iseq_t *iseq); return iseq != NULL && rb_simple_iseq_p(iseq) && !(ci->flag & VM_CALL_KW_SPLAT) /* Top of vm_callee_setup_arg. In this case, opt_pc is 0. */ && (!IS_ARGS_SPLAT(ci) && !IS_ARGS_KEYWORD(ci) && !(METHOD_ENTRY_VISI(cc->me) == METHOD_VISI_PROTECTED)); /* CC_SET_FASTPATH */ @@ -143,7 +143,7 @@ compile_insn(FILE *f, const struct rb_is https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L143 if (mjit_opts.warnings || mjit_opts.verbose) fprintf(stderr, "MJIT warning: JIT stack assumption is not the same between branches (%d != %u)\n", status->stack_size_for_pos[next_pos], b->stack_size); - status->success = FALSE; + status->success = false; } } @@ -160,7 +160,7 @@ compile_insns(FILE *f, const struct rb_i https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L160 struct compile_branch branch; branch.stack_size = stack_size; - branch.finish_p = FALSE; + branch.finish_p = false; while (pos < body->iseq_size && !ALREADY_COMPILED_P(status, pos) && !branch.finish_p) { #if OPT_DIRECT_THREADED_CODE || OPT_CALL_THREADED_CODE @@ -175,7 +175,7 @@ compile_insns(FILE *f, const struct rb_i https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L175 if (status->success && branch.stack_size > body->stack_max) { if (mjit_opts.warnings || mjit_opts.verbose) fprintf(stderr, "MJIT warning: JIT stack size (%d) exceeded its max size (%d)\n", branch.stack_size, body->stack_max); - status->success = FALSE; + status->success = false; } if (!status->success) break; @@ -196,16 +196,16 @@ compile_cancel_handler(FILE *f, const st https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L196 fprintf(f, " return Qundef;\n"); } -/* Compile ISeq to C code in F. It returns 1 if it succeeds to compile. */ -int +// Compile ISeq to C code in `f`. It returns true if it succeeds to compile. +bool mjit_compile(FILE *f, const struct rb_iseq_constant_body *body, const char *funcname, struct rb_call_cache *cc_entries, union iseq_inline_storage_entry *is_entries) { struct compile_status status; - status.success = TRUE; + status.success = true; status.local_stack_p = !body->catch_except_p; status.stack_size_for_pos = (int *)malloc(sizeof(int) * body->iseq_size); if (status.stack_size_for_pos == NULL) - return FALSE; + return false; memset(status.stack_size_for_pos, NOT_COMPILED_STACK_SIZE, sizeof(int) * body->iseq_size); status.cc_entries = cc_entries; status.is_entries = is_entries; Index: mjit_worker.c =================================================================== --- mjit_worker.c (revision 66777) +++ mjit_worker.c (revision 66778) @@ -132,10 +132,10 @@ struct rb_mjit_unit { https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L132 #ifndef _MSC_VER /* This value is always set for `compact_all_jit_code`. Also used for lazy deletion. */ char *o_file; - /* TRUE if it's inherited from parent Ruby process and lazy deletion should be skipped. + /* true if it's inherited from parent Ruby process and lazy deletion should be skipped. `o_file = NULL` can't be used to skip lazy deletion because `o_file` could be used by child for `compact_all_jit_code`. */ - int o_file_inherited_p; + bool o_file_inherited_p; #endif #if defined(_WIN32) /* DLL cannot be removed while loaded on Windows. If this is set, it'll be lazily deleted. */ @@ -171,11 +171,11 @@ extern rb_pid_t ruby_waitpid_locked(rb_v https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L171 freed. */ struct mjit_options mjit_opts; -/* TRUE if MJIT is enabled. */ -int mjit_enabled = FALSE; -/* TRUE if JIT-ed code should be called. When `ruby_vm_event_enabled_global_flags & ISEQ_TRACE_EVENTS` - and `mjit_call_p == FALSE`, any JIT-ed code execution is cancelled as soon as possible. */ -int mjit_call_p = FALSE; +// true if MJIT is enabled. +bool mjit_enabled = false; +// true if JIT-ed code should be called. When `ruby_vm_event_enabled_global_flags & ISEQ_TRACE_EVENTS` +// and `mjit_call_p == false`, any JIT-ed code execution is cancelled as soon as possible. +bool mjit_call_p = false; /* Priority queue of iseqs waiting for JIT compilation. This variable is a pointer to head unit of the queue. */ @@ -198,14 +198,14 @@ static rb_nativethread_cond_t mjit_clien https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L198 static rb_nativethread_cond_t mjit_worker_wakeup; /* A thread conditional to wake up workers if at the end of GC. */ static rb_nativethread_cond_t mjit_gc_wakeup; -/* True when GC is working. */ -static int in_gc; -/* True when JIT is working. */ -static int in_jit; -/* Set to TRUE to stop worker. */ -static int stop_worker_p; -/* Set to TRUE if worker is stopped. */ -static int worker_stopped; +// True when GC is working. +static bool in_gc; +// True when JIT is working. +static bool in_jit; +// Set to TRUE to stop worker. +static bool stop_worker_p; +// Set to TRUE if worker is stopped. +static bool worker_stopped; /* Path of "/tmp", which can be changed to $TMP in MinGW. */ static char *tmp_dir; @@ -363,7 +363,7 @@ clean_object_files(struct rb_mjit_unit * https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L363 char *so_file = unit->so_file; unit->so_file = NULL; - /* unit->so_file is set only when mjit_opts.save_temps is FALSE. */ + // unit->so_file is set only when mjit_opts.save_temps is false. remove_file(so_file); free(so_file); } @@ -444,14 +444,12 @@ real_ms_time(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L444 } #endif -/* Return TRUE if class_serial is not obsoleted. This is used by mjit_compile.c. */ -int +// Return true if class_serial is not obsoleted. This is used by mjit_compile.c. +bool mjit_valid_class_serial_p(rb_serial_t class_serial) { - int found_p; - CRITICAL_SECTION_START(3, "in valid_class_serial_p"); - found_p = rb_hash_stlike_lookup(valid_class_serials, LONG2FIX(class_serial), NULL); + bool found_p = rb_hash_stlike_lookup(valid_class_serials, LONG2FIX(class_serial), NULL); CRITICAL_SECTION_FINISH(3, "in valid_class_serial_p"); return found_p; } @@ -652,8 +650,8 @@ remove_so_file(const char *so_file, stru https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L650 #define append_lit(p, str) append_str2(p, str, rb_strlen_lit(str)) #ifdef _MSC_VER -/* Compile C file to so. It returns 1 if it succeeds. (mswin) */ -static int +// Compile C file to so. It returns true if it succeeds. (mswin) +static bool compile_c_to_so(const char *c_file, const char *so_file) { int exit_code; @@ -703,7 +701,7 @@ compile_c_to_so(const char *c_file, cons https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L701 args = form_args(5, CC_LDSHARED_ARGS, CC_CODEFLAG_ARGS, files, CC_LIBS, CC_DLDFLAGS_ARGS); if (args == NULL) - return FALSE; + return false; exit_code = exec_process(cc_path, args); free(args); @@ -770,8 +768,8 @@ make_pch(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L768 CRITICAL_SECTION_FINISH(3, "in make_pch"); } -/* Compile .c file to .o file. It returns 1 if it succeeds. (non-mswin) */ -static int +// Compile .c file to .o file. It returns true if it succeeds. (non-mswin) +static bool compile_c_to_o(const char *c_file, const char *o_file) { int exit_code; @@ -791,7 +789,7 @@ compile_c_to_o(const char *c_file, const https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L789 # endif args = form_args(5, cc_common_args, CC_CODEFLAG_ARGS, files, CC_LIBS, CC_DLDFLAGS_ARGS); if (args == NULL) - return FALSE; + return false; exit_code = exec_process(cc_path, args); free(args); @@ -801,8 +799,8 @@ compile_c_to_o(const char *c_file, const https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L799 return exit_code == 0; } -/* Link .o files to .so file. It returns 1 if it succeeds. (non-mswin) */ -static int +// Link .o files to .so file. It returns true if it succeeds. (non-mswin) +static bool link_o_to_so(const char **o_files, const char *so_file) { int exit_code; @@ -819,7 +817,7 @@ link_o_to_so(const char **o_files, const https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L817 args = form_args(6, CC_LDSHARED_ARGS, CC_CODEFLAG_ARGS, options, o_files, CC_LIBS, CC_DLDFLAGS_ARGS); if (args == NULL) - return FALSE; + return false; exit_code = exec_process(cc_path, args); free(args); @@ -840,7 +838,7 @@ compact_all_jit_code(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L838 static const char so_ext[] = DLEXT; char so_file[MAXPATHLEN]; const char **o_files; - int i = 0, success; + int i = 0; /* Abnormal use case of rb_mjit_unit that doesn't have ISeq */ unit = calloc(1, sizeof(struct rb_mjit_unit)); /* To prevent GC, don't use ZALLOC */ @@ -858,7 +856,7 @@ compact_all_jit_code(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L856 } start_time = real_ms_time(); - success = link_o_to_so(o_files, so_file); + bool success = link_o_to_so(o_files, so_file); end_time = real_ms_time(); /* TODO: Shrink this big critical section. For now, this is needed to prevent failure by missing .o files. @@ -983,7 +981,6 @@ static mjit_func_t https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L981 convert_unit_to_func(struct rb_mjit_unit *unit, struct rb_call_cache *cc_entries, union iseq_inline_storage_entry *is_entries) { char c_file_buff[MAXPATHLEN], *c_file = c_file_buff, *so_file, funcname[35]; /* TODO: reconsider `35` */ - int success; int fd; FILE *f; void *func; @@ -1044,10 +1041,10 @@ convert_unit_to_func(struct rb_mjit_unit https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1041 if (!mjit_opts.save_temps) remove_file(c_file); free_unit(unit); - in_jit = FALSE; /* just being explicit for return */ + in_jit = false; // just being explicit for return } else { - in_jit = TRUE; + in_jit = true; } CRITICAL_SECTION_FINISH(3, "before mjit_compile to wait GC finish"); if (!in_jit) { @@ -1062,11 +1059,11 @@ convert_unit_to_func(struct rb_mjit_unit https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1059 verbose(2, "start compilation: %s@%s:%d -> %s", label, path, lineno, c_file); fprintf(f, "/* %s@%s:%d */\n\n", label, path, lineno); } - success = mjit_compile(f, unit->iseq->body, funcname, cc_entries, is_entries); + bool success = mjit_compile(f, unit->iseq->body, funcname, cc_entries, is_entries); /* release blocking mjit_gc_start_hook */ CRITICAL_SECTION_START(3, "after mjit_compile to wakeup client for GC"); - in_jit = FALSE; + in_jit = false; verbose(3, "Sending wakeup signal to client in a mjit-worker for GC"); rb_native_cond_signal(&mjit_client_wakeup); CRITICAL_SECTION_FINISH(3, "in worker to wakeup client for GC"); @@ -1084,7 +1081,7 @@ convert_unit_to_func(struct rb_mjit_unit https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1081 success = compile_c_to_so(c_file, so_file); #else /* splitting .c -> .o step and .o -> .so step, to cache .o files in the future */ - if ((success = compile_c_to_o(c_file, o_file)) != 0) { + if ((success = compile_c_to_o(c_file, o_file)) != false) { const char *o_files[2] = { NULL, NULL }; o_files[0] = o_file; success = link_o_to_so(o_files, so_file); @@ -1124,7 +1121,7 @@ typedef struct { https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1121 struct rb_mjit_unit *unit; struct rb_call_cache *cc_entries; union iseq_inline_storage_entry *is_entries; - int finish_p; + bool finish_p; } mjit_copy_job_t; /* Singleton MJIT copy job. This is made global since it needs to be durable even when MJIT worker thread is stopped. @@ -1138,12 +1135,12 @@ int rb_workqueue_register(unsigned flags https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1135 /* We're lazily copying cache values from main thread because these cache values could be different between ones on enqueue timing and ones on dequeue timing. - Return TRUE if copy succeeds. */ -static int + Return true if copy succeeds. */ +static bool copy_cache_from_main_thread(mjit_copy_job_t *job) { CRITICAL_SECTION_START(3, "in copy_cache_from_main_thread"); - job->finish_p = FALSE; /* allow dispatching this job in mjit_copy_job_handler */ + job->finish_p = false; // allow dispatching this job in mjit_copy_job_handler CRITICAL_SECTION_FINISH(3, "in copy_cache_from_main_thread"); if (UNLIKELY(mjit_opts.wait)) { @@ -1152,7 +1149,7 @@ copy_cache_from_main_thread(mjit_copy_jo https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1149 } if (!rb_workqueue_register(0, mjit_copy_job_handler, (void *)job)) - return FALSE; + return false; CRITICAL_SECTION_START(3, "in MJIT copy job wait"); /* checking `stop_worker_p` too because `RUBY_VM_CHECK_INTS(ec)` may not lush mjit_copy_job_handler when EC_EXEC_TAG() is not TAG_NONE, and then @@ -1179,9 +1176,9 @@ mjit_worker(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1176 } #endif if (pch_status == PCH_FAILED) { - mjit_enabled = FALSE; + mjit_enabled = false; CRITICAL_SECTION_START(3, "in worker to update worker_stopped"); - worker_stopped = TRUE; + worker_stopped = true; verbose(3, "Sending wakeup signal to client in a mjit-worker"); rb_native_cond_signal(&mjit_client_wakeup); CRITICAL_SECTION_FINISH(3, "in worker to update worker_stopped"); @@ -1199,7 +1196,7 @@ mjit_worker(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1196 verbose(3, "Getting wakeup from client"); } unit = get_from_list(&unit_queue); - job->finish_p = TRUE; /* disable dispatching this job in mjit_copy_job_handler while it's being modified */ + job->finish_p = true; // disable dispatching this job in mjit_copy_job_handler while it's being modified CRITICAL_SECTION_FINISH(3, "in worker dequeue"); if (unit) { @@ -1216,7 +1213,7 @@ mjit_worker(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1213 /* Copy ISeq's inline caches values to avoid race condition. */ if (job->cc_entries != NULL || job->is_entries != NULL) { - if (copy_cache_from_main_thread(job) == FALSE) { + if (copy_cache_from_main_thread(job) == false) { continue; /* retry postponed_job failure, or stop worker */ } } @@ -1245,10 +1242,10 @@ mjit_worker(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1242 } } - /* Disable dispatching this job in mjit_copy_job_handler while memory allocated by alloca - could be expired after finishing this function. */ - job->finish_p = TRUE; + // Disable dispatching this job in mjit_copy_job_handler while memory allocated by alloca + // could be expired after finishing this function. + job->finish_p = true; - /* To keep mutex unlocked when it is destroyed by mjit_finish, don't wrap CRITICAL_SECTION here. */ - worker_stopped = TRUE; + // To keep mutex unlocked when it is destroyed by mjit_finish, don't wrap CRITICAL_SECTION here. + worker_stopped = true; } Index: internal.h ======================================== (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/