1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

YJIT: Fix invalidation for c_call and c_return (#6719)

Browse Source 
Follow-up for 2b8191bdad7545b71f270d2b25a34cd2b3afa02f. Since that
commit, we stopped doing code invalidation the second time the call and
return events are enabled. We need to do it every time these events are
enabled because we might have generated code while these events are
disabled.

Also rename locals and edit comments to make it more clear that the iseq
rewrite code path only happens the first time a particular iseq trace
event is enabled.
This commit is contained in:
Alan Wu 2022-11-13 12:51:19 -05:00 committed by GitHub
parent a1b460bd6e
commit bc8ba244b8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
Notes: git 2022-11-13 17:51:38 +00:00 
Merged-By: maximecb <maximecb@ruby-lang.org>
2 changed files with 29 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
bootstraptest/test_yjit.rb
View File

@ -2249,6 +2249,26 @@ assert_equal '[[:c_return, :itself, main]]', %q{
events
}
# test c_call invalidation
assert_equal '[[:c_call, :itself]]', %q{
# enable the event once to make sure invalidation
# happens the second time we enable it
TracePoint.new(:c_call) {}.enable{}
def compiled
itself
end
# assume first call compiles
compiled
events = []
tp = TracePoint.new(:c_call) { |tp| events << [tp.event, tp.method_id] }
tp.enable { compiled }
events
}
# test enabling tracing for a suspended fiber
assert_equal '[[:return, 42]]', %q{
def traced_method

15
vm_trace.c
View File

@ -87,9 +87,11 @@ update_global_event_hook(rb_event_flag_t prev_events, rb_event_flag_t new_events
{
rb_event_flag_t new_iseq_events = new_events & ISEQ_TRACE_EVENTS;
rb_event_flag_t enabled_iseq_events = ruby_vm_event_enabled_global_flags & ISEQ_TRACE_EVENTS;
bool trace_iseq_p = new_iseq_events & ~enabled_iseq_events;
bool first_time_iseq_events_p = new_iseq_events & ~enabled_iseq_events;
bool enable_c_call = (prev_events & RUBY_EVENT_C_CALL) == 0 && (new_events & RUBY_EVENT_C_CALL);
bool enable_c_return = (prev_events & RUBY_EVENT_C_RETURN) == 0 && (new_events & RUBY_EVENT_C_RETURN);
if (trace_iseq_p) {
if (first_time_iseq_events_p) {
// :class events are triggered only in ISEQ_TYPE_CLASS, but mjit_target_iseq_p ignores such iseqs.
// Thus we don't need to cancel JIT-ed code for :class events.
if (new_iseq_events != RUBY_EVENT_CLASS) {
@ -97,13 +99,12 @@ update_global_event_hook(rb_event_flag_t prev_events, rb_event_flag_t new_events
mjit_cancel_all("TracePoint is enabled");
}
/* write all ISeqs if and only if new events are added */
// write all ISeqs only when new events are added for the first time
rb_iseq_trace_set_all(new_iseq_events | enabled_iseq_events);
}
else {
// if c_call or c_return is activated:
if (((prev_events & RUBY_EVENT_C_CALL) == 0 && (new_events & RUBY_EVENT_C_CALL)) ||
((prev_events & RUBY_EVENT_C_RETURN) == 0 && (new_events & RUBY_EVENT_C_RETURN))) {
if (enable_c_call || enable_c_return) {
rb_clear_attr_ccs();
}
}
@ -112,8 +113,10 @@ update_global_event_hook(rb_event_flag_t prev_events, rb_event_flag_t new_events
ruby_vm_event_enabled_global_flags |= new_events;
rb_objspace_set_event_hook(new_events);
if (trace_iseq_p) {
if (first_time_iseq_events_p || enable_c_call || enable_c_return) {
// Invalidate all code when ISEQs are modified to use trace_* insns above.
// Also invalidate when enabling c_call or c_return because generated code
// never fires these events.
// Internal events fire inside C routines so don't need special handling.
// Do this after event flags updates so other ractors see updated vm events
// when they wake up.