diff --git a/array.c b/array.c index b4718238763bab..4496dde2626500 100644 --- a/array.c +++ b/array.c @@ -8423,12 +8423,12 @@ rb_ary_deconstruct(VALUE ary) * * [1, 'one', :one, [2, 'two', :two]] * - * - A {%w or %W string-array Literal}[rdoc-ref:syntax/literals.rdoc@25w+and+-25W-3A+String-Array+Literals]: + * - A {%w or %W string-array Literal}[rdoc-ref:syntax/literals.rdoc@w-and-w-String-Array-Literals]: * * %w[foo bar baz] # => ["foo", "bar", "baz"] * %w[1 % *] # => ["1", "%", "*"] * - * - A {%i or %I symbol-array Literal}[rdoc-ref:syntax/literals.rdoc@25i+and+-25I-3A+Symbol-Array+Literals]: + * - A {%i or %I symbol-array Literal}[rdoc-ref:syntax/literals.rdoc@i+and-I-Symbol-Array+Literals]: * * %i[foo bar baz] # => [:foo, :bar, :baz] * %i[1 % *] # => [:"1", :%, :*] @@ -8690,8 +8690,8 @@ rb_ary_deconstruct(VALUE ary) * * First, what's elsewhere. Class \Array: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here], + * - Inherits from {class Object}[rdoc-ref:Object@Whats-Here]. + * - Includes {module Enumerable}[rdoc-ref:Enumerable@Whats-Here], * which provides dozens of additional methods. * * Here, class \Array provides methods that are useful for: diff --git a/complex.c b/complex.c index 85d724f273b3ea..1ba786a5bb8703 100644 --- a/complex.c +++ b/complex.c @@ -2645,9 +2645,9 @@ float_arg(VALUE self) * First, what's elsewhere: * * - Class \Complex inherits (directly or indirectly) - * from classes {Numeric}[rdoc-ref:Numeric@What-27s+Here] - * and {Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes (indirectly) module {Comparable}[rdoc-ref:Comparable@What-27s+Here]. + * from classes {Numeric}[rdoc-ref:Numeric@Whats-Here] + * and {Object}[rdoc-ref:Object@Whats-Here]. + * - Includes (indirectly) module {Comparable}[rdoc-ref:Comparable@Whats-Here]. * * Here, class \Complex has methods for: * diff --git a/dir.rb b/dir.rb index eb1a408ee3ac5b..9b83f688227d6c 100644 --- a/dir.rb +++ b/dir.rb @@ -31,7 +31,7 @@ # A \Dir object is in some ways array-like: # # - It has instance methods #children, #each, and #each_child. -# - It includes {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here]. +# - It includes {module Enumerable}[rdoc-ref:Enumerable@Whats+Here]. # # == \Dir As Stream-Like # @@ -85,8 +85,8 @@ # # First, what's elsewhere. Class \Dir: # -# - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. -# - Includes {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here], +# - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. +# - Includes {module Enumerable}[rdoc-ref:Enumerable@Whats+Here], # which provides dozens of additional methods. # # Here, class \Dir provides methods that are useful for: diff --git a/doc/float.rb b/doc/float.rb index 01668bfc6dacf1..f9068dfb1b82cc 100644 --- a/doc/float.rb +++ b/doc/float.rb @@ -72,9 +72,9 @@ # First, what's elsewhere. Class \Float: # # - Inherits from -# {class Numeric}[rdoc-ref:Numeric@What-27s+Here] -# and {class Object}[rdoc-ref:Object@What-27s+Here]. -# - Includes {module Comparable}[rdoc-ref:Comparable@What-27s+Here]. +# {class Numeric}[rdoc-ref:Numeric@Whats+Here] +# and {class Object}[rdoc-ref:Object@Whats+Here]. +# - Includes {module Comparable}[rdoc-ref:Comparable@Whats+Here]. # # Here, class \Float provides methods for: # diff --git a/doc/string.rb b/doc/string.rb index 304ab60c298967..e2dfb37c9fa5eb 100644 --- a/doc/string.rb +++ b/doc/string.rb @@ -163,8 +163,8 @@ # # First, what's elsewhere. Class +String+: # -# - Inherits from the {Object class}[rdoc-ref:Object@What-27s+Here]. -# - Includes the {Comparable module}[rdoc-ref:Comparable@What-27s+Here]. +# - Inherits from the {Object class}[rdoc-ref:Object@Whats+Here]. +# - Includes the {Comparable module}[rdoc-ref:Comparable@Whats+Here]. # # Here, class +String+ provides methods that are useful for: # diff --git a/eval.c b/eval.c index deadd5dd6414fb..fd370a43ccd2f2 100644 --- a/eval.c +++ b/eval.c @@ -329,17 +329,24 @@ ruby_exec_node(void *n) /* * call-seq: - * Module.nesting -> array - * - * Returns the list of +Modules+ nested at the point of call. + * Module.nesting -> array + * + * Returns nested module as an array of Module objects: + * + * module M0 + * def self.speak = Module.nesting + * module M1 + * def self.speak = Module.nesting + * module M2 + * def self.speak = Module.nesting + * end + * end + * end + * M0.speak # => [M0] + * M0.speak.first.class # => Module + * M0::M1.speak # => [M0::M1, M0] + * M0::M1::M2.speak # => [M0::M1::M2, M0::M1, M0] * - * module M1 - * module M2 - * $a = Module.nesting - * end - * end - * $a #=> [M1::M2, M1] - * $a[0].name #=> "M1::M2" */ static VALUE diff --git a/file.c b/file.c index 8f4e9d86c8241c..706b60c9997919 100644 --- a/file.c +++ b/file.c @@ -7507,7 +7507,7 @@ const char ruby_null_device[] = * * First, what's elsewhere. Class \File: * - * - Inherits from {class IO}[rdoc-ref:IO@What-27s+Here], + * - Inherits from {class IO}[rdoc-ref:IO@Whats+Here], * in particular, methods for creating, reading, and writing files * - Includes module FileTest, * which provides dozens of additional methods. diff --git a/hash.c b/hash.c index 07eeb779e9f197..e116eb8ab6f026 100644 --- a/hash.c +++ b/hash.c @@ -7216,8 +7216,8 @@ static const rb_data_type_t env_data_type = { * * First, what's elsewhere. Class +Hash+: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here], + * - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. + * - Includes {module Enumerable}[rdoc-ref:Enumerable@Whats+Here], * which provides dozens of additional methods. * * Here, class +Hash+ provides methods that are useful for: @@ -7528,8 +7528,8 @@ Init_Hash(void) * * First, what's elsewhere. Class +ENV+: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Extends {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here], + * - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. + * - Extends {module Enumerable}[rdoc-ref:Enumerable@Whats+Here], * * Here, class +ENV+ provides methods that are useful for: * diff --git a/io.c b/io.c index 25c66550f5c382..8563fa6536c02f 100644 --- a/io.c +++ b/io.c @@ -15469,8 +15469,8 @@ set_LAST_READ_LINE(VALUE val, ID _x, VALUE *_y) * * First, what's elsewhere. Class \IO: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here], + * - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. + * - Includes {module Enumerable}[rdoc-ref:Enumerable@Whats+Here], * which provides dozens of additional methods. * * Here, class \IO provides methods that are useful for: diff --git a/lib/net/http.rb b/lib/net/http.rb index 98d6793aee033d..98639978a2fbf6 100644 --- a/lib/net/http.rb +++ b/lib/net/http.rb @@ -460,7 +460,7 @@ class HTTPHeaderSyntaxError < StandardError; end # # First, what's elsewhere. Class Net::HTTP: # - # - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. + # - Inherits from {class Object}[rdoc-ref:Object#class-object-whats-here]. # # This is a categorized summary of methods and attributes. # @@ -1304,7 +1304,7 @@ def response_body_encoding=(value) # Sets whether to determine the proxy from environment variable # 'ENV['http_proxy']'; - # see {Proxy Using ENV['http_proxy']}[rdoc-ref:Net::HTTP@Proxy+Using+-27ENV-5B-27http_proxy-27-5D-27]. + # see {Proxy Using ENV['http_proxy']}[rdoc-ref:Net::HTTP@Proxy+Using+ENVHTTPProxy]. attr_writer :proxy_from_env # Sets the proxy address; diff --git a/lib/resolv.rb b/lib/resolv.rb index fa7d4e2e4753b3..b6ff3485182e5d 100644 --- a/lib/resolv.rb +++ b/lib/resolv.rb @@ -487,13 +487,18 @@ def each_name(address) # * Resolv::DNS::Resource::IN::A # * Resolv::DNS::Resource::IN::AAAA # * Resolv::DNS::Resource::IN::ANY + # * Resolv::DNS::Resource::IN::CAA # * Resolv::DNS::Resource::IN::CNAME # * Resolv::DNS::Resource::IN::HINFO + # * Resolv::DNS::Resource::IN::HTTPS + # * Resolv::DNS::Resource::IN::LOC # * Resolv::DNS::Resource::IN::MINFO # * Resolv::DNS::Resource::IN::MX # * Resolv::DNS::Resource::IN::NS # * Resolv::DNS::Resource::IN::PTR # * Resolv::DNS::Resource::IN::SOA + # * Resolv::DNS::Resource::IN::SRV + # * Resolv::DNS::Resource::IN::SVCB # * Resolv::DNS::Resource::IN::TXT # * Resolv::DNS::Resource::IN::WKS # diff --git a/numeric.c b/numeric.c index e8df2a6aa0568c..36101882943761 100644 --- a/numeric.c +++ b/numeric.c @@ -3680,9 +3680,9 @@ rb_int128_to_numeric(rb_int128_t n) * First, what's elsewhere. Class \Integer: * * - Inherits from - * {class Numeric}[rdoc-ref:Numeric@What-27s+Here] - * and {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Comparable}[rdoc-ref:Comparable@What-27s+Here]. + * {class Numeric}[rdoc-ref:Numeric@Whats+Here] + * and {class Object}[rdoc-ref:Object@Whats+Here]. + * - Includes {module Comparable}[rdoc-ref:Comparable@Whats+Here]. * * Here, class \Integer provides methods for: * @@ -6365,8 +6365,8 @@ int_s_try_convert(VALUE self, VALUE num) * * First, what's elsewhere. Class \Numeric: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Comparable}[rdoc-ref:Comparable@What-27s+Here]. + * - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. + * - Includes {module Comparable}[rdoc-ref:Comparable@Whats+Here]. * * Here, class \Numeric provides methods for: * diff --git a/object.c b/object.c index 75186a30c66868..07eb1d8e975251 100644 --- a/object.c +++ b/object.c @@ -4357,8 +4357,8 @@ rb_f_loop_size(VALUE self, VALUE args, VALUE eobj) * * First, what's elsewhere. Class \Object: * - * - Inherits from {class BasicObject}[rdoc-ref:BasicObject@What-27s+Here]. - * - Includes {module Kernel}[rdoc-ref:Kernel@What-27s+Here]. + * - Inherits from {class BasicObject}[rdoc-ref:BasicObject@Whats+Here]. + * - Includes {module Kernel}[rdoc-ref:Kernel@Whats+Here]. * * Here, class \Object provides methods for: * diff --git a/range.c b/range.c index fd08a81de7b8b1..36afdfa7619005 100644 --- a/range.c +++ b/range.c @@ -2768,8 +2768,8 @@ range_overlap(VALUE range, VALUE other) * * First, what's elsewhere. Class \Range: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here], + * - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. + * - Includes {module Enumerable}[rdoc-ref:Enumerable@Whats+Here], * which provides dozens of additional methods. * * Here, class \Range provides methods that are useful for: diff --git a/set.c b/set.c index 4d8178ffc080de..484439a40a6ab7 100644 --- a/set.c +++ b/set.c @@ -2051,8 +2051,8 @@ rb_set_size(VALUE set) * * First, what's elsewhere. \Class \Set: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here], + * - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. + * - Includes {module Enumerable}[rdoc-ref:Enumerable@Whats+Here], * which provides dozens of additional methods. * * In particular, class \Set does not have many methods of its own diff --git a/string.c b/string.c index 464eab21463ff3..a36eb6e9f381c0 100644 --- a/string.c +++ b/string.c @@ -12174,8 +12174,8 @@ rb_str_unicode_normalized_p(int argc, VALUE *argv, VALUE str) * * First, what's elsewhere. Class +Symbol+: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Comparable}[rdoc-ref:Comparable@What-27s+Here]. + * - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. + * - Includes {module Comparable}[rdoc-ref:Comparable@Whats+Here]. * * Here, class +Symbol+ provides methods that are useful for: * diff --git a/struct.c b/struct.c index 65410ebdf302b8..61aff40a32c081 100644 --- a/struct.c +++ b/struct.c @@ -2134,8 +2134,8 @@ rb_data_inspect(VALUE s) * * First, what's elsewhere. Class \Struct: * - * - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. - * - Includes {module Enumerable}[rdoc-ref:Enumerable@What-27s+Here], + * - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. + * - Includes {module Enumerable}[rdoc-ref:Enumerable@Whats+Here], * which provides dozens of additional methods. * * See also Data, which is a somewhat similar, but stricter concept for defining immutable diff --git a/test/ruby/test_thread.rb b/test/ruby/test_thread.rb index b2d8e73693807c..60e3aa772a8642 100644 --- a/test/ruby/test_thread.rb +++ b/test/ruby/test_thread.rb @@ -1664,4 +1664,37 @@ def test_mn_threads_sub_millisecond_sleep assert_operator elapsed, :>=, 0.1, "sub-millisecond sleeps should not return immediately" end; end + + # [Bug #21840] + def test_mutex_owner_doesnt_starve_waiters + assert_ruby_status([], "#{<<~"begin;"}\n#{<<~'end;'}") + begin; + m = Mutex.new + + fib = lambda { |n| + return n if n <= 1 + fib(n - 1) + fib(n - 2) + } + + t1_running = false + t1 = Thread.new do + t1_running = true + loop do + fib(20) + m.synchronize do + File.open(__FILE__) { } # reset timeslice due to blocking operation + end + end + end + + loop until t1_running + + 3.times.map do + Thread.new do + m.synchronize do + end + end + end.each(&:join) + end; + end end diff --git a/thread_sync.c b/thread_sync.c index e3916c97cbd0a6..2963b6db73b123 100644 --- a/thread_sync.c +++ b/thread_sync.c @@ -10,6 +10,8 @@ typedef struct rb_mutex_struct { rb_thread_t *th; // even if the fiber is collected, we might need access to the thread in mutex_free struct rb_mutex_struct *next_mutex; struct ccan_list_head waitq; /* protected by GVL */ + uint32_t saved_running_time_us; + bool wait_waking; // Is there a thread waiting to be woken up by this mutex? Reset during every wakeup. } rb_mutex_t; /* sync_waiter is always on-stack */ @@ -212,8 +214,15 @@ mutex_locked(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) static inline bool do_mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) { + // NOTE: we can successfully lock a mutex even if there are other threads waiting on it. First one to it wins. if (mutex->ec_serial == 0) { RUBY_DEBUG_LOG("%p ok", mutex); + if (mutex->wait_waking) { + // If we acquired `mutex` without contention and before the thread that was popped off the waitq, we're going + // to set our running_time back to what it was here during mutex unlock if it got reset during our critical + // section. This is to prevent starvation of other threads waiting on the mutex. + mutex->saved_running_time_us = th->running_time_us; + } mutex_locked(mutex, th, ec_serial); return true; @@ -350,7 +359,8 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) } ccan_list_del(&sync_waiter.node); - // unlocked by another thread while sleeping + // If mutex->ec_serial != 0, the mutex was locked by another thread before we had the chance to acquire it. + // We'll put ourselves on the waitq and sleep again. if (!mutex->ec_serial) { mutex_set_owner(mutex, th, ec_serial); } @@ -391,6 +401,7 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) if (saved_ints) th->ec->interrupt_flag = saved_ints; if (mutex->ec_serial == ec_serial) mutex_locked(mutex, th, ec_serial); + mutex->wait_waking = false; } RUBY_DEBUG_LOG("%p locked", mutex); @@ -454,6 +465,15 @@ rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) struct sync_waiter *cur = 0, *next; + + if (mutex->wait_waking) { + uint32_t saved = mutex->saved_running_time_us; + if (th->running_time_us < saved) { + th->running_time_us = saved; + } + } + + mutex->saved_running_time_us = 0; mutex->ec_serial = 0; thread_mutex_remove(th, mutex); @@ -469,6 +489,7 @@ rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) case THREAD_RUNNABLE: /* from someone else calling Thread#run */ case THREAD_STOPPED_FOREVER: /* likely (rb_mutex_lock) */ RUBY_DEBUG_LOG("wakeup th:%u", rb_th_serial(cur->th)); + mutex->wait_waking = true; rb_threadptr_interrupt(cur->th); return NULL; case THREAD_STOPPED: /* probably impossible */ @@ -480,7 +501,6 @@ rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) } } } - // We did not find any threads to wake up, so we can just return with no error: return NULL; } diff --git a/timev.rb b/timev.rb index cf8a88e64eff0c..005c3d481a0ebf 100644 --- a/timev.rb +++ b/timev.rb @@ -170,8 +170,8 @@ # # First, what's elsewhere. Class +Time+: # -# - Inherits from {class Object}[rdoc-ref:Object@What-27s+Here]. -# - Includes {module Comparable}[rdoc-ref:Comparable@What-27s+Here]. +# - Inherits from {class Object}[rdoc-ref:Object@Whats+Here]. +# - Includes {module Comparable}[rdoc-ref:Comparable@Whats+Here]. # # Here, class +Time+ provides methods that are useful for: # diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index d06e84536f6345..6ed855ddf9c688 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -390,7 +390,7 @@ impl Assembler { } let mut asm_local = Assembler::new_with_asm(&self); - let live_ranges: Vec = take(&mut self.live_ranges); + let live_ranges = take(&mut self.live_ranges); let mut iterator = self.instruction_iterator(); let asm = &mut asm_local; @@ -755,7 +755,7 @@ impl Assembler { asm_local.accept_scratch_reg = true; asm_local.stack_base_idx = self.stack_base_idx; asm_local.label_names = self.label_names.clone(); - asm_local.live_ranges.resize(self.live_ranges.len(), LiveRange { start: None, end: None }); + asm_local.live_ranges = LiveRanges::new(self.live_ranges.len()); // Create one giant block to linearize everything into asm_local.new_block_without_id(); @@ -1691,7 +1691,7 @@ impl Assembler { /// /// If a, b, and c are all registers. fn merge_three_reg_mov( - live_ranges: &[LiveRange], + live_ranges: &LiveRanges, iterator: &mut InsnIter, asm: &mut Assembler, left: &Opnd, diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index f2f7bc61659d04..f0fcece8a1a7d3 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -21,18 +21,33 @@ use crate::state::rb_zjit_record_exit_stack; #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] pub struct BlockId(pub usize); +#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] +pub struct VRegId(pub usize); + impl From for usize { fn from(val: BlockId) -> Self { val.0 } } +impl From for usize { + fn from(val: VRegId) -> Self { + val.0 + } +} + impl std::fmt::Display for BlockId { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!(f, "l{}", self.0) } } +impl std::fmt::Display for VRegId { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "v{}", self.0) + } +} + /// Dummy HIR block ID used when creating test or invalid LIR blocks const DUMMY_HIR_BLOCK_ID: usize = usize::MAX; /// Dummy RPO index used when creating test or invalid LIR blocks @@ -131,7 +146,7 @@ pub enum MemBase /// Register: Every Opnd::Mem should have MemBase::Reg as of emit. Reg(u8), /// Virtual register: Lowered to MemBase::Reg or MemBase::Stack in alloc_regs. - VReg(usize), + VReg(VRegId), /// Stack slot: Lowered to MemBase::Reg in scratch_split. Stack { stack_idx: usize, num_bits: u8 }, } @@ -158,7 +173,7 @@ impl fmt::Display for Mem { write!(f, "[")?; match self.base { MemBase::Reg(reg_no) => write!(f, "{}", mem_base_reg(reg_no))?, - MemBase::VReg(idx) => write!(f, "v{idx}")?, + MemBase::VReg(idx) => write!(f, "{idx}")?, MemBase::Stack { stack_idx, num_bits } if num_bits == 64 => write!(f, "Stack[{stack_idx}]")?, MemBase::Stack { stack_idx, num_bits } => write!(f, "Stack{num_bits}[{stack_idx}]")?, } @@ -196,7 +211,7 @@ pub enum Opnd Value(VALUE), /// Virtual register. Lowered to Reg or Mem in Assembler::alloc_regs(). - VReg{ idx: usize, num_bits: u8 }, + VReg{ idx: VRegId, num_bits: u8 }, // Low-level operands, for lowering Imm(i64), // Raw signed immediate @@ -212,8 +227,8 @@ impl fmt::Display for Opnd { None => write!(f, "None"), Value(VALUE(value)) if *value < 10 => write!(f, "Value({value:x})"), Value(VALUE(value)) => write!(f, "Value(0x{value:x})"), - VReg { idx, num_bits } if *num_bits == 64 => write!(f, "v{idx}"), - VReg { idx, num_bits } => write!(f, "VReg{num_bits}(v{idx})"), + VReg { idx, num_bits } if *num_bits == 64 => write!(f, "{idx}"), + VReg { idx, num_bits } => write!(f, "VReg{num_bits}({idx})"), Imm(value) if value.abs() < 10 => write!(f, "Imm({value:x})"), Imm(value) => write!(f, "Imm(0x{value:x})"), UImm(value) if *value < 10 => write!(f, "{value:x}"), @@ -282,7 +297,7 @@ impl Opnd } /// Unwrap the index of a VReg - pub fn vreg_idx(&self) -> usize { + pub fn vreg_idx(&self) -> VRegId { match self { Opnd::VReg { idx, .. } => *idx, _ => unreachable!("trying to unwrap {self:?} into VReg"), @@ -321,10 +336,10 @@ impl Opnd pub fn map_index(self, indices: &[usize]) -> Opnd { match self { Opnd::VReg { idx, num_bits } => { - Opnd::VReg { idx: indices[idx], num_bits } + Opnd::VReg { idx: VRegId(indices[idx.0]), num_bits } } Opnd::Mem(Mem { base: MemBase::VReg(idx), disp, num_bits }) => { - Opnd::Mem(Mem { base: MemBase::VReg(indices[idx]), disp, num_bits }) + Opnd::Mem(Mem { base: MemBase::VReg(VRegId(indices[idx.0])), disp, num_bits }) }, _ => self } @@ -1355,12 +1370,44 @@ impl LiveRange { } } +/// Type-safe wrapper around `Vec` that can be indexed by VRegId +#[derive(Clone, Debug, Default)] +pub struct LiveRanges(Vec); + +impl LiveRanges { + pub fn new(size: usize) -> Self { + Self(vec![LiveRange { start: None, end: None }; size]) + } + + pub fn len(&self) -> usize { + self.0.len() + } + + pub fn get(&self, vreg_id: VRegId) -> Option<&LiveRange> { + self.0.get(vreg_id.0) + } +} + +impl std::ops::Index for LiveRanges { + type Output = LiveRange; + + fn index(&self, idx: VRegId) -> &Self::Output { + &self.0[idx.0] + } +} + +impl std::ops::IndexMut for LiveRanges { + fn index_mut(&mut self, idx: VRegId) -> &mut Self::Output { + &mut self.0[idx.0] + } +} + /// StackState manages which stack slots are used by which VReg pub struct StackState { /// The maximum number of spilled VRegs at a time stack_size: usize, /// Map from index at the C stack for spilled VRegs to Some(vreg_idx) if allocated - stack_slots: Vec>, + stack_slots: Vec>, /// Copy of Assembler::stack_base_idx. Used for calculating stack slot offsets. stack_base_idx: usize, } @@ -1376,7 +1423,7 @@ impl StackState { } /// Allocate a stack slot for a given vreg_idx - fn alloc_stack(&mut self, vreg_idx: usize) -> Opnd { + fn alloc_stack(&mut self, vreg_idx: VRegId) -> Opnd { for stack_idx in 0..self.stack_size { if self.stack_slots[stack_idx].is_none() { self.stack_slots[stack_idx] = Some(vreg_idx); @@ -1437,7 +1484,7 @@ struct RegisterPool { /// Some(vreg_idx) if the register at the index in `pool` is used by the VReg. /// None if the register is not in use. - pool: Vec>, + pool: Vec>, /// The number of live registers. /// Provides a quick way to query `pool.filter(|r| r.is_some()).count()` @@ -1461,7 +1508,7 @@ impl RegisterPool { /// Mutate the pool to indicate that the register at the index /// has been allocated and is live. - fn alloc_opnd(&mut self, vreg_idx: usize) -> Opnd { + fn alloc_opnd(&mut self, vreg_idx: VRegId) -> Opnd { for (reg_idx, reg) in self.regs.iter().enumerate() { if self.pool[reg_idx].is_none() { self.pool[reg_idx] = Some(vreg_idx); @@ -1473,7 +1520,7 @@ impl RegisterPool { } /// Allocate a specific register - fn take_reg(&mut self, reg: &Reg, vreg_idx: usize) -> Opnd { + fn take_reg(&mut self, reg: &Reg, vreg_idx: VRegId) -> Opnd { let reg_idx = self.regs.iter().position(|elem| elem.reg_no == reg.reg_no) .unwrap_or_else(|| panic!("Unable to find register: {}", reg.reg_no)); assert_eq!(self.pool[reg_idx], None, "register already allocated for VReg({:?})", self.pool[reg_idx]); @@ -1499,7 +1546,7 @@ impl RegisterPool { } /// Return a list of (Reg, vreg_idx) tuples for all live registers - fn live_regs(&self) -> Vec<(Reg, usize)> { + fn live_regs(&self) -> Vec<(Reg, VRegId)> { let mut live_regs = Vec::with_capacity(self.live_regs); for (reg_idx, ®) in self.regs.iter().enumerate() { if let Some(vreg_idx) = self.pool[reg_idx] { @@ -1510,7 +1557,7 @@ impl RegisterPool { } /// Return vreg_idx if a given register is already in use - fn vreg_for(&self, reg: &Reg) -> Option { + fn vreg_for(&self, reg: &Reg) -> Option { let reg_idx = self.regs.iter().position(|elem| elem.reg_no == reg.reg_no).unwrap(); self.pool[reg_idx] } @@ -1536,7 +1583,7 @@ pub struct Assembler { current_block_id: BlockId, /// Live range for each VReg indexed by its `idx`` - pub(super) live_ranges: Vec, + pub(super) live_ranges: LiveRanges, /// Names of labels pub(super) label_names: Vec, @@ -1568,7 +1615,7 @@ impl Assembler leaf_ccall_stack_size: None, basic_blocks: Vec::default(), current_block_id: BlockId(0), - live_ranges: Vec::default(), + live_ranges: LiveRanges::default(), idx: 0, } } @@ -1602,7 +1649,7 @@ impl Assembler // Initialize live_ranges to match the old assembler's size // This allows reusing VRegs from the old assembler - asm.live_ranges.resize(old_asm.live_ranges.len(), LiveRange { start: None, end: None }); + asm.live_ranges = LiveRanges::new(old_asm.live_ranges.len()); asm } @@ -1780,8 +1827,8 @@ impl Assembler /// Build an Opnd::VReg and initialize its LiveRange pub(super) fn new_vreg(&mut self, num_bits: u8) -> Opnd { - let vreg = Opnd::VReg { idx: self.live_ranges.len(), num_bits }; - self.live_ranges.push(LiveRange { start: None, end: None }); + let vreg = Opnd::VReg { idx: VRegId(self.live_ranges.len()), num_bits }; + self.live_ranges.0.push(LiveRange { start: None, end: None }); vreg } @@ -1794,7 +1841,7 @@ impl Assembler // Initialize the live range of the output VReg to insn_idx..=insn_idx if let Some(Opnd::VReg { idx, .. }) = insn.out_opnd() { - assert!(*idx < self.live_ranges.len()); + assert!(idx.0 < self.live_ranges.len()); assert_eq!(self.live_ranges[*idx], LiveRange { start: None, end: None }); self.live_ranges[*idx] = LiveRange { start: Some(insn_idx), end: Some(insn_idx) }; } @@ -1805,7 +1852,7 @@ impl Assembler match *opnd { Opnd::VReg { idx, .. } | Opnd::Mem(Mem { base: MemBase::VReg(idx), .. }) => { - assert!(idx < self.live_ranges.len()); + assert!(idx.0 < self.live_ranges.len()); assert_ne!(self.live_ranges[idx].end, None); self.live_ranges[idx].end = Some(self.live_ranges[idx].end().max(insn_idx)); } @@ -1894,7 +1941,7 @@ impl Assembler let mut vreg_opnd: Vec> = vec![None; self.live_ranges.len()]; // List of registers saved before a C call, paired with the VReg index. - let mut saved_regs: Vec<(Reg, usize)> = vec![]; + let mut saved_regs: Vec<(Reg, VRegId)> = vec![]; // Remember the indexes of Insn::FrameSetup to update the stack size later let mut frame_setup_idxs: Vec<(BlockId, usize)> = vec![]; @@ -1906,7 +1953,7 @@ impl Assembler let asm = &mut asm_local; - let live_ranges: Vec = take(&mut self.live_ranges); + let live_ranges = take(&mut self.live_ranges); while let Some((index, mut insn)) = iterator.next(asm) { // Remember the index of FrameSetup to bump slot_count when we know the max number of spilled VRegs. @@ -1924,7 +1971,7 @@ impl Assembler let new_opnd = pool.alloc_opnd(vreg_idx); asm.mov(new_opnd, C_RET_OPND); pool.dealloc_opnd(&Opnd::Reg(C_RET_REG)); - vreg_opnd[vreg_idx] = Some(new_opnd); + vreg_opnd[vreg_idx.0] = Some(new_opnd); } true @@ -1943,7 +1990,7 @@ impl Assembler // uses this operand. If it is, we can return the allocated // register to the pool. if live_ranges[idx].end() == index { - if let Some(opnd) = vreg_opnd[idx] { + if let Some(opnd) = vreg_opnd[idx.0] { pool.dealloc_opnd(&opnd); } else { unreachable!("no register allocated for insn {:?}", insn); @@ -1987,7 +2034,7 @@ impl Assembler }; if let Some(vreg_idx) = vreg_idx { if live_ranges[vreg_idx].end() == index { - debug!("Allocating a register for VReg({}) at instruction index {} even though it does not live past this index", vreg_idx, index); + debug!("Allocating a register for {vreg_idx} at instruction index {index} even though it does not live past this index"); } // This is going to be the output operand that we will set on the // instruction. CCall and LiveReg need to use a specific register. @@ -2012,7 +2059,7 @@ impl Assembler if let Some(Opnd::VReg{ idx, .. }) = opnd_iter.next() { if live_ranges[*idx].end() == index { - if let Some(Opnd::Reg(reg)) = vreg_opnd[*idx] { + if let Some(Opnd::Reg(reg)) = vreg_opnd[idx.0] { out_reg = Some(pool.take_reg(®, vreg_idx)); } } @@ -2031,7 +2078,7 @@ impl Assembler // extends beyond the index of the instruction. let out = insn.out_opnd_mut().unwrap(); let out_opnd = out_opnd.with_num_bits(out_num_bits); - vreg_opnd[out.vreg_idx()] = Some(out_opnd); + vreg_opnd[out.vreg_idx().0] = Some(out_opnd); *out = out_opnd; } @@ -2040,10 +2087,10 @@ impl Assembler while let Some(opnd) = opnd_iter.next() { match *opnd { Opnd::VReg { idx, num_bits } => { - *opnd = vreg_opnd[idx].unwrap().with_num_bits(num_bits); + *opnd = vreg_opnd[idx.0].unwrap().with_num_bits(num_bits); }, Opnd::Mem(Mem { base: MemBase::VReg(idx), disp, num_bits }) => { - *opnd = match vreg_opnd[idx].unwrap() { + *opnd = match vreg_opnd[idx.0].unwrap() { Opnd::Reg(reg) => Opnd::Mem(Mem { base: MemBase::Reg(reg.reg_no), disp, num_bits }), // If the base is spilled, lower it to MemBase::Stack, which scratch_split will lower to MemBase::Reg. Opnd::Mem(mem) => Opnd::Mem(Mem { base: pool.stack_state.mem_to_stack_membase(mem), disp, num_bits }), @@ -2058,7 +2105,7 @@ impl Assembler // register if let Some(idx) = vreg_idx { if live_ranges[idx].end() == index { - if let Some(opnd) = vreg_opnd[idx] { + if let Some(opnd) = vreg_opnd[idx.0] { pool.dealloc_opnd(&opnd); } else { unreachable!("no register allocated for insn {:?}", insn); @@ -2849,7 +2896,7 @@ impl Assembler { asm_local.accept_scratch_reg = self.accept_scratch_reg; asm_local.stack_base_idx = self.stack_base_idx; asm_local.label_names = self.label_names.clone(); - asm_local.live_ranges.resize(self.live_ranges.len(), LiveRange { start: None, end: None }); + asm_local.live_ranges = LiveRanges::new(self.live_ranges.len()); // Create one giant block to linearize everything into asm_local.new_block_without_id(); diff --git a/zjit/src/backend/tests.rs b/zjit/src/backend/tests.rs index 701029b8ec0c2c..32b6fe9b5ef31e 100644 --- a/zjit/src/backend/tests.rs +++ b/zjit/src/backend/tests.rs @@ -3,7 +3,6 @@ use crate::backend::lir::*; use crate::cruby::*; use crate::codegen::c_callable; use crate::options::rb_zjit_prepare_options; -use crate::hir; #[test] fn test_add() { diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index a4cf8dfcc5e892..b045e0f3a3d04b 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -140,7 +140,7 @@ impl Assembler { { let mut asm_local = Assembler::new_with_asm(&self); let asm = &mut asm_local; - let live_ranges: Vec = take(&mut self.live_ranges); + let live_ranges = take(&mut self.live_ranges); let mut iterator = self.instruction_iterator(); while let Some((index, mut insn)) = iterator.next(asm) { @@ -166,7 +166,7 @@ impl Assembler { // When we split an operand, we can create a new VReg not in `live_ranges`. // So when we see a VReg with out-of-range index, it's created from splitting // from the loop above and we know it doesn't outlive the current instruction. - let vreg_outlives_insn = |vreg_idx| { + let vreg_outlives_insn = |vreg_idx: VRegId| { live_ranges .get(vreg_idx) .is_some_and(|live_range: &LiveRange| live_range.end() > index) @@ -472,7 +472,7 @@ impl Assembler { asm_local.accept_scratch_reg = true; asm_local.stack_base_idx = self.stack_base_idx; asm_local.label_names = self.label_names.clone(); - asm_local.live_ranges.resize(self.live_ranges.len(), LiveRange { start: None, end: None }); + asm_local.live_ranges = LiveRanges::new(self.live_ranges.len()); // Create one giant block to linearize everything into asm_local.new_block_without_id(); diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 2aa74dce8be26c..79004c8737e4ee 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -5125,6 +5125,34 @@ impl Function { Ok(()) } + // Validate that every instruction use is from a block-local definition, which is a temporary + // constraint until we get a global register allocator. + // TODO(tenderworks): Remove this + fn temporary_validate_block_local_definite_assignment(&self) -> Result<(), ValidationError> { + for block in self.rpo() { + let mut assigned = InsnSet::with_capacity(self.insns.len()); + for ¶m in &self.blocks[block.0].params { + assigned.insert(param); + } + // Check that each instruction's operands are assigned + for &insn_id in &self.blocks[block.0].insns { + let insn_id = self.union_find.borrow().find_const(insn_id); + let mut operands = VecDeque::new(); + let insn = self.find(insn_id); + self.worklist_traverse_single_insn(&insn, &mut operands); + for operand in operands { + if !assigned.get(operand) { + return Err(ValidationError::OperandNotDefined(block, insn_id, operand)); + } + } + if insn.has_output() { + assigned.insert(insn_id); + } + } + } + Ok(()) + } + /// Checks that each instruction('s representative) appears only once in the CFG. fn validate_insn_uniqueness(&self) -> Result<(), ValidationError> { let mut seen = InsnSet::with_capacity(self.insns.len()); @@ -5425,6 +5453,7 @@ impl Function { pub fn validate(&self) -> Result<(), ValidationError> { self.validate_block_terminators_and_jumps()?; self.validate_definite_assignment()?; + self.temporary_validate_block_local_definite_assignment()?; self.validate_insn_uniqueness()?; self.validate_types()?; Ok(()) @@ -7577,6 +7606,16 @@ mod validation_tests { assert_matches_err(function.validate_definite_assignment(), ValidationError::OperandNotDefined(entry, val, dangling)); } + #[test] + fn not_defined_within_bb_block_local() { + let mut function = Function::new(std::ptr::null()); + let entry = function.entry_block; + // Create an instruction without making it belong to anything. + let dangling = function.new_insn(Insn::Const{val: Const::CBool(true)}); + let val = function.push_insn(function.entry_block, Insn::ArrayDup { val: dangling, state: InsnId(0usize) }); + assert_matches_err(function.temporary_validate_block_local_definite_assignment(), ValidationError::OperandNotDefined(entry, val, dangling)); + } + #[test] fn using_non_output_insn() { let mut function = Function::new(std::ptr::null()); @@ -7588,6 +7627,17 @@ mod validation_tests { assert_matches_err(function.validate_definite_assignment(), ValidationError::OperandNotDefined(entry, val, ret)); } + #[test] + fn using_non_output_insn_block_local() { + let mut function = Function::new(std::ptr::null()); + let entry = function.entry_block; + let const_ = function.push_insn(function.entry_block, Insn::Const{val: Const::CBool(true)}); + // Ret is a non-output instruction. + let ret = function.push_insn(function.entry_block, Insn::Return { val: const_ }); + let val = function.push_insn(function.entry_block, Insn::ArrayDup { val: ret, state: InsnId(0usize) }); + assert_matches_err(function.temporary_validate_block_local_definite_assignment(), ValidationError::OperandNotDefined(entry, val, ret)); + } + #[test] fn not_dominated_by_diamond() { // This tests that one branch is missing a definition which fails.