Skip to content

Commit

Permalink
YJIT: Fix invalidation for c_call and c_return (#6719)
Browse files Browse the repository at this point in the history
Follow-up for 2b8191b. 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.
  • Loading branch information
XrXr committed Nov 13, 2022
1 parent a1b460b commit bc8ba24
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
20 changes: 20 additions & 0 deletions bootstraptest/test_yjit.rb
Expand Up @@ -2249,6 +2249,26 @@ def shouldnt_compile
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
Expand Down
15 changes: 9 additions & 6 deletions vm_trace.c
Expand Up @@ -87,23 +87,24 @@ 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) {
// Stop calling all JIT-ed code. We can't rewrite existing JIT-ed code to trace_ insns for now.
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();
}
}
Expand All @@ -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.
Expand Down

0 comments on commit bc8ba24

Please sign in to comment.