diff --git a/compile.c b/compile.c index 472804dfb7508e..d719764b150b9d 100644 --- a/compile.c +++ b/compile.c @@ -9589,6 +9589,14 @@ compile_builtin_function_call(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NOD ADD_INSN1(ret, line_node, putobject, Qfalse); return compile_builtin_mandatory_only_method(iseq, node, line_node); } + else if (strcmp("local_self!", builtin_func) == 0) { + // Push the local named "self" (e.g. the `self:` keyword + // parameter of Ractor.shareable_proc) onto the stack. + ID id_self; + CONST_ID(id_self, "self"); + compile_lvar(iseq, ret, line_node, id_self); + return COMPILE_OK; + } else if (1) { rb_bug("can't find builtin function:%s", builtin_func); } diff --git a/ext/strscan/strscan.c b/ext/strscan/strscan.c index 5f5ad1b9bff231..c6271bce517c30 100644 --- a/ext/strscan/strscan.c +++ b/ext/strscan/strscan.c @@ -617,6 +617,17 @@ match_target(struct strscanner *p) } } +static inline bool +curr_char_head_p(struct strscanner *p) +{ + const char *pbeg = S_PBEG(p); + const char *curr = CURPTR(p); + + if (curr == pbeg) return true; + return rb_enc_left_char_head(pbeg, curr, S_PEND(p), + rb_enc_get(p->str)) == curr; +} + static inline void set_registers(struct strscanner *p, size_t pos, size_t length) { @@ -753,6 +764,10 @@ strscan_do_scan(VALUE self, VALUE pattern, int succptr, int getstr, int headonly return Qnil; } + if (!curr_char_head_p(p)) { + return Qnil; + } + if (RB_TYPE_P(pattern, T_REGEXP)) { OnigPosition ret; RB_OBJ_WRITE(self, &p->regex, pattern); diff --git a/lib/prism/parse_result/newlines.rb b/lib/prism/parse_result/newlines.rb index ad8d8b6f55afc9..41729bf81344ef 100644 --- a/lib/prism/parse_result/newlines.rb +++ b/lib/prism/parse_result/newlines.rb @@ -65,6 +65,117 @@ def visit_lambda_node(node) end end + # Permit def nodes to mark newlines within themselves. The body of an + # endless method definition never emits newline events, so in that case + # mark every line as already seen while visiting it instead. Nested + # scopes (blocks, lambdas, etc.) reset the lines and emit events again. + # + #: (DefNode node) -> void + def visit_def_node(node) + old_lines = @lines + @lines = Array.new(old_lines.size, !node.equal_loc.nil?) + + begin + super(node) + ensure + @lines = old_lines + end + end + + # Permit class nodes to mark newlines within themselves. + # + #: (ClassNode node) -> void + def visit_class_node(node) + old_lines = @lines + @lines = Array.new(old_lines.size, false) + + begin + super(node) + ensure + @lines = old_lines + end + end + + # Permit module nodes to mark newlines within themselves. + # + #: (ModuleNode node) -> void + def visit_module_node(node) + old_lines = @lines + @lines = Array.new(old_lines.size, false) + + begin + super(node) + ensure + @lines = old_lines + end + end + + # Permit singleton class nodes to mark newlines within themselves. + # + #: (SingletonClassNode node) -> void + def visit_singleton_class_node(node) + old_lines = @lines + @lines = Array.new(old_lines.size, false) + + begin + super(node) + ensure + @lines = old_lines + end + end + + # Statements inside string interpolation do not emit newline events, so + # mark every line as already seen while visiting them. Nested scopes + # (blocks, lambdas, defs, etc.) reset the lines and emit events again. + # + #: (EmbeddedStatementsNode node) -> void + def visit_embedded_statements_node(node) + old_lines = @lines + @lines = Array.new(old_lines.size, true) + + begin + super(node) + ensure + @lines = old_lines + end + end + + # The predicate of a while loop is compiled at the end of the loop, + # after the body, so any statements it contains (from parentheses) + # emit their line events again even if the lines were already seen. + # + #: (WhileNode node) -> void + def visit_while_node(node) + old_lines = @lines + @lines = Array.new(old_lines.size, false) + + begin + visit(node.predicate) + ensure + @lines = old_lines + end + + visit(node.statements) + end + + # The predicate of an until loop is compiled at the end of the loop, + # after the body, so any statements it contains (from parentheses) + # emit their line events again even if the lines were already seen. + # + #: (UntilNode node) -> void + def visit_until_node(node) + old_lines = @lines + @lines = Array.new(old_lines.size, false) + + begin + visit(node.predicate) + ensure + @lines = old_lines + end + + visit(node.statements) + end + # Mark if nodes as newlines. # #: (IfNode node) -> void @@ -144,14 +255,26 @@ def newline_flag!(lines) # :nodoc: class UntilNode < Node #: (Array[bool] lines) -> void def newline_flag!(lines) # :nodoc: - predicate.newline_flag!(lines) + if location.start_offset == keyword_loc.start_offset && predicate.is_a?(ParenthesesNode) + # A parenthesized predicate emits its own line event when it is + # compiled at the end of the loop, in addition to this one. + super + else + predicate.newline_flag!(lines) + end end end class WhileNode < Node #: (Array[bool] lines) -> void def newline_flag!(lines) # :nodoc: - predicate.newline_flag!(lines) + if location.start_offset == keyword_loc.start_offset && predicate.is_a?(ParenthesesNode) + # A parenthesized predicate emits its own line event when it is + # compiled at the end of the loop, in addition to this one. + super + else + predicate.newline_flag!(lines) + end end end @@ -162,6 +285,98 @@ def newline_flag!(lines) # :nodoc: end end + # The line event for a statement is emitted where its first instruction is + # compiled, so nodes whose first instruction comes from a sub-expression + # delegate their newline flag to that sub-expression: assignments to their + # value, calls to their receiver, and array, hash, and interpolated string + # literals to their first element. Static literals are the exception: they + # are compiled to a single instruction on the first line of the literal, so + # they do not delegate. + + class LocalVariableWriteNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + value.newline_flag!(lines) + end + end + + class InstanceVariableWriteNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + value.newline_flag!(lines) + end + end + + class ClassVariableWriteNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + value.newline_flag!(lines) + end + end + + class GlobalVariableWriteNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + value.newline_flag!(lines) + end + end + + class ConstantWriteNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + value.newline_flag!(lines) + end + end + + class ConstantPathWriteNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + value.newline_flag!(lines) + end + end + + class MultiWriteNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + value.newline_flag!(lines) + end + end + + class CallNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + if (receiver = self.receiver) + receiver.newline_flag!(lines) + else + super + end + end + end + + class ArrayNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + first = elements.first + if first && !static_literal? + first.newline_flag!(lines) + else + super + end + end + end + + class HashNode < Node + #: (Array[bool] lines) -> void + def newline_flag!(lines) # :nodoc: + first = elements.first + if first && !static_literal? + first.newline_flag!(lines) + else + super + end + end + end + class InterpolatedMatchLastLineNode < Node #: (Array[bool] lines) -> void def newline_flag!(lines) # :nodoc: @@ -182,7 +397,11 @@ class InterpolatedStringNode < Node #: (Array[bool] lines) -> void def newline_flag!(lines) # :nodoc: first = parts.first - first.newline_flag!(lines) if first + if first && !static_literal? + first.newline_flag!(lines) + else + super + end end end diff --git a/prism_compile.c b/prism_compile.c index fef1f805c61b8f..db0eb3fab2225a 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -3694,6 +3694,18 @@ retry:; PUSH_INSN1(ret, *node_location, putobject, Qfalse); return pm_compile_builtin_mandatory_only_method(iseq, scope_node, call_node, node_location); } + else if (strcmp("local_self!", builtin_func) == 0) { + // Push the local named "self" (e.g. the `self:` keyword parameter + // of Ractor.shareable_proc) onto the stack. + pm_constant_id_t self_id = pm_parser_constant_find(scope_node->parser, (const uint8_t *) "self", 4); + if (self_id == 0) { + COMPILE_ERROR(iseq, node_location->line, "local_self! called but 'self' not found in local table"); + return COMPILE_NG; + } + pm_local_index_t self_index = pm_lookup_local_index(iseq, scope_node, self_id, /*start_depth=*/0); + PUSH_GETLOCAL(ret, *node_location, self_index.index, self_index.level); + return COMPILE_OK; + } else if (1) { rb_bug("can't find builtin function:%s", builtin_func); } diff --git a/ractor.c b/ractor.c index 857a5fe1f8cf14..192da2ea8b58f5 100644 --- a/ractor.c +++ b/ractor.c @@ -4227,4 +4227,16 @@ rb_ractor_autoload_load(VALUE module, ID name) } } +VALUE +rb_builtin_shareable_proc(rb_execution_context_t *ec, VALUE self, VALUE arg_self) +{ + return ractor_shareable_proc(ec, arg_self, false); +} + +VALUE +rb_builtin_shareable_lambda(rb_execution_context_t *ec, VALUE self, VALUE arg_self) +{ + return ractor_shareable_proc(ec, arg_self, true); +} + #include "ractor.rbinc" diff --git a/ractor.rb b/ractor.rb index ee3a496ae8f222..e5ca3a480d0d87 100644 --- a/ractor.rb +++ b/ractor.rb @@ -706,9 +706,7 @@ def unmonitor port def self.shareable_proc self: nil Primitive.attr! :use_block - __builtin_cexpr!(%Q{ - ractor_shareable_proc(ec, *LOCAL_PTR(self), false) - }) + Primitive.rb_builtin_shareable_proc(Primitive.local_self!) end # @@ -720,9 +718,7 @@ def self.shareable_proc self: nil def self.shareable_lambda self: nil Primitive.attr! :use_block - __builtin_cexpr!(%Q{ - ractor_shareable_proc(ec, *LOCAL_PTR(self), true) - }) + Primitive.rb_builtin_shareable_lambda(Primitive.local_self!) end # \Port objects transmit messages between Ractors. diff --git a/test/prism/newline_test.rb b/test/prism/newline_test.rb index 0accd8af6d81d2..12504e912dd2be 100644 --- a/test/prism/newline_test.rb +++ b/test/prism/newline_test.rb @@ -2,32 +2,16 @@ require_relative "test_helper" -return unless defined?(RubyVM::InstructionSequence) +# There have also been changes made in other versions of Ruby, so we only want +# to test on the most recent versions. +return if !defined?(RubyVM::InstructionSequence) || RUBY_VERSION < "3.4.0" module Prism class NewlineTest < TestCase - skips = %w[ - errors_test.rb - locals_test.rb - regexp_test.rb - test_helper.rb - unescape_test.rb - api/parse_stream_test.rb - api/raise_error_test.rb - encoding/regular_expression_encoding_test.rb - encoding/string_encoding_test.rb - result/breadth_first_search_test.rb - result/static_literals_test.rb - result/warnings_test.rb - ruby/find_fixtures.rb - ruby/find_test.rb - ruby/parser_test.rb - ruby/ripper_test.rb - ruby/ruby_parser_test.rb - ] - + # If you are coming from ruby/ruby, a test failure here means that TracePoint `:line` events changed. + # Before adding a skip, make sure that you actually intended for such a difference to happen. base = __dir__ - (Dir["{,api/,encoding/,result/,ruby/}*.rb", base: base] - skips).each do |relative| + Dir["{,api/,encoding/,result/,ruby/}*.rb", base: base].each do |relative| define_method(:"test_#{relative}") do assert_newlines(base, relative) end @@ -44,33 +28,6 @@ def assert_newlines(base, relative) assert_empty result.errors actual = prism_lines(result) - source.each_line.with_index(1) do |line, line_number| - # Lines like `while (foo = bar)` result in two line flags in the - # bytecode but only one newline flag in the AST. We need to remove the - # extra line flag from the bytecode to make the test pass. - if line.match?(/while \(/) - index = expected.index(line_number) - expected.delete_at(index) if index - end - - # Lines like `foo =` where the value is on the next line result in - # another line flag in the bytecode but only one newline flag in the - # AST. - if line.match?(/^\s+\w+ =$/) - if source.lines[line_number].match?(/^\s+case/) - actual[actual.index(line_number)] += 1 - else - actual.delete_at(actual.index(line_number)) - end - end - - if line.match?(/^\s+\w+ = \[$/) - if !expected.include?(line_number) && !expected.include?(line_number + 2) - actual[actual.index(line_number)] += 1 - end - end - end - assert_equal expected, actual end diff --git a/test/strscan/test_stringscanner.rb b/test/strscan/test_stringscanner.rb index 8bf24c73012202..8530be3893ba7a 100644 --- a/test/strscan/test_stringscanner.rb +++ b/test/strscan/test_stringscanner.rb @@ -302,6 +302,24 @@ def test_scan assert_equal("", s.scan(//)) end + def test_scan_at_non_character_boundary + omit("not supported on TruffleRuby") if RUBY_ENGINE == "truffleruby" + + dot = Regexp.new(".".encode(Encoding::UTF_16BE)) + empty = Regexp.new("".encode(Encoding::UTF_16BE)) + b = "b".encode(Encoding::UTF_16BE) + scanner = create_string_scanner("ab".encode(Encoding::UTF_16BE)) + + scanner.pos = 1 # in the middle of "a" + assert_nil(scanner.scan(dot)) + assert_nil(scanner.scan(empty)) + assert_nil(scanner.scan(b)) + + scanner.pos = 2 # on a character boundary + assert_equal("", scanner.scan(empty).encode(Encoding::UTF_8)) + assert_equal("b", scanner.scan(b).encode(Encoding::UTF_8)) + end + def test_scan_string s = create_string_scanner("stra strb\0strc") assert_equal('str', s.scan('str')) diff --git a/tool/mk_builtin_loader.rb b/tool/mk_builtin_loader.rb index 87335015dd942e..64029aae3ba8d4 100644 --- a/tool/mk_builtin_loader.rb +++ b/tool/mk_builtin_loader.rb @@ -104,6 +104,10 @@ def visit_call_node(source, node, name, locals, requires, bs, inlines) raise "attr (#{arg["unescaped"]}) was not in: leaf, inline_block, use_block" unless BUILTIN_ATTRS.include?(arg["unescaped"]) end + return true + when "local_self" + raise "`local_self` args must be empty" if argc != 0 + raise "`local_self` requires `self` local to be present" unless locals.include?(:self) return true when "mandatory_only" # This is a call to Primitive.mandatory_only?. This method does not @@ -261,10 +265,7 @@ def generate_cexpr(ofile, lineno, line_file, body_lineno, text, locals, func_nam # Avoid generating fetches of lvars we don't need. This is imperfect as it # will match text inside strings or other false positives. - local_ptrs = [] - local_candidates = text.gsub(/\bLOCAL_PTR\(\K[a-zA-Z_][a-zA-Z0-9_]*(?=\))/) { - local_ptrs << $&; '' - }.scan(/[a-zA-Z_][a-zA-Z0-9_]*/) + local_candidates = text.scan(/[a-zA-Z_][a-zA-Z0-9_]*/) f.puts '{' lineno += 1 @@ -272,11 +273,9 @@ def generate_cexpr(ofile, lineno, line_file, body_lineno, text, locals, func_nam locals&.reverse_each&.with_index{|param, i| next unless Symbol === param param = param.to_s - lvar = local_candidates.include?(param) - next unless lvar or local_ptrs.include?(param) - f.puts "VALUE *const #{param}__ptr = (VALUE *)&ec->cfp->ep[#{-3 - i}];" - f.puts "MAYBE_UNUSED(const VALUE) #{param} = *#{param}__ptr;" if lvar - lineno += lvar ? 2 : 1 + next unless local_candidates.include?(param) + f.puts "MAYBE_UNUSED(const VALUE) #{param} = ec->cfp->ep[#{-3 - i}];" + lineno += 1 } f.puts "#line #{body_lineno} \"#{line_file}\"" lineno += 1 diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index be4117def694d2..f9eef057ca8bf6 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -194,6 +194,7 @@ fn main() { .allowlist_var("rb_cNumeric") .allowlist_var("rb_cRange") .allowlist_var("rb_cString") + .allowlist_var("rb_cProc") .allowlist_var("rb_cThread") .allowlist_var("rb_cArray") .allowlist_var("rb_cHash") diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index ebf983315c3200..c7ca0f826675d6 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -617,6 +617,13 @@ impl From for Opnd { } } +/// `hir::BlockHandler` lowered for codegen: a block ISEQ (encoded as a specval +/// at frame push) or an already-guarded Proc VALUE. +pub enum BlockHandler { + Iseq(IseqPtr), + Proc(Opnd), +} + /// Context for a side exit. If `SideExit` matches, it reuses the same code. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct SideExit { diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 377564566afcf0..54d64a4ea4984a 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -686,13 +686,26 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio &Insn::Send { cd, block: None, state, reason, .. } => gen_send_without_block(jit, asm, function, cd, &function.frame_state(state), reason), &Insn::Send { cd, block: Some(BlockHandler::BlockIseq(blockiseq)), state, reason, .. } => gen_send(jit, asm, function, cd, blockiseq, &function.frame_state(state), reason), &Insn::Send { cd, block: Some(BlockHandler::BlockArg), state, reason, .. } => gen_send(jit, asm, function, cd, std::ptr::null(), &function.frame_state(state), reason), + &Insn::Send { block: Some(BlockHandler::BlockArgProc(_)), .. } => unreachable!("BlockArgProc only appears in SendDirect"), &Insn::SendForward { cd, blockiseq, state, reason, .. } => gen_send_forward(jit, asm, function, cd, blockiseq, &function.frame_state(state), reason), Insn::SendDirect(insn) => { let SendDirectData { cd, cme, iseq, recv, args, kw_bits, jit_entry_idx, block, state, .. } = &**insn; + let block = block.map(|bh| match bh { + BlockHandler::BlockIseq(blockiseq) => lir::BlockHandler::Iseq(blockiseq), + BlockHandler::BlockArgProc(proc_id) => { + let proc_type = function.type_of(proc_id); + assert!( + proc_type.is_subtype(Type::from_class(unsafe { rb_cProc })), + "BlockArgProc operand must be a Proc, got {proc_type}", + ); + lir::BlockHandler::Proc(opnd!(proc_id)) + } + BlockHandler::BlockArg => unreachable!("BlockArg in SendDirect"), + }); gen_send_iseq_direct( cb, jit, asm, function, *cd, *cme, *iseq, opnd!(recv), opnds!(args), - *kw_bits, *jit_entry_idx, &function.frame_state(*state), *block, + *kw_bits, *jit_entry_idx, &function.frame_state(*state), block, ) } Insn::PushInlineFrame { cme, iseq, recv, num_args, blockiseq, state, .. } => { @@ -1798,7 +1811,7 @@ fn gen_send_iseq_direct( kw_bits: u32, jit_entry_idx: u16, state: &FrameState, - block: Option, + block: Option, ) -> lir::Opnd { gen_incr_counter(asm, Counter::iseq_optimized_send_count); @@ -1823,11 +1836,14 @@ fn gen_send_iseq_direct( gen_spill_locals(jit, asm, state); asm.stack_map(stack_map, jit_frame, state.depth); - // This mirrors vm_caller_setup_arg_block() in for the `blockiseq != NULL` case. - // The HIR specialization guards ensure we will only reach here for literal blocks, - // not &block forwarding, &:foo, etc. Thise are rejected in `type_specialize` by - // `unspecializable_call_type`. - let block_handler = block.map(|bh| match bh { BlockHandler::BlockIseq(b) => gen_block_handler_specval(asm, b), BlockHandler::BlockArg => unreachable!("BlockArg in gen_send_iseq_direct") }); + // This mirrors vm_caller_setup_arg_block(). + // Unsupported block args (BlockHandler::BlockArg) are rejected upstream in `type_specialize`. + let block_handler = block.map(|bh| match bh { + // the `blockiseq != NULL` case + lir::BlockHandler::Iseq(b) => gen_block_handler_specval(asm, b), + // the VM_CALL_ARGS_BLOCKARG case, where vm_to_proc(block_code) returns the given Proc as is + lir::BlockHandler::Proc(proc) => proc, + }); let callee_is_bmethod = VM_METHOD_TYPE_BMETHOD == unsafe { get_cme_def_type(cme) }; @@ -2091,8 +2107,7 @@ fn gen_invoke_block_iseq_direct( asm_comment!(asm, "switch to new CFP"); let new_cfp = asm.sub(CFP, RUBY_SIZEOF_CONTROL_FRAME.into()); - asm.mov(CFP, new_cfp); - asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP); + asm.mov(CFP, new_cfp); // will be published at `ec->cfp` after callee's entrypoint // JIT-to-JIT convention: self as c_args[0], then positional args. The block is // gated to simple + lead-only + exact arity, so there are no optionals/kw/block. diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 2ad70ce3b108ee..83a20ceb20659c 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -369,6 +369,51 @@ fn test_string_intern() { assert_snapshot!(assert_compiles(r#"test"#), @":foo123"); } +#[test] +fn test_string_to_sym_invalid_encoding_unused() { + eval(r#" + def test(str) + str.to_sym + :converted + end + "#); + assert_snapshot!(assert_compiles(r#" + test("warmup") + test("warmup") + begin + test("\xFF".force_encoding(Encoding::UTF_8)) + rescue EncodingError + :encoding_error + end + "#), @":encoding_error"); +} + +#[test] +fn test_string_subclass_to_sym() { + assert_snapshot!(assert_compiles(r#" + class MyString < String; end + def test(str) = str.to_sym + value = MyString.new("key") + test(value) + test(value) + [test(value), test(MyString.new("other"))] + "#), @"[:key, :other]"); +} + +#[test] +fn test_string_subclass_to_sym_redefined() { + assert_snapshot!(assert_compiles_allowing_exits(r#" + class MyString < String; end + def test(str) = str.to_sym + value = MyString.new("key") + test(value) + test(value) + original = test(value) + MyString.class_eval { def to_sym = :overridden } + [original, test(value)] + "#), @"[:key, :overridden]"); +} + #[test] fn test_duphash() { eval(" @@ -1997,6 +2042,79 @@ fn test_send_nil_block_arg() { "), @"false"); } +#[test] +fn test_send_proc_block_arg() { + assert_snapshot!(inspect(" + def foo = yield 3 + def test + blk = proc { |x| x * 2 } + foo(&blk) + end + test + test + "), @"6"); +} + +#[test] +fn test_send_proc_block_arg_side_exit() { + assert_snapshot!(inspect(" + def foo = yield 3 + def test(blk) = foo(&blk) + test(proc { |x| x * 2 }) + test(proc { |x| x * 2 }) + [test(proc { |x| x * 2 }), test(:succ)] + "), @"[6, 4]"); +} + +#[test] +fn test_send_proc_block_arg_lambda() { + assert_snapshot!(inspect(" + def foo = yield 3 + def test + blk = lambda { |x| x * 2 } + foo(&blk) + end + test + test + "), @"6"); +} + +#[test] +fn test_send_proc_block_arg_rest_optional_keyword_callee() { + // Callee has rest/optional/keyword params (so `prepare_direct_send_args` builds + // a `NewArray` for the rest param) and also `yield`s, so it's eligible for the + // guarded-Proc block-arg direct-send specialization. The NewArray allocation + // happens between the Proc guard and the callee send. + assert_snapshot!(inspect(" + def foo(opt = 10, *rest, kw: 20) = yield(opt + rest.sum + kw) + def test + blk = proc { |x| x + 1 } + foo(1, 2, &blk) + end + test + test + "), @"24"); +} + +#[test] +fn test_send_proc_subclass_block_arg_falls_back() { + // A Proc subclass instance is not an exact-class Proc, so the block arg's + // profiled type should not match `is_proc` (which requires class_exact:Proc), + // and the call should fall back to a dynamic send rather than being + // (incorrectly) treated as a guardable exact Proc. + assert_snapshot!(inspect(" + class MyProc < Proc; end + + def foo = yield 3 + def test + blk = MyProc.new { |x| x * 2 } + foo(&blk) + end + test + test + "), @"6"); +} + #[test] fn test_send_symbol_block_arg() { assert_snapshot!(inspect(" @@ -3650,6 +3768,29 @@ fn test_opt_eq_string_distinct_objects() { assert_contains_opcode("test", YARVINSN_opt_eq); } +#[test] +fn test_opt_eq_string_symbol_arg_after_inlining() { + eval(r#" + # frozen_string_literal: true + class Foo + def self.bar(l, r) = l == r + end + def test(flag) + foo = Foo + if flag + foo.bar("a", "b") + else + foo.bar("a", :sym) + end + end + "#); + assert_snapshot!(inspect(r#" + test(true) # profile opt_eq in bar + test(true) # compile test, inlining bar with a Symbol argument on the untaken branch + [test(true), test(false)] + "#), @"[false, false]"); +} + #[test] fn test_opt_eqq_string_same_operand() { assert_snapshot!(inspect(r#" @@ -6629,6 +6770,56 @@ fn test_profile_frames_during_direct_jit_to_jit_entry() { }); } +// Same as test_profile_frames_during_direct_jit_to_jit_entry, but for a direct `yield` to an ISEQ block. +#[cfg(all( + any(target_os = "linux", target_os = "macos"), + any(target_arch = "x86_64", target_arch = "aarch64"), +))] +#[test] +fn test_profile_frames_during_direct_block_entry() { + with_inlining_threshold(0, || { + eval(r#" + def profiled_yield_each(n) + i = 0 + while i < n + yield i + i += 1 + end + end + + def profiled_yield_shallow(n) + sum = 0 + profiled_yield_each(n) { |x| sum += x } + sum + end + + # Same VM frame depth as profiled_yield_shallow, on a deeper native stack + def profiled_yield_deep(n) + [n].each { |m| return __send__(:profiled_yield_shallow, m) } + end + + def profiled_yield_loop(n) + i = 0 + sum = 0 + while i < n + sum += profiled_yield_deep(1) + sum += profiled_yield_shallow(2) + i += 1 + end + sum + end + + profiled_yield_loop(3) + profiled_yield_loop(3) + profiled_yield_loop(3) + "#); + + let profiler = signal_profiler::Profiler::start(10); + assert_snapshot!(assert_compiles("profiled_yield_loop(1_000_000)"), @"1000000"); + assert!(profiler.samples() > 0, "rb_profile_frames was not called from SIGPROF handler"); + }); +} + #[test] fn test_profile_under_nested_jit_call() { assert_snapshot!(inspect(" diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index a3e76f21f5602f..50b92644e6a8da 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2222,6 +2222,7 @@ unsafe extern "C" { pub static mut rb_cModule: VALUE; pub static mut rb_cNilClass: VALUE; pub static mut rb_cNumeric: VALUE; + pub static mut rb_cProc: VALUE; pub static mut rb_cRange: VALUE; pub static mut rb_cRegexp: VALUE; pub static mut rb_cSet: VALUE; diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index 7daf8ce374c899..d77799a2afc3f8 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -277,6 +277,7 @@ pub fn init() -> Annotations { annotate!(rb_cFloat, "to_i", inline_float_to_i); annotate!(rb_cFloat, "to_int", inline_float_to_i); annotate!(rb_cString, "to_s", inline_string_to_s, types::StringExact); + annotate!(rb_cString, "to_sym", inline_string_to_sym, types::Symbol); annotate!(rb_cFloat, "nan?", types::BoolExact, no_gc, leaf, elidable); annotate!(rb_cFloat, "finite?", types::BoolExact, no_gc, leaf, elidable); annotate!(rb_cFloat, "infinite?", types::Fixnum.union(types::NilClass), no_gc, leaf, elidable); @@ -324,6 +325,15 @@ fn inline_string_to_s(fun: &mut hir::Function, block: hir::BlockId, recv: hir::I None } +fn inline_string_to_sym(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { + debug_assert!(args.is_empty()); + if fun.likely_a(recv, types::String, state) { + let recv = fun.coerce_to(block, recv, types::String, state); + return Some(fun.push_insn(block, hir::Insn::StringIntern { val: recv, state })); + } + None +} + fn inline_falseclass_and(fun: &mut hir::Function, block: hir::BlockId, _recv: hir::InsnId, args: &[hir::InsnId], _state: hir::InsnId) -> Option { // FalseClass#& just returns Qfalse and ignores its argument. let &[_] = args else { return None; }; diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index bd2278656ea683..0b174658d902fe 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -897,6 +897,10 @@ pub enum BlockHandler { BlockIseq(IseqPtr), /// Block arg passed via &proc (e.g. `foo(&block)`) BlockArg, + /// Block arg proven to be a Proc by a guard. The InsnId refers to the + /// guarded Proc value, which becomes the callee frame's block handler + /// (a Proc VALUE is itself a valid block handler). + BlockArgProc(InsnId), } /// Identifier used by LoadField/StoreField/LoadArg for HIR dumps. Variants @@ -1364,7 +1368,7 @@ pub enum Insn { /// `$visit_one` macro for a single InsnId field and `$visit_many` macro for a /// slice/Vec of InsnIds. Used by both `for_each_operand` and `for_each_operand_mut`. macro_rules! for_each_operand_impl { - ($self:expr, $visit_one:ident, $visit_many:ident) => { + ($self:expr, $visit_one:ident, $visit_many:ident $(, $mut:tt)?) => { match $self { Insn::Comment { .. } | Insn::Const { .. } @@ -1618,6 +1622,9 @@ macro_rules! for_each_operand_impl { Insn::SendDirect(insn) => { $visit_one!(insn.recv); $visit_many!(insn.args); + if let Some(BlockHandler::BlockArgProc(id)) = &$($mut)? insn.block { + $visit_one!(*id); + } $visit_one!(insn.state); } Insn::CCallWithFrame(insn) => { @@ -1756,7 +1763,7 @@ impl Insn { pub fn for_each_operand_mut(&mut self, mut f: impl FnMut(&mut InsnId)) { macro_rules! visit_one { ($p:expr) => { f(&mut $p) }; } macro_rules! visit_many { ($s:expr) => { for id in ($s).iter_mut() { f(id) } }; } - for_each_operand_impl!(self, visit_one, visit_many); + for_each_operand_impl!(self, visit_one, visit_many, mut); } /// Call `f` on each operand, short-circuiting on the first error. @@ -2230,10 +2237,14 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { }, Insn::SendDirect(insn) => { let SendDirectData { recv, cme, iseq, args, block, jit_entry_idx, .. } = &**insn; - let blockiseq = block.map(|bh| match bh { BlockHandler::BlockIseq(iseq) => iseq, BlockHandler::BlockArg => unreachable!() }); - let blockiseq_ptr = blockiseq.map_or(ptr::null(), |iseq| self.ptr_map.map_ptr(iseq)); + let block = match block { + Some(BlockHandler::BlockArgProc(proc_id)) => format!("&{proc_id}"), + Some(BlockHandler::BlockIseq(blockiseq)) => format!("{:p}", self.ptr_map.map_ptr(*blockiseq)), + Some(BlockHandler::BlockArg) => unreachable!("BlockArg in SendDirect"), + None => format!("{:p}", ptr::null::()), + }; let method_name = unsafe { (**cme).called_id }; - write!(f, "SendDirect {recv}, {blockiseq_ptr:p}, :{method_name} ({:?})", self.ptr_map.map_ptr(*iseq))?; + write!(f, "SendDirect {recv}, {block}, :{method_name} ({:?})", self.ptr_map.map_ptr(*iseq))?; if *jit_entry_idx != 0 { write!(f, ", jit_entry_idx={jit_entry_idx}")?; } @@ -2258,6 +2269,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { write!(f, "Send {recv}, {:p}, :{}", self.ptr_map.map_ptr(blockiseq), ruby_call_method_name(*cd))?, Some(BlockHandler::BlockArg) => write!(f, "Send {recv}, &block, :{}", ruby_call_method_name(*cd))?, + Some(BlockHandler::BlockArgProc(_)) => + unreachable!("BlockArgProc only appears in SendDirect"), None => write!(f, "Send {recv}, :{}", ruby_call_method_name(*cd))?, } @@ -2404,6 +2417,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { write!(f, ", block={:p}", self.ptr_map.map_ptr(*blockiseq))?, Some(BlockHandler::BlockArg) => write!(f, ", block=&block")?, + Some(BlockHandler::BlockArgProc(_)) => + unreachable!("BlockArgProc only appears in SendDirect"), None => {} } Ok(()) @@ -4013,6 +4028,8 @@ impl Function { } } + /// Extract the original value out of guards and RefineType instructions. Because it drops + /// the most recent type information, this should be used only for checking pointer eqality. fn chase_insn(&self, insn: InsnId) -> InsnId { let id = self.union_find.borrow().find_const(insn); match self.insns[id] { @@ -4061,19 +4078,23 @@ impl Function { } /// Materialize a validated SendDirect call in the selected runtime path. - fn emit_send_direct_args(&mut self, block: BlockId, call: SendDirectCall, original_args: &[InsnId], state: InsnId) -> SendDirectArgs { + /// + /// `send_frame_state` is the frame state the SendDirect uses, which strips a profiled-nil block arg from the stack. + /// `exit_state` is the pre-send frame state (block arg still on the stack). Any guard that side-exits before the call + /// re-executes the `send` in the interpreter, so it must reconstruct the stack with the block arg present. + fn emit_send_direct_args(&mut self, block: BlockId, call: SendDirectCall, original_args: &[InsnId], send_frame_state: InsnId, exit_state: InsnId) -> SendDirectArgs { let args: Vec<_> = call .args .into_iter() - .map(|arg| self.emit_send_direct_arg(block, arg, state)) + .map(|arg| self.emit_send_direct_arg(block, arg, exit_state)) .collect(); // If args were reordered or synthesized, create a new snapshot with the updated stack. let send_state = if args != original_args { - let new_state = self.frame_state(state).with_replaced_args(&args, original_args.len()); + let new_state = self.frame_state(send_frame_state).with_replaced_args(&args, original_args.len()); self.push_insn(block, Insn::Snapshot { state: Box::new(new_state) }) } else { - state + send_frame_state }; SendDirectArgs { @@ -4836,25 +4857,35 @@ impl Function { def_type = unsafe { get_cme_def_type(cme) }; } - // Check if we can optimize `foo(&block)` where block is nil to a send without block. + // Check if we can optimize `foo(&block)` to a direct send: either `block` is + // nil (strip it and send without a block) or `block` is monomorphically an + // exact-class Proc (pass it through as the callee frame's specval). // `state` keeps referring to the pre-send frame state (block arg still on the // stack). Any guard that side-exits before the call re-executes the `send` in // the interpreter, so it must reconstruct the stack with the block arg present. // Only the direct-send frame setup uses `send_frame_state`, which has the nil - // block arg stripped from the stack. + // or Proc block arg stripped from the stack. let mut send_block = send_block; let mut send_frame_state = state; let mut args = match resolved.insn(self) { Insn::Send { args, .. } => args.to_vec(), _ => panic!("Expected Send instruction"), }; - let mut stripped_nil_block = false; + let mut stripped_block_arg = false; if send_block == Some(BlockHandler::BlockArg) && def_type == VM_METHOD_TYPE_ISEQ { + // Reject complex argument passing before emitting any block-arg guard; + // a guard in front of a send that stays dynamic gates nothing. + if unspecializable_call_type(flags & !VM_CALL_ARGS_BLOCKARG) { + self.count_complex_call_features(block, flags, state); + self.set_dynamic_send_reason(insn_id, ComplexArgPass); + self.push_insn_id(block, insn_id); continue; + } // The block arg is the last element in args if let Some(&block_arg) = args.last() { + let original_argc = args.len(); let statically_nil = self.is_a(block_arg, types::NilClass); - let profiled_nil = self.profiled_type_of_at(block_arg, state) - .map_or(false, |pt| pt.is_nil()); + let block_arg_profiled_type = self.profiled_type_of_at(block_arg, state); + let profiled_nil = block_arg_profiled_type.map_or(false, |pt| pt.is_nil()); if statically_nil || profiled_nil { if !statically_nil { // Guard needed when relying on profiled type. Uses the original @@ -4878,13 +4909,26 @@ impl Function { args = args[..args.len() - 1].to_vec(); send_block = None; has_block = false; - stripped_nil_block = true; + stripped_block_arg = true; // Frame state for the direct send only: the block arg is removed // from the stack so the callee frame is laid out correctly. - let new_state = self.frame_state(state).with_replaced_args(&args, args.len() + 1); + let new_state = self.frame_state(state).with_replaced_args(&args, original_argc); + send_frame_state = self.push_insn(block, Insn::Snapshot { state: Box::new(new_state) }); + } else if block_arg_profiled_type.is_some_and(|pt| pt.is_proc()) { + // Guard the Proc and pass it through as the callee frame's + // specval. + let guarded = self.guard_type_recompile( + block, block_arg, + Type::from_profiled_type(block_arg_profiled_type.unwrap()), + state, Recompile, + ); + _ = args.pop(); + send_block = Some(BlockHandler::BlockArgProc(guarded)); + stripped_block_arg = true; + let new_state = self.frame_state(state).with_replaced_args(&args, original_argc); send_frame_state = self.push_insn(block, Insn::Snapshot { state: Box::new(new_state) }); } else { - // Can't prove block arg is nil + // Can't prove block arg is nil or a Proc self.set_dynamic_send_reason(insn_id, SendBlockArgNotNil); self.push_insn_id(block, insn_id); continue; } @@ -4893,8 +4937,8 @@ impl Function { // If the call site info indicates that the `Function` has overly complex arguments, then do not optimize into a `SendDirect`. // Optimized methods(`VM_METHOD_TYPE_OPTIMIZED`) and C methods handle their own argument constraints (e.g., kw_splat for Proc call). - // Mask out ARGS_BLOCKARG only if we've already handled the nil block arg case above. - let mut flags_for_check = if stripped_nil_block { flags & !VM_CALL_ARGS_BLOCKARG } else { flags }; + // Mask out ARGS_BLOCKARG only if we've already handled the nil/Proc block arg case above. + let mut flags_for_check = if stripped_block_arg { flags & !VM_CALL_ARGS_BLOCKARG } else { flags }; if def_type == VM_METHOD_TYPE_ISEQ { // Caller splat specialization currently only supports ISEQ callees, so // skip the generic splat rejection here and validate its profile below. @@ -4959,7 +5003,7 @@ impl Function { } let SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx } = - self.emit_send_direct_args(block, call, &args, send_frame_state); + self.emit_send_direct_args(block, call, &args, send_frame_state, state); let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, cd, cme, iseq, args: send_args, kw_bits, jit_entry_idx, state: send_state, block: send_block }))); self.make_equal_to(insn_id, replacement); } else if !has_block && def_type == VM_METHOD_TYPE_BMETHOD { @@ -5000,7 +5044,7 @@ impl Function { } let SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx } = - self.emit_send_direct_args(block, call, &args, send_frame_state); + self.emit_send_direct_args(block, call, &args, send_frame_state, state); let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, cd, cme, iseq, args: send_args, kw_bits, jit_entry_idx, state: send_state, block: None }))); self.make_equal_to(insn_id, replacement); } else if !has_block && def_type == VM_METHOD_TYPE_IVAR && args.is_empty() { @@ -5168,6 +5212,7 @@ impl Function { let blockiseq = match send_block { Some(BlockHandler::BlockArg) => unreachable!("unsupported &block should have been filtered out"), + Some(BlockHandler::BlockArgProc(_)) => unreachable!("BlockArgProc is only built for ISEQ callees"), Some(BlockHandler::BlockIseq(blockiseq)) => Some(blockiseq), None => None, }; @@ -5520,7 +5565,7 @@ impl Function { emit_super_call_guards(self, block, super_cme, current_cme, mid, state, frame_state_iseq); let SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx } = - self.emit_send_direct_args(block, call, &args, state); + self.emit_send_direct_args(block, call, &args, state, state); // Use SendDirect with the super method's CME and ISEQ. let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, @@ -5805,14 +5850,20 @@ impl Function { }; let SendDirectData { recv, cme, iseq, kw_bits, jit_entry_idx, block: call_block, state, .. } = **data; let args_len = data.args.len(); - // SendDirect invariant: block is either None or BlockIseq. - // BlockArg is rejected upstream during type specialization. + // TODO: Inline callees that receive a &proc block handler. The inlined body + // only knows a static blockiseq for yield/defined?(yield); it would need to + // be taught to dispatch to the runtime Proc instead. + if matches!(call_block, Some(BlockHandler::BlockArgProc(_))) { + search_start = send_pos + 1; + continue; + } // TODO(max): If we accept BlockArg here, we need to change the folding of Defined // in HIR construction for the defined opcode to check the send flags of the method // being inlined, too. let blockiseq: Option = call_block.map(|bh| match bh { BlockHandler::BlockIseq(bi) => bi, BlockHandler::BlockArg => unreachable!("BlockArg in SendDirect"), + BlockHandler::BlockArgProc(_) => unreachable!("skipped above"), }); // Apply the cheap optimization heuristics (size, budget, denylist) @@ -6849,11 +6900,9 @@ impl Function { } } &Insn::StringEqual { left, right } => { - let left = self.chase_insn(left); - let right = self.chase_insn(right); // If both operands resolve to the same SSA value, // String#== is guaranteed to be true. - if left == right { + if self.chase_insn(left) == self.chase_insn(right) { self.new_insn(Insn::Const { val: Const::Value(Qtrue) }) } else { let left_type = self.type_of(left); @@ -6862,7 +6911,7 @@ impl Function { (Some(left_obj), Some(right_obj)) if left_obj.is_frozen() && right_obj.is_frozen() => { - // For known frozen objects, evaluate String#== at compile time. + // For known frozen Strings, evaluate String#== at compile time. let val = unsafe { rb_yarv_str_eql_internal(left_obj, right_obj) }; self.new_insn(Insn::Const { val: Const::Value(val) }) } @@ -8021,7 +8070,7 @@ impl Function { } // Instructions with String operands Insn::StringCopy { val, .. } => self.assert_subtype(insn_id, val, types::StringExact), - Insn::StringIntern { val, .. } => self.assert_subtype(insn_id, val, types::StringExact), + Insn::StringIntern { val, .. } => self.assert_subtype(insn_id, val, types::String), Insn::StringAppend { recv, other, recv_flags, other_flags, .. } => { self.assert_subtype(insn_id, recv, types::StringExact)?; self.assert_subtype(insn_id, other, types::String)?; @@ -9719,21 +9768,36 @@ fn add_iseq_to_hir( queue.push_back((state.clone(), target, target_idx, local_inval)); break; // Don't enqueue the next block as a successor } - YARVINSN_getlocal_WC_0 => { + opcode @ (YARVINSN_getlocal | YARVINSN_getlocal_WC_0 | YARVINSN_getlocal_WC_1) => { let ep_offset = get_arg(pc, 0).as_u32(); - if !local_inval { + let level = match opcode { + YARVINSN_getlocal => get_arg(pc, 1).as_u32(), + YARVINSN_getlocal_WC_0 => 0, + YARVINSN_getlocal_WC_1 => 1, + _ => unreachable!() + }; + + if level != 0 { + // Load local from EP; no change to FrameState as it describes level 0. + let ep = fun.get_ep(block, level); + let val = fun.get_local_from_ep(block, iseq, ep, ep_offset, level, types::BasicObject); + state.stack_push(val); + } else if !local_inval { + assert!(level == 0); // from place in decision tree // The FrameState is the source of truth for locals until invalidated. // In case of JIT-to-JIT send locals might never end up in EP memory. let val = state.getlocal(ep_offset); state.stack_push(val); } else if ep_escaped { + assert!(level == 0); // from place in decision tree // Read the local using EP let ep = fun.get_ep(block, 0); let val = fun.get_local_from_ep(block, iseq, ep, ep_offset, 0, types::BasicObject); state.setlocal(ep_offset, val); // remember the result to spill on side-exits state.stack_push(val); } else { - assert!(local_inval); // if check above + assert!(local_inval); // from place in decision tree + assert!(level == 0); // from place in decision tree // There has been some non-leaf call since JIT entry or the last patch point, // so add a patch point to make sure locals have not been escaped. let exit_id = fun.push_insn(block, Insn::Snapshot { state: Box::new(exit_state.without_locals()) }); // skip spilling locals @@ -9745,51 +9809,37 @@ fn add_iseq_to_hir( state.stack_push(val); } } - YARVINSN_setlocal_WC_0 => { + opcode @ (YARVINSN_setlocal | YARVINSN_setlocal_WC_0 | YARVINSN_setlocal_WC_1) => { let ep_offset = get_arg(pc, 0).as_u32(); + let level = match opcode { + YARVINSN_setlocal => get_arg(pc, 1).as_u32(), + YARVINSN_setlocal_WC_0 => 0, + YARVINSN_setlocal_WC_1 => 1, + _ => unreachable!(), + }; let val = state.stack_pop()?; - if ep_escaped { + + if level != 0 { + fun.push_insn(block, Insn::SetLocal { val, ep_offset, level, state: exit_id }); + } else if ep_escaped { + assert!(level == 0); // from place in decision tree // Write the local using EP fun.push_insn(block, Insn::SetLocal { val, ep_offset, level: 0, state: exit_id }); - } else if local_inval { + state.setlocal(ep_offset, val); + } else if !local_inval { + assert!(level == 0); // from place in decision tree + assert!(!ep_escaped); // from place in decision tree + state.setlocal(ep_offset, val); + } else { + assert!(local_inval); // from place in decision tree + assert!(level == 0); // from place in decision tree // If there has been any non-leaf call since JIT entry or the last patch point, // add a patch point to make sure locals have not been escaped. let exit_id = fun.push_insn(block, Insn::Snapshot { state: Box::new(exit_state.without_locals()) }); // skip spilling locals fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoEPEscape(iseq), state: exit_id }); local_inval = false; + state.setlocal(ep_offset, val); } - // Write the local into FrameState - state.setlocal(ep_offset, val); - } - YARVINSN_getlocal_WC_1 => { - let ep_offset = get_arg(pc, 0).as_u32(); - let ep = fun.get_ep(block, 1); - state.stack_push(fun.get_local_from_ep(block, iseq, ep, ep_offset, 1, types::BasicObject)); - } - YARVINSN_setlocal_WC_1 => { - let ep_offset = get_arg(pc, 0).as_u32(); - fun.push_insn(block, Insn::SetLocal { val: state.stack_pop()?, ep_offset, level: 1, state: exit_id }); - } - YARVINSN_getlocal => { - let ep_offset = get_arg(pc, 0).as_u32(); - let level = get_arg(pc, 1).as_u32(); - if level == 0 && !local_inval { - // Same optimization as getlocal_WC_0: use FrameState - let val = state.getlocal(ep_offset); - state.stack_push(val); - } else { - let ep = fun.get_ep(block, level); - let val = fun.get_local_from_ep(block, iseq, ep, ep_offset, level, types::BasicObject); - if level == 0 { - state.setlocal(ep_offset, val); - } - state.stack_push(val); - } - } - YARVINSN_setlocal => { - let ep_offset = get_arg(pc, 0).as_u32(); - let level = get_arg(pc, 1).as_u32(); - fun.push_insn(block, Insn::SetLocal { val: state.stack_pop()?, ep_offset, level, state: exit_id }); } YARVINSN_setblockparam => { let ep_offset = get_arg(pc, 0).as_u32(); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 1041ca448eb3d2..d8179b45424fe3 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -26,6 +26,32 @@ mod hir_opt_tests { hir_string_proc(&format!("{}.method(:{})", "self", method)) } + #[test] + fn test_send_direct_rest_array_keeps_stripped_block_arg_in_exit_state() { + eval(" + def rest_callee(*args) = args + def test(obj, &block) = rest_callee(obj, &block) + test(1) + test(1) + "); + let iseq = crate::cruby::with_rubyvm(|| get_method_iseq("self", "test")); + unsafe { crate::cruby::rb_zjit_profile_disable(iseq) }; + let mut function = iseq_to_hir(iseq).unwrap(); + function.optimize(); + function.validate().unwrap(); + + let new_array_stack_sizes: Vec = (0..function.num_insns()) + .map(InsnId::from) + .filter_map(|insn_id| match function.find(insn_id) { + Insn::NewArray { state, .. } => Some(function.frame_state(state).stack().len()), + _ => None, + }) + .collect(); + // [self, obj, block] + assert!(!new_array_stack_sizes.is_empty(), "{}", hir_string_function(&function)); + assert!(new_array_stack_sizes.iter().all(|&size| size == 3), "{new_array_stack_sizes:?}"); + } + #[test] fn test_fold_iftrue_away() { eval(" @@ -4630,6 +4656,44 @@ mod hir_opt_tests { v6:BasicObject = LoadArg :self@0 v7:BasicObject = LoadArg :l@1 Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v22:ObjectSubclass[class_exact:Proc] = GuardType v10, ObjectSubclass[class_exact:Proc] recompile + PatchPoint MethodRedefined(Object@0x1008, foo@0x1010, cme:0x1018) + v25:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v9, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v26:BasicObject = SendDirect v25, &v22, :foo (0x1040) + CheckInterrupts + Return v26 + "); + } + + #[test] + fn test_yield_proc_subclass_falls_back() { + // A Proc subclass instance profiles with a class other than exactly Proc, so + // `ProfiledType::is_proc()` is false and the block arg specialization does not + // apply; the call stays a dynamic Send instead of being (incorrectly) treated + // as a guardable exact Proc. + let result = eval(" + class MyProc < Proc; end + def foo = yield(5) + def test(blk) = foo(&blk) + blk = MyProc.new { |x| x * 10 } + test(blk) + test(blk) + "); + assert_eq!(VALUE::fixnum_from_usize(50), result); + assert_snapshot!(hir_string("test"), @" + fn test@:4: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :blk@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :blk@1 + Jump bb3(v6, v7) bb3(v9:BasicObject, v10:BasicObject): v16:BasicObject = Send v9, &block, :foo, v10 # SendFallbackReason: Send: block argument is not nil CheckInterrupts @@ -7066,9 +7130,11 @@ mod hir_opt_tests { v49:BasicObject = LoadField v18, :VM_ENV_DATA_INDEX_SPECVAL@0x1003 Jump bb6(v49, v10) bb6(v16:BasicObject, v17:BasicObject): - v52:BasicObject = Send v14, &block, :then, v16 # SendFallbackReason: Send: block argument is not nil + v58:ObjectSubclass[class_exact:Proc] = GuardType v16, ObjectSubclass[class_exact:Proc] recompile + PatchPoint MethodRedefined(Integer@0x1010, then@0x1018, cme:0x1020) + v62:BasicObject = SendDirect v14, &v58, :then (0x1048) CheckInterrupts - Return v52 + Return v62 "); } @@ -12259,6 +12325,236 @@ mod hir_opt_tests { "); } + #[test] + fn test_send_with_proc_block_arg_specialized() { + eval(r#" + def foo = yield + + def test + blk = proc { 42 } + foo(&blk) + end + test; test + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v5:BasicObject = LoadArg :self@0 + v6:NilClass = Const Value(nil) + v7:CPtr = GetEP 0 + StoreField v7, :blk@0x1000, v6 + Jump bb3(v5) + bb3(v10:BasicObject): + v42:NilClass = Const Value(nil) + PatchPoint MethodRedefined(Object@0x1008, proc@0x1010, cme:0x1018) + v35:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v10, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v36:BasicObject = CCallWithFrame v35, :Kernel#proc@0x1040, block=0x1048 + v17:CPtr = GetEP 0 + v18:BasicObject = LoadField v17, :blk@0x1000 + SetLocal :blk, l0, EP@3, v36 + v24:CPtr = GetEP 0 + v25:BasicObject = LoadField v24, :blk@0x1000 + v37:ObjectSubclass[class_exact:Proc] = GuardType v25, ObjectSubclass[class_exact:Proc] recompile + PatchPoint MethodRedefined(Object@0x1008, foo@0x1068, cme:0x1070) + v41:BasicObject = SendDirect v35, &v37, :foo (0x1098) + CheckInterrupts + Return v41 + "); + } + + #[test] + fn test_send_with_proc_block_arg_to_block_param_callee_falls_back() { + eval(r#" + def foo(&b) = b.call + + def test + blk = proc { 42 } + foo(&blk) + end + test; test + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v5:BasicObject = LoadArg :self@0 + v6:NilClass = Const Value(nil) + v7:CPtr = GetEP 0 + StoreField v7, :blk@0x1000, v6 + Jump bb3(v5) + bb3(v10:BasicObject): + v39:NilClass = Const Value(nil) + PatchPoint MethodRedefined(Object@0x1008, proc@0x1010, cme:0x1018) + v35:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v10, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v36:BasicObject = CCallWithFrame v35, :Kernel#proc@0x1040, block=0x1048 + v17:CPtr = GetEP 0 + v18:BasicObject = LoadField v17, :blk@0x1000 + SetLocal :blk, l0, EP@3, v36 + v24:CPtr = GetEP 0 + v25:BasicObject = LoadField v24, :blk@0x1000 + v37:ObjectSubclass[class_exact:Proc] = GuardType v25, ObjectSubclass[class_exact:Proc] recompile + v27:BasicObject = Send v35, &block, :foo, v37 # SendFallbackReason: Complex argument passing + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_send_with_proc_block_arg_to_blockless_callee_falls_back() { + eval(r#" + def foo = 42 + + def test + blk = proc { 42 } + foo(&blk) + end + test; test + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v5:BasicObject = LoadArg :self@0 + v6:NilClass = Const Value(nil) + v7:CPtr = GetEP 0 + StoreField v7, :blk@0x1000, v6 + Jump bb3(v5) + bb3(v10:BasicObject): + v39:NilClass = Const Value(nil) + PatchPoint MethodRedefined(Object@0x1008, proc@0x1010, cme:0x1018) + v35:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v10, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v36:BasicObject = CCallWithFrame v35, :Kernel#proc@0x1040, block=0x1048 + v17:CPtr = GetEP 0 + v18:BasicObject = LoadField v17, :blk@0x1000 + SetLocal :blk, l0, EP@3, v36 + v24:CPtr = GetEP 0 + v25:BasicObject = LoadField v24, :blk@0x1000 + v37:ObjectSubclass[class_exact:Proc] = GuardType v25, ObjectSubclass[class_exact:Proc] recompile + v27:BasicObject = Send v35, &block, :foo, v37 # SendFallbackReason: Complex argument passing + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_send_with_proc_block_arg_to_rest_optional_keyword_callee() { + // The callee has rest/optional/keyword params, so `prepare_direct_send_args` + // builds a `NewArray` for the rest param. That allocation sits between the + // Proc `GuardType` and the `SendDirect` that stores the Proc into the callee + // frame's specval, so the guarded Proc is only reachable from the VReg at that + // point (it is not in the block-arg-stripped snapshot). + eval(r#" + def foo(opt = 10, *rest, kw: 20) = yield(opt + rest.sum + kw) + + def test + blk = proc { |x| x + 1 } + foo(1, 2, &blk) + end + test; test + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v5:BasicObject = LoadArg :self@0 + v6:NilClass = Const Value(nil) + v7:CPtr = GetEP 0 + StoreField v7, :blk@0x1000, v6 + Jump bb3(v5) + bb3(v10:BasicObject): + v49:NilClass = Const Value(nil) + PatchPoint MethodRedefined(Object@0x1008, proc@0x1010, cme:0x1018) + v39:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v10, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v40:BasicObject = CCallWithFrame v39, :Kernel#proc@0x1040, block=0x1048 + v17:CPtr = GetEP 0 + v18:BasicObject = LoadField v17, :blk@0x1000 + SetLocal :blk, l0, EP@3, v40 + v24:Fixnum[1] = Const Value(1) + v26:Fixnum[2] = Const Value(2) + v28:CPtr = GetEP 0 + v29:BasicObject = LoadField v28, :blk@0x1000 + v41:ObjectSubclass[class_exact:Proc] = GuardType v29, ObjectSubclass[class_exact:Proc] recompile + PatchPoint MethodRedefined(Object@0x1008, foo@0x1068, cme:0x1070) + v45:ArrayExact = NewArray v26 + v46:Fixnum[20] = Const Value(20) + v48:BasicObject = SendDirect v39, &v41, :foo (0x1098), jit_entry_idx=1, v24, v45, v46 + CheckInterrupts + Return v48 + "); + } + + #[test] + fn test_send_with_splat_and_proc_block_arg_falls_back() { + // Splat plus a block arg is complex argument passing, so this falls back to a + // dynamic send. The fallback must happen *before* the Proc `GuardType` is + // emitted: a guard in front of a dynamic send gates nothing and would only pay + // a side exit plus a recompile on every non-Proc block arg. + eval(r#" + def foo(a) = yield a + + def test + blk = proc { |x| x + 1 } + args = [1] + foo(*args, &blk) + end + test; test + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:NilClass = Const Value(nil) + v8:CPtr = GetEP 0 + StoreField v8, :blk@0x1000, v7 + v10:NilClass = Const Value(nil) + StoreField v8, :args@0x1001, v10 + Jump bb3(v6) + bb3(v13:BasicObject): + v54:NilClass = Const Value(nil) + v53:NilClass = Const Value(nil) + PatchPoint MethodRedefined(Object@0x1008, proc@0x1010, cme:0x1018) + v51:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v13, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v52:BasicObject = CCallWithFrame v51, :Kernel#proc@0x1040, block=0x1048 + v21:CPtr = GetEP 0 + v22:BasicObject = LoadField v21, :blk@0x1000 + v23:BasicObject = LoadField v21, :args@0x1001 + SetLocal :blk, l0, EP@4, v52 + v28:ArrayExact[VALUE(0x1068)] = Const Value(VALUE(0x1068)) + v29:ArrayExact = ArrayDup v28 + SetLocal :args, l0, EP@3, v29 + v35:CPtr = GetEP 0 + v36:BasicObject = LoadField v35, :args@0x1001 + v38:ArrayExact = ToArray v36 + v40:CPtr = GetEP 0 + v41:BasicObject = LoadField v40, :blk@0x1000 + v43:BasicObject = Send v51, &block, :foo, v38, v41 # SendFallbackReason: Complex argument passing + CheckInterrupts + Return v43 + "); + } + #[test] fn test_inline_attr_reader_constant() { eval(" @@ -12840,6 +13136,94 @@ mod hir_opt_tests { "); } + #[test] + fn test_inline_string_to_sym() { + eval(r#" + def test(str) = str.to_sym + test("warmup") + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :str@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :str@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, to_sym@0x1010, cme:0x1018) + v24:StringExact = GuardType v10, StringExact recompile + v25:Symbol = StringIntern v24 + CheckInterrupts + Return v25 + "); + } + + #[test] + fn test_inline_string_subclass_to_sym() { + eval(r#" + class MyString < String; end + def test(str) = str.to_sym + test(MyString.new("warmup")) + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :str@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :str@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + PatchPoint NoSingletonClass(MyString@0x1008) + PatchPoint MethodRedefined(MyString@0x1008, to_sym@0x1010, cme:0x1018) + v24:StringSubclass[class_exact:MyString] = GuardType v10, StringSubclass[class_exact:MyString] recompile + v25:Symbol = StringIntern v24 + CheckInterrupts + Return v25 + "); + } + + #[test] + fn test_inline_string_intern() { + eval(r#" + def test(str) = str.intern + test("warmup") + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :str@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :str@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, intern@0x1010, cme:0x1018) + v24:StringExact = GuardType v10, StringExact recompile + v25:Symbol = StringIntern v24 + CheckInterrupts + Return v25 + "); + } + #[test] fn test_fixnum_to_s_returns_string() { eval(r#" @@ -17193,6 +17577,72 @@ mod hir_opt_tests { "); } + #[test] + fn test_not_fold_string_equal_non_string_through_guard() { + eval(r#" + # frozen_string_literal: true + class Foo + def self.bar(l, r) = l == r + end + def test(flag) + foo = Foo + if flag + foo.bar("a", "b") + else + foo.bar("a", :sym) + end + end + test(true) + test(true) + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:7: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :flag@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :flag@1 + Jump bb3(v7, v8) + bb3(v11:BasicObject, v12:BasicObject): + v101:NilClass = Const Value(nil) + PatchPoint StableConstantNames(0x1008, Foo) + v18:ClassSubclass[Foo@0x1010] = Const Value(VALUE(0x1010)) + PatchPoint NoEPEscape(test) + v25:CBool = Test v12 + v26:Falsy = RefineType v12, Falsy + CondBranch v25, bb5(), bb4() + bb5(): + v28:Truthy = RefineType v12, Truthy + v32:StringExact[VALUE(0x1018)] = Const Value(VALUE(0x1018)) + v34:StringExact[VALUE(0x1020)] = Const Value(VALUE(0x1020)) + PatchPoint MethodRedefined(Class@0x1028, bar@0x1030, cme:0x1038) + PushInlineFrame :bar, v18 (0x1060), num_args=2 + PatchPoint NoSingletonClass(String@0x1080) + PatchPoint MethodRedefined(String@0x1080, ==@0x1088, cme:0x1090) + v113:FalseClass = Const Value(false) + PopInlineFrame + CheckInterrupts + Return v113 + bb4(): + v48:StringExact[VALUE(0x1018)] = Const Value(VALUE(0x1018)) + v50:StaticSymbol[:sym] = Const Value(VALUE(0x10b8)) + PatchPoint MethodRedefined(Class@0x1028, bar@0x1030, cme:0x1038) + PushInlineFrame :bar, v18 (0x1060), num_args=2 + PatchPoint NoSingletonClass(String@0x1080) + PatchPoint MethodRedefined(String@0x1080, ==@0x1088, cme:0x1090) + v111 = GuardType v50, String + v112:BoolExact = StringEqual v48, v111 + PopInlineFrame + CheckInterrupts + Return v112 + "); + } + #[test] fn opt_neq_string_nil_falls_back_to_basic_object_neq() { eval(r#" @@ -21481,7 +21931,8 @@ mod hir_opt_tests { bb5(): v21:Truthy = RefineType v12, Truthy v25:Fixnum[42] = Const Value(42) - v28:BasicObject = Send v11, &block, :passthrough_recompile_blockarg, v25, v13 # SendFallbackReason: Send: block argument is not nil + v46:ObjectSubclass[class_exact:Proc] = GuardType v13, ObjectSubclass[class_exact:Proc] recompile + v28:BasicObject = Send v11, &block, :passthrough_recompile_blockarg, v25, v46 # SendFallbackReason: Complex argument passing CheckInterrupts Return v28 bb4(): diff --git a/zjit/src/hir/tests.rs b/zjit/src/hir/tests.rs index 964209a3f1d981..ca8c9fa8e7c51c 100644 --- a/zjit/src/hir/tests.rs +++ b/zjit/src/hir/tests.rs @@ -1206,6 +1206,38 @@ pub(crate) mod hir_build_tests { "); } + #[test] + fn test_setlocal_getlocal_no_operands_unification() { + eval_with_options(" + def test(a) + x = a + x = 2 + x + end + ", "{ operands_unification: false }"); + assert_contains_opcodes("test", &[YARVINSN_getlocal, YARVINSN_setlocal]); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :a@0x1000 + v4:NilClass = Const Value(nil) + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :a@1 + v9:NilClass = Const Value(nil) + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:NilClass): + v20:Fixnum[2] = Const Value(2) + CheckInterrupts + Return v20 + "); + } + #[test] fn test_nested_setlocal_getlocal() { eval(" diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index 1afdd64059e51c..91bdad07765e91 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -342,6 +342,17 @@ impl ProfiledType { self.class == unsafe { rb_cInteger } && self.flags.is_immediate() } + /// Whether the profiled class is exactly Proc (subclasses return false). + /// + /// This is stricter than the interpreter, which accepts anything for which + /// `rb_obj_is_proc()` is true. That checks the object's typed data type, not its + /// class, so Proc subclasses pass. We use the class as a conservative approximation: + /// Proc has no allocator, so an object whose class is exactly Proc always has + /// `proc_data_type`. Subclasses fall back to a dynamic send. + pub fn is_proc(&self) -> bool { + self.class == unsafe { rb_cProc } + } + pub fn is_string(&self) -> bool { if self.flags.is_object_profiling() { panic!("should not call is_string on object-profiled ProfiledType");