diff options
| author | NARUSE, Yui <naruse@airemix.jp> | 2023-01-18 17:18:44 +0900 |
|---|---|---|
| committer | NARUSE, Yui <naruse@airemix.jp> | 2023-01-18 17:18:44 +0900 |
| commit | 97c32b49e2fe4de8b57ce05146e63b2aa64c7a44 (patch) | |
| tree | f145b3c0300a78e7b70ca28e841555243a0d39d7 | |
| parent | 52ea5ea9906c3a96c60a68e01b303672602a6832 (diff) | |
merge revision(s) 43ff0c2c488c80aaf83b486d45bcd4a92ebe3848: [Backport #19299]
YJIT: Fix `yield` into block with >=30 locals on ARM
It's a register spill issue. Fix by moving the Qnil fill snippet to
after registers are released.
[Bug #19299]
---
test/ruby/test_yjit.rb | 14 ++++++++++++++
yjit/src/codegen.rs | 29 ++++++++++++++---------------
2 files changed, 28 insertions(+), 15 deletions(-)
| -rw-r--r-- | test/ruby/test_yjit.rb | 14 | ||||
| -rw-r--r-- | version.h | 2 | ||||
| -rw-r--r-- | yjit/src/codegen.rs | 29 |
3 files changed, 29 insertions, 16 deletions
diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb index 0ad443c885..1a552a1074 100644 --- a/test/ruby/test_yjit.rb +++ b/test/ruby/test_yjit.rb @@ -1028,6 +1028,20 @@ class TestYJIT < Test::Unit::TestCase RUBY end + def test_invokeblock_many_locals + # [Bug #19299] + assert_compiles(<<~'RUBY', result: :ok) + def foo + yield + end + + foo do + a1=a2=a3=a4=a5=a6=a7=a8=a9=a10=a11=a12=a13=a14=a15=a16=a17=a18=a19=a20=a21=a22=a23=a24=a25=a26=a27=a28=a29=a30 = :ok + a30 + end + RUBY + end + private def code_gc_helpers @@ -11,7 +11,7 @@ # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 0 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 6 +#define RUBY_PATCHLEVEL 7 #include "ruby/version.h" #include "ruby/internal/abi.h" diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index e12a29240d..0347f6fc89 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -4455,19 +4455,6 @@ fn gen_push_frame( }; asm.store(Opnd::mem(64, sp, SIZEOF_VALUE_I32 * -2), specval); - // Arm requires another register to load the immediate value of Qnil before storing it. - // So doing this after releasing the register for specval to avoid register spill. - let num_locals = frame.local_size; - if num_locals > 0 { - asm.comment("initialize locals"); - - // Initialize local variables to Qnil - for i in 0..num_locals { - let offs = (SIZEOF_VALUE as i32) * (i - num_locals - 3); - asm.store(Opnd::mem(64, sp, offs), Qnil.into()); - } - } - // Write env flags at sp[-1] // sp[-1] = frame_type; asm.store(Opnd::mem(64, sp, SIZEOF_VALUE_I32 * -1), frame.frame_type.into()); @@ -4505,6 +4492,20 @@ fn gen_push_frame( asm.mov(cfp_opnd(RUBY_OFFSET_CFP_SELF), frame.recv); asm.mov(cfp_opnd(RUBY_OFFSET_CFP_BLOCK_CODE), 0.into()); + // This Qnil fill snippet potentially requires 2 more registers on Arm, one for Qnil and + // another for calculating the address in case there are a lot of local variables. So doing + // this after releasing the register for specval and the receiver to avoid register spill. + let num_locals = frame.local_size; + if num_locals > 0 { + asm.comment("initialize locals"); + + // Initialize local variables to Qnil + for i in 0..num_locals { + let offs = (SIZEOF_VALUE as i32) * (i - num_locals - 3); + asm.store(Opnd::mem(64, sp, offs), Qnil.into()); + } + } + if set_sp_cfp { // Saving SP before calculating ep avoids a dependency on a register // However this must be done after referencing frame.recv, which may be SP-relative @@ -4778,8 +4779,6 @@ fn gen_return_branch( } } - - /// Pushes arguments from an array to the stack that are passed with a splat (i.e. *args) /// It optimistically compiles to a static size that is the exact number of arguments /// needed for the function. |
